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

[Daniel Dunbar]

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.

Again, please don't just drop support for a feature without understanding
why it was there.

 - Daniel


> >
> >      # If non-null, a tuple of the last test code and name.
> >      self.lastTestResult = None
> > @@ -48,8 +50,10 @@ class LitLogObserver(LogLineObserver):
> >      # We have finished getting information for one test, handle it.
> >      code, name = self.lastTestResult
> >
> > -    # If the test failed, add a log entry for it.
> > -    if code in self.failingCodes:
> > +    # If the test failed, add a log entry for it (unless we have
> reached the
> > +    # max).
> > +    if code in self.failingCodes and (self.maxLogs is None or
> > +                                      self.numLogs < self.maxLogs):
> >        # If a verbose log was not provided, just add a one line
> description.
> >        if self.activeVerboseLog is None:
> >          self.activeVerboseLog = ['%s: %s' % (code, name)]
> > @@ -57,6 +61,7 @@ class LitLogObserver(LogLineObserver):
> >        # Add the log to the build status.
> >        self.step.addCompleteLog(name.replace('/', '__'),
> >                                 '\n'.join(self.activeVerboseLog))
> > +      self.numLogs += 1
> >
> >      # Reset the current state.
> >      self.lastTestResult = None
> > @@ -112,7 +117,9 @@ class LitTestCommand(Test):
> >    def __init__(self, ignore=[], flaky=[], max_logs=20,
> >                 *args, **kwargs):
> >      Test.__init__(self, *args, **kwargs)
> > -    self.logObserver = LitLogObserver()
> > +    self.maxLogs = int(max_logs)
> > +    self.logObserver = LitLogObserver(self.maxLogs)
> > +    self.addFactoryArguments(max_logs=max_logs)
> >      self.addLogObserver('stdio', self.logObserver)
> >
> >    def evaluateCommand(self, cmd):
> > @@ -182,8 +189,8 @@ FAIL: test-three (3 of 3)
> >          ('test-three', 'FAIL: test-three')])
> >
> >  class TestCommand(unittest.TestCase):
> > -  def parse_log(self, text):
> > -    cmd = LitTestCommand()
> > +  def parse_log(self, text, **kwargs):
> > +    cmd = LitTestCommand(**kwargs)
> >      cmd.logObserver.step = StepProxy()
> >      for ln in text.split('\n'):
> >        cmd.logObserver.outLineReceived(ln)
> > @@ -200,5 +207,12 @@ class TestCommand(unittest.TestCase):
> >        cmd = self.parse_log("""%s: test-one (1 of 1)""" %
> (failing_code,))
> >        self.assertEqual(cmd.evaluateCommand(RemoteCommandProxy(0)),
> FAILURE)
> >
> > +  def test_max_logs(self):
> > +    cmd = self.parse_log("""
> > +FAIL: test-one (1 of 2)
> > +FAIL: test-two (2 of 2)
> > +""", max_logs=1)
> > +    self.assertEqual(cmd.logObserver.step.logs, [('test-one', 'FAIL:
> test-one')])
> > +
> >  if __name__ == '__main__':
> >    unittest.main()
> >
> >
> > _______________________________________________
> > llvm-commits mailing list
> > llvm-commits at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130201/3e553ac3/attachment.html>