[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries
Daniel Vetter
daniel at ffwll.ch
Sat Mar 15 05:36:08 PDT 2014
On Fri, Nov 22, 2013 at 12:10:43AM +0100, Marek Olšák wrote:
> It looks like people have different expectations from "notrun" and
> "skip", so I have a proposal:
> - add a notrun page similar to the skipped page
> - don't include the notrun and skip statuses in the regression and fixes pages
> - add the following options that affect the behavior of notrun and
> skip when generating the regression and fixes pages:
> 1) --skip-as-pass
> 2) --notrun-as-pass
> 3) --skip-worse-than-fail (pass->skip is a regression, fail->skip is a
> regression, crash->skip is a fix; the crash is and always should be
> the worst status)
> 4) --notrun-worse-than-fail
I kinda missed that this change to make "skip" better than "pass" went in
... is anyone working on flags to frob this around?
For the kernel due to the "never break abi" rule I really want to count
"skip" as worse than "fail" (or anything else fwiw). If no one's working
on this I'll look into adding a hack for tests/igt.py.
-Daniel
>
> Marek
>
> On Thu, Nov 21, 2013 at 10:33 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> > 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
> >
> > _______________________________________________
> > Piglit mailing list
> > Piglit at lists.freedesktop.org
> > http://lists.freedesktop.org/mailman/listinfo/piglit
> >
> _______________________________________________
> Piglit mailing list
> Piglit at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/piglit
--
Daniel Vetter
Software Engineer, Intel Corporation
+41 (0) 79 365 57 48 - http://blog.ffwll.ch
More information about the Piglit
mailing list