On Thu, Nov 21, 2013 at 12:04 AM, Dylan Baker <[email protected]> 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.
It's the way I originally implemented the "fixes" and "regressions" pages. Since when is "skip -> crash" a fix? > > 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. Well, changes like "fail -> notrun" usually don't happen in my case, because if I create a summary from multiple test runs, there are the same sets of tests. Now that I fixed it :), I don't have to look at the "changes" page anymore, and it will be very useful for me to be able to see new tests that fail in the regressions page. That was the whole point of it: not having to look at the changes. If people don't like this, I think we should add either new pages for different usage cases or a command-line switch to select the desired behavior. Marek _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
