[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries

Daniel Vetter daniel at ffwll.ch
Thu Nov 21 00:32:52 PST 2013


On Wed, Nov 20, 2013 at 03:04:34PM -0800, Dylan Baker wrote:
> On Monday, November 18, 2013 03:33:33 PM Marek Olšák wrote:
> > From: Marek Olšák <marek.olsak at amd.com>
> > 
> > Somebody broke this.
> > ---
> >  framework/status.py        | 48 +++++++++++++--------------------
> >  framework/summary.py       |  6 ++---
> >  framework/tests/summary.py | 66
> > ++++++++++++++++------------------------------ 3 files changed, 44
> > insertions(+), 76 deletions(-)
> > 
> > diff --git a/framework/status.py b/framework/status.py
> > index 3a9e2d3..c8f7375 100644
> > --- a/framework/status.py
> > +++ b/framework/status.py
> > @@ -21,6 +21,8 @@
> > 
> >  """ Status ordering from best to worst:
> > 
> > +NotRun
> > +skip
> >  pass
> >  dmesg-warn
> >  warn
> > @@ -28,33 +30,21 @@ dmesg-fail
> >  fail
> >  crash
> >  timeout
> > -skip
> > -
> > -
> > -The following are regressions:
> > -
> > -pass|warn|dmesg-warn|fail|dmesg-fail|crash|timeout -> skip
> > -pass|warn|dmesg-warn|fail|dmesg-fail|crash -> timeout|skip
> > -pass|warn|dmesg-warn|fail|dmesg-fail -> crash|timeout|skip
> > -pass|warn|dmesg-warn|fail -> dmesg-fail|crash|timeout|skip
> > -pass|warn|dmesg-warn -> fail|dmesg-fail|crash|timeout|skip
> > -pass|warn -> dmesg-warn|fail|dmesg-fail|crash|timeout|skip
> > -pass -> warn|dmesg-warn|fail|dmesg-fail|crash|timeout|skip
> > 
> > +(NotRun, pass, skip) are considered equivalent for regression testing.
> > 
> > -The following are fixes:
> > +The motivation is if you accidentally expose a feature that doesn't work,
> > +you'll get skip->fail, which is a regression. If you disable the feature,
> > +you'll get fail->skip, which is a fix.
> > 
> > -skip -> pass|warn|dmesg-warn|fail|dmesg-fail|crash|timeout
> > -timeout|skip -> pass|warn|dmesg-warn|fail|dmesg-fail|crash
> > -crash|timeout|skip - >pass|warn|dmesg-warn|fail|dmesg-fail
> > -dmesg-fail|crash|timeout|skip -> pass|warn|dmesg-warn|fail
> > -fail|dmesg-fail|crash|timeout|skip -> pass|warn|dmesg-warn
> > -dmesg-warn|fail|dmesg-fail|crash|timeout|skip -> pass|warn
> > -warn|dmesg-warn|fail|dmesg-fail|crash|timeout|skip -> pass
> > +NotRun->fail should also be considered a regression for you not to miss
> > +new failing tests.
> > 
> > +The formula for determining regressions is:
> > +  max(old_status, pass) < new_status
> > 
> > -NotRun -> * and * -> NotRun is a change, but not a fix or a regression.
> > This is -because NotRun is not a status, but a representation of an unknown
> > status. +The formula for determining fixes is:
> > +  old_status > max(new_status, pass)
> > 
> >  """
> > 
> > @@ -159,6 +149,13 @@ class NotRun(Status):
> >      def __init__(self):
> >          pass
> > 
> > +class Skip(Status):
> > +    name = 'skip'
> > +    value = 5
> > +    fraction = (0, 0)
> > +
> > +    def __init__(self):
> > +        pass
> > 
> >  class Pass(Status):
> >      name = 'pass'
> > @@ -217,10 +214,3 @@ class Timeout(Status):
> >          pass
> > 
> > 
> > -class Skip(Status):
> > -    name = 'skip'
> > -    value = 60
> > -    fraction = (0, 0)
> > -
> > -    def __init__(self):
> > -        pass
> > diff --git a/framework/summary.py b/framework/summary.py
> > index bbb423f..6ee1226 100644
> > --- a/framework/summary.py
> > +++ b/framework/summary.py
> > @@ -306,7 +306,7 @@ class Summary:
> > 
> >              # Problems include: warn, dmesg-warn, fail, dmesg-fail, and
> > crash. # Skip does not go on this page, it has the 'skipped' page -        
> >    if so.Skip() > max(status) > so.Pass():
> > +            if max(status) > so.Pass():
> >                  self.tests['problems'].add(test)
> > 
> >              # Find all tests with a status of skip
> > @@ -317,9 +317,9 @@ class Summary:
> >              for i in xrange(len(status) - 1):
> >                  first = status[i]
> >                  last = status[i + 1]
> > -                if first < last and so.NotRun() not in (first, last):
> > +                if max(first, so.Pass()) < last:
> >                      self.tests['regressions'].add(test)
> > -                if first > last and so.NotRun() not in (first, last):
> > +                if first > max(last, so.Pass()):
> >                      self.tests['fixes'].add(test)
> >                  # Changes cannot be added in the fixes and regressions
> > passes # becasue NotRun is a change, but not a regression or fix diff --git
> > a/framework/tests/summary.py b/framework/tests/summary.py index
> > de67c1d..b0905aa 100644
> > --- a/framework/tests/summary.py
> > +++ b/framework/tests/summary.py
> > @@ -38,37 +38,7 @@ from helpers import test_iterations, create_testresult,
> > create_test
> > 
> >  """ Status ordering from best to worst:
> > 
> > -pass
> > -dmesg-warn
> > -warn
> > -dmesg-fail
> > -fail
> > -crash
> > -skip
> > -
> > -
> > -The following are regressions:
> > -
> > -pass|warn|dmesg-warn|fail|dmesg-fail|crash -> skip
> > -pass|warn|dmesg-warn|fail|dmesg-fail -> crash|skip
> > -pass|warn|dmesg-warn|fail -> dmesg-fail|crash|skip
> > -pass|warn|dmesg-warn -> fail|dmesg-fail|crash|skip
> > -pass|warn -> dmesg-warn|fail|dmesg-fail|crash|skip
> > -pass -> warn|dmesg-warn|fail|dmesg-fail|crash|skip
> > -
> > -
> > -The following are fixes:
> > -
> > -skip -> pass|warn|dmesg-warn|fail|dmesg-fail|crash
> > -crash|skip - >pass|warn|dmesg-warn|fail|dmesg-fail
> > -dmesg-fail|crash|skip -> pass|warn|dmesg-warn|fail
> > -fail|dmesg-fail|crash|skip -> pass|warn|dmesg-warn
> > -dmesg-warn|fail|dmesg-fail|crash|skip -> pass|warn
> > -warn|dmesg-warn|fail|dmesg-fail|crash|skip -> pass
> > -
> > -
> > -NotRun -> * and * -> NotRun is a change, but not a fix or a regression.
> > This is -because NotRun is not a status, but a representation of an unknown
> > status. +See ../summary.py.
> > 
> >  """
> > 
> > @@ -85,47 +55,55 @@ REGRESSIONS = [("pass", "warn"),
> >                 ("pass", "dmesg-warn"),
> >                 ("pass", "fail"),
> >                 ("pass", "dmesg-fail"),
> > -               ("pass", "skip"),
> >                 ("pass", "crash"),
> >                 ("dmesg-warn", "warn"),
> >                 ("dmesg-warn", "dmesg-fail"),
> >                 ("dmesg-warn", "fail"),
> >                 ("dmesg-warn", "crash"),
> > -               ("dmesg-warn", "skip"),
> >                 ("warn", "fail"),
> >                 ("warn", "crash"),
> > -               ("warn", "skip"),
> >                 ("warn", "dmesg-fail"),
> >                 ("dmesg-fail", "crash"),
> > -               ("dmesg-fail", "skip"),
> >                 ("dmesg-fail", "fail"),
> >                 ("fail", "crash"),
> > -               ("fail", "skip"),
> > -               ("crash", "skip")]
> > +               ("skip", "crash"),
> > +               ("skip", "fail"),
> > +               ("skip", "dmesg-fail"),
> > +               ("skip", "warn"),
> > +               ("skip", "dmesg-warn"),
> > +               ("notrun", "crash"),
> > +               ("notrun", "fail"),
> > +               ("notrun", "dmesg-fail"),
> > +               ("notrun", "warn"),
> > +               ("notrun", "dmesg-warn")]
> > 
> > 
> >  # List of possible fixes
> > -FIXES = [("skip", "crash"),
> > -         ("skip", "fail"),
> > -         ("skip", "dmesg-fail"),
> > -         ("skip", "warn"),
> > -         ("skip", "dmesg-warn"),
> > -         ("skip", "pass"),
> > -         ("crash", "fail"),
> > +FIXES = [("crash", "fail"),
> >           ("crash", "dmesg-fail"),
> >           ("crash", "warn"),
> >           ("crash", "dmesg-warn"),
> >           ("crash", "pass"),
> > +         ("crash", "skip"),
> > +         ("crash", "notrun"),
> >           ("fail", "dmesg-fail"),
> >           ("fail", "warn"),
> >           ("fail", "dmesg-warn"),
> >           ("fail", "pass"),
> > +         ("fail", "skip"),
> > +         ("fail", "notrun"),
> >           ("dmesg-fail", "warn"),
> >           ("dmesg-fail", "dmesg-warn"),
> >           ("dmesg-fail", "pass"),
> > +         ("dmesg-fail", "skip"),
> > +         ("dmesg-fail", "notrun"),
> >           ("warn", "dmesg-warn"),
> >           ("warn", "pass"),
> > +         ("warn", "skip"),
> > +         ("warn", "notrun"),
> >           ("dmesg-warn", "pass")]
> > +         ("dmesg-warn", "skip")]
> > +         ("dmesg-warn", "notrun")]
> > 
> > 
> >  # List of statuses that should be problems.
> 
> I finally have had a chance to look at this and have some concerns.
> I don't like your interpretation of skips as fixes and regressions, but I'm 
> not strongly attached to them, so if other developers like that whatever.

At least for the kernel the current behaviour of anything -> skip counting
as a regression is the right thing - we never break backwards compat. So
if a test suddenly starts skipping that means something broke somewhere.

So I'm strongly in favour of the current placing of the 'skip' result in
the ordering. Maybe we need a runtime switch to put skip either at the end
or beginning depending upon the requirements of what's getting tested (or
how the testing approach is overall).
-Daniel
-- 
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch


More information about the Piglit mailing list