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

[Dylan Baker]

On Sunday, October 13, 2013 11:08:27 PM Kenneth Graunke wrote:
> 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

fixed
> 
> > +                # 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"?

No. 7 is skip, and skips don't go on the problems page, only on the skipped 
page. I've updated the comment to reflect this.

> 
> >                      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?

whoops. yes, I meant 7
fixed

> 
> >                      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...

It's passing all of the tests, but the tests are obviously incomplete, as 
there is nothing to check that only skips are ending up on the skipped page. 
I'll add a test for that.

> 
> With those fixed, this would get a:
> Reviewed-by: Kenneth Graunke <kenneth at whitecape.org>
-------------- 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/20131014/258b6c39/attachment.pgp>