[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries
Dylan Baker
baker.dylan.c at gmail.com
Thu Nov 21 13:33:18 PST 2013
On Thursday, November 21, 2013 12:22:31 PM Tom Stellard 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.
> >
> > However, I'm completely against "not run" being a fix or a regeression,
> > becasue they are not. a test with that status litterly wasn't run. It
> > doesn't have any status. IF it had been run it might have been the same,
> > or it might have been different, It cannot be a fix or a regression.
>
> There is one use case that I run into a lot where it is useful to have
> "Not Run" show up on the changes pages. If a test with subtests crashes
> all of its subtests show up as "Not Run", so if you introduce a
> regressions that crashes a test with subtests, you will never find out about
> if "Not Run" status is not included on the changes page.
That was the previous behavior, what I'm complaining about is not run showing
up in fixes and regressions.
>
> -Tom
>
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
-------------- 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/cead7e0b/attachment-0001.pgp>
More information about the Piglit
mailing list