[zorg] r174139 - LitTestCommand: Add back support for max_logs argument.

[Daniel Dunbar]

On Fri, Feb 1, 2013 at 9:31 AM, David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Feb 1, 2013 at 9:14 AM, Daniel Dunbar <daniel at zuster.org> wrote:
> > On Fri, Feb 1, 2013 at 8:20 AM, David Blaikie <dblaikie at gmail.com>
> wrote:
> >>
> >> On Thu, Jan 31, 2013 at 5:44 PM, Daniel Dunbar <daniel at zuster.org>
> wrote:
> >> > Author: ddunbar
> >> > Date: Thu Jan 31 19:44:23 2013
> >> > New Revision: 174139
> >> >
> >> > URL: http://llvm.org/viewvc/llvm-project?rev=174139&view=rev
> >> > Log:
> >> > LitTestCommand: Add back support for max_logs argument.
> >> >
> >> > Modified:
> >> >     zorg/trunk/zorg/buildbot/commands/LitTestCommand.py
> >> >
> >> > Modified: zorg/trunk/zorg/buildbot/commands/LitTestCommand.py
> >> > URL:
> >> >
> http://llvm.org/viewvc/llvm-project/zorg/trunk/zorg/buildbot/commands/LitTestCommand.py?rev=174139&r1=174138&r2=174139&view=diff
> >> >
> >> >
> ==============================================================================
> >> > --- zorg/trunk/zorg/buildbot/commands/LitTestCommand.py (original)
> >> > +++ zorg/trunk/zorg/buildbot/commands/LitTestCommand.py Thu Jan 31
> >> > 19:44:23 2013
> >> > @@ -20,9 +20,11 @@ class LitLogObserver(LogLineObserver):
> >> >    # step results.
> >> >    failingCodes = set(['FAIL', 'XPASS', 'KPASS', 'UNRESOLVED'])
> >> >
> >> > -  def __init__(self):
> >> > +  def __init__(self, maxLogs=None):
> >> >      LogLineObserver.__init__(self)
> >> >      self.resultCounts = {}
> >> > +    self.maxLogs = maxLogs
> >> > +    self.numLogs = 0
> >>
> >> What's the particular motivation for this feature?
> >
> >
> > This is mainly just to protect the buildbot and web UI. In cases where
> there
> > is a catastrophic failure of some kind, we don't want buildbot to be
> showing
> > 13k tests and logs in the web UI.
> >
> > The primary goal of the extracted logs is to make it easy for users to
> jump
> > examine a specific failure. This is substantially less useful as more
> test
> > failures are present so it makes sense to cap it at some point.
> >
> >> Lit does the same
> >> thing - logging every failure & it's not terribly usable there, it
> >> seems much more usable in a web UI where you can scroll around a list,
> >> click on the failures, etc. If it's really a problem, shouldn't we
> >> just fix lit to do this instead?
> >
> >
> > I don't think it is a problem in the lit console based context.
>
> Why not? It seems easier to navigate in a web UI than a console.
>

That's just my opinion, tempered by experience. The console has one log
that scrolls up and down, it doesn't blow up as bad with tons of failures.
The buildbot UI doesn't scale in the same way.

> Again, please don't just drop support for a feature without understanding
> > why it was there.
>
> I asked whether anyone wanted to pre-commit review my changes - the
> general opinion was that post-commit review was just as good because
> it'd be done before anyone updated their buildmasters anyway.
>

My understanding was that you were just rewriting things, not changing
functionality.

 - Daniel


> - David
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130201/3a408ec4/attachment.html>