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

[David Blaikie]

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.

> 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.

- David