[Piglit] [PATCH 3/7] framework/summary.py: Fix problems with status handling

[Kenneth Graunke]

On 09/27/2013 02:39 PM, Dylan Baker wrote:
> Fixes status hanlding (fixes, regressions, changes) to be compliant with
> the status handling in framework_tests/summary.py. Mainly this addresses
> notrun -> * and * -> notrun, as well as a few corner cases.
> 
> Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
> ---
>  framework/summary.py | 47 +++++++++++++++++++++++++----------------------
>  1 file changed, 25 insertions(+), 22 deletions(-)
> 
> diff --git a/framework/summary.py b/framework/summary.py
> index 7b768b7..b579457 100644
> --- a/framework/summary.py
> +++ b/framework/summary.py
> @@ -323,12 +323,16 @@ class Summary:
>                      return 1
>                  elif status == 'pass':
>                      return 2
> -                elif status in ['warn', 'dmesg-warn']:
> +                elif status == 'dmesg-warn':
>                      return 3
> -                elif status in ['fail', 'dmesg-fail']:
> +                elif status == 'warn':
>                      return 4
> -                elif status == 'crash':
> +                elif status == 'dmesg-fail':
>                      return 5
> +                elif status == 'fail':
> +                    return 6
> +                elif status == 'crash':
> +                    return 7
>  
>              openGroup('fake')
>              openGroup('all')
> @@ -419,14 +423,18 @@ class Summary:
>              """
>              if status == 'pass':
>                  return 1
> -            elif status in ['warn', 'dmesg-warn']:
> +            elif status == 'dmesg-warn':
>                  return 2
> -            elif status in ['fail', 'dmesg-fail']:
> +            elif status == 'warn':
>                  return 3
> -            elif status == 'skip':
> +            elif status == 'dmesg-fail':
>                  return 4
> -            elif status == 'crash':
> +            elif status == 'fail':
>                  return 5
> +            elif status == 'crash':
> +                return 6
> +            elif status == 'skip':
> +                return 7
>              elif status == 'special':
>                  return 0
>  
> @@ -438,34 +446,29 @@ class Summary:
>                  except KeyError:
>                      status.append(find_regressions("special"))
>  
> -            if 'changes' in lists:
> -                # Check and append self.tests['changes']
> -                # A set cannot contain duplicate entries, so creating a set
> -                # out the list will reduce it's length to 1 if all entries
> -                # are the same, meaning it is not a change
> -                if len(set(status)) > 1:
> -                    self.tests['changes'].add(test)
> -
>              if 'problems' in lists:
> -                # If the result contains a value other than 1 (pass) or 4
> -                # (skip) it is a problem. Skips are not problems becasuse
> -                # they have Their own page.
> -                if [i for e in [2, 3, 5] for i in status if e is i]:
> +                # Problems include: warn, dmes-warn, fail, dmesg-fail, and

Typo here: dmes-warn -> dmesg-warn

> +                # crash
> +                if 7 > max(status) > 1:

Interesting - I didn't realize that you could chain comparisons like
this.  But you can, according to
http://docs.python.org/2/reference/expressions.html:

"Comparisons can be chained arbitrarily, e.g., x < y <= z is equivalent
to x < y and y <= z, except that y is evaluated only once (but in both
cases z is not evaluated at all when x < y is found to be false)."

But...shouldn't this be "7 >= max(status) > 1"?  Otherwise it looks like
it'll miss crash.  Or for that matter, just "max(status) > 1"?


>                      self.tests['problems'].add(test)
>  
>              if 'skipped' in lists:
>                  # Find all tests with a status of skip
> -                if 4 in status:
> +                if 6 in status:

6 is crash.  Don't you mean 7?

>                      self.tests['skipped'].add(test)
>  
>              if 'fixes' in lists:
>                  # Find both fixes and regressions, and append them to the
>                  # proper lists
>                  for i in xrange(len(status) - 1):
> -                    if status[i] < status[i + 1] and status[i] != 0:
> +                    first = status[i]
> +                    last = status[i + 1]
> +                    if first < last and 0 not in (first, last):
>                          self.tests['regressions'].add(test)
> -                    if status[i] > 1 and status[i + 1] == 1:
> +                    if first > last and 0 not in (first, last):
>                          self.tests['fixes'].add(test)
> +                    if first != last:
> +                        self.tests['changes'].add(test)
>  
>      def __find_totals(self):
>          """
> 

Other than fixing those, which appear to be last minute changes, this
looks okay.  Still, did you actually run your tests on this?  Kind of
concerning...

With those fixed, this would get a:
Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>