[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