[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries
Marek Olšák
maraeo at gmail.com
Sat Mar 15 07:09:33 PDT 2014
I've been thinking about it and I think the best solution would be to
add 2 new pages:
- "feature changes" containing changes from "skip" to "anything but
skip and notrun" and vice versa.
- "test changes" containing changes from "notrun" to "anything but
notrun" and vice versa.
Marek
On Sat, Mar 15, 2014 at 1:36 PM, Daniel Vetter <daniel at ffwll.ch> wrote:
> 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