[Piglit] [PATCH 1/2] summary: fix regression and fixes summaries

[Dylan Baker]

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.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 490 bytes
Desc: This is a digitally signed message part.
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20131120/5ff00c0b/attachment.pgp>