[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries
Marek Olšák
maraeo at gmail.com
Thu Nov 21 15:10:43 PST 2013
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
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
>
More information about the Piglit
mailing list