[Piglit] [PATCH] framework: fix comparing NOTRUN/SKIP to other statuses

[Ilia Mirkin]

On Sat, Apr 5, 2014 at 11:29 AM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Saturday, April 05, 2014 10:31:46 AM Ilia Mirkin wrote:
>> The current logic causes transitions involving notrun/skip to not show
>> up in problems. Remove the special handling of lt/gt/etc so that
>> max(status) can work, and reorder the logic that figures out which
>> category to select.
>>
>> Cc: Chris Forbes <chrisf at ijw.co.nz>
>> Cc: Dylan Baker <baker.dylan.c at gmail.com>
>> Signed-off-by: Ilia Mirkin <imirkin at alum.mit.edu>
>> ---
>>
>> Chris reported this on IRC, and then I went back and noticed that I was
>> getting the error too, although I didn't notice.
>>
>> Dylan, I'm not sure why you did things the way that you had it... but this
>> appears to work just fine.
>>
>>  framework/status.py  | 12 ------------
>>  framework/summary.py | 16 ++++++----------
>>  2 files changed, 6 insertions(+), 22 deletions(-)
>>
>> diff --git a/framework/status.py b/framework/status.py
>> index bb7c594..d5e5f8e 100644
>> --- a/framework/status.py
>> +++ b/framework/status.py
>> @@ -195,12 +195,6 @@ class NoChangeStatus(Status):
>>      def __init__(self, name, value=0, fraction=(0, 0)):
>>          super(NoChangeStatus, self).__init__(name, value, fraction)
>>
>> -    def __lt__(self, other):
>> -        return False
>> -
>> -    def __le__(self, other):
>> -        return False
>> -
>>      def __eq__(self, other):
>>          if isinstance(other, (str, unicode, Status)):
>>              return unicode(self) == unicode(other)
>> @@ -211,12 +205,6 @@ class NoChangeStatus(Status):
>>              return unicode(self) != unicode(other)
>>          raise TypeError("Cannot compare type: {}".format(type(other)))
>>
>> -    def __ge__(self, other):
>> -        return False
>> -
>> -    def __gt__(self, other):
>> -        return False
>> -
>>
>>  NOTRUN = NoChangeStatus('Not Run')
>>
>> diff --git a/framework/summary.py b/framework/summary.py
>> index 6d2a7bb..eab0a4e 100644
>> --- a/framework/summary.py
>> +++ b/framework/summary.py
>> @@ -325,20 +325,16 @@ class Summary:
>>              for i in xrange(len(status) - 1):
>>                  first = status[i]
>>                  last = status[i + 1]
>> -                if first < last:
>> -                    self.tests['regressions'].add(test)
>> -                    self.tests['changes'].add(test)
>> -                    continue
>> -                elif first > last:
>> -                    self.tests['fixes'].add(test)
>> -                    self.tests['changes'].add(test)
>> -                    continue
>> -
>>                  if first in [so.SKIP, so.NOTRUN] and last not in [so.SKIP,
>> so.NOTRUN]: self.tests['enabled'].add(test)
>> -                    self.tests['changes'].add(test)
>>                  elif last in [so.SKIP, so.NOTRUN] and first not in
>> [so.SKIP, so.NOTRUN]: self.tests['disabled'].add(test)
>> +                elif first < last:
>> +                    self.tests['regressions'].add(test)
>> +                elif first > last:
>> +                    self.tests['fixes'].add(test)
>> +
>> +                if first != last:
>>                      self.tests['changes'].add(test)
>>
>>      def __find_totals(self):
>
> There was a reason I did it the way I did, if you compare comprehensive
> results to subset results, the page is flooded with 'Not Run' results. this is
> a really common work flow for doing early hardware bring up. Maybe we'd be
> better off not putting skips on the disabled page?
>
> I've attached an example of what I mean.

I'm confused. Which page are you talking about?

Oh, disabled doesn't currently count as a change? That's fine, I can
put that back in, I'm fairly ambivalent about it. That behavioural
change was actually accidental, I didn't notice that that case was
different. Will send out a fixed version shortly.