[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries
Dylan Baker
baker.dylan.c at gmail.com
Thu Nov 21 13:32:43 PST 2013
On Thursday, November 21, 2013 09:32:52 AM Daniel Vetter wrote:
> 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
Cool, if we have a usecase for skip as a regression, great. no more arguments
from me.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131121/cbe4d706/attachment.pgp>
More information about the Piglit
mailing list