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 <[email protected]> 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 <[email protected]> >> 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 <[email protected]> >> >> > > >> >> > > 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 >> >> > [email protected] >> >> > http://lists.freedesktop.org/mailman/listinfo/piglit >> > >> > _______________________________________________ >> > Piglit mailing list >> > [email protected] >> > http://lists.freedesktop.org/mailman/listinfo/piglit >> > >> _______________________________________________ >> Piglit mailing list >> [email protected] >> http://lists.freedesktop.org/mailman/listinfo/piglit > > -- > Daniel Vetter > Software Engineer, Intel Corporation > +41 (0) 79 365 57 48 - http://blog.ffwll.ch _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
