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

Tue Apr 8 13:02:49 PDT 2014

On Tue, Apr 8, 2014 at 3:53 PM, Dylan Baker <baker.dylan.c at gmail.com> wrote:
> On Tuesday, April 08, 2014 15:44:54 Ilia Mirkin wrote:
>
>> On Tue, Apr 8, 2014 at 3:36 PM, Dylan Baker <baker.dylan.c at gmail.com>
>> wrote:
>
>> > On Monday, April 07, 2014 22:51:27 Ilia Mirkin wrote:
>
>> >> The current logic causes transitions involving notrun/skip to not show
>
>> >>
>
>> >> up in problems. It also encodes display decisions in lt/gt/etc
>
>> >>
>
>> >> operators. This change adjusts the status class to be logically simpler
>
>> >>
>
>> >> by removing the special handling of lt/gt/etc so that max(status) can
>
>> >>
>
>> >> work, and reorders the logic that figures out which category to select.
>
>> >>
>
>> >>
>
>> >>
>
>> >> Tests are updated to encode the new order -- notrun/skip are now
>
>> >>
>
>> >> comparable to (and less than) the other statuses, but are unordered
>
>> >>
>
>> >> between one another.
>
>> >>
>
>> >>
>
>> >>
>
>> >> 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>
>
>> >>
>
>> >> ---
>
>> >>
>
>> >>
>
>> >>
>
>> >> v2 -> v3:
>
>> >>
>
>> >>
>
>> >>
>
>> >> - Modify tests to pass, add some more that exemplify the desired
>
>> >> behaviour
>
>> >>
>
>> >>
>
>> >>
>
>> >> framework/status.py | 23 ++++++-----------------
>
>> >>
>
>> >> framework/summary.py | 14 +++++---------
>
>> >>
>
>> >> framework/tests/status_tests.py | 30 +++++++++++++++++++++---------
>
>> >>
>
>> >> framework/tests/summary_tests.py | 5 ++++-
>
>> >>
>
>> >> 4 files changed, 36 insertions(+), 36 deletions(-)
>
>> >>
>
>> >>
>
>> >>
>
>> >> diff --git a/framework/status.py b/framework/status.py
>
>> >>
>
>> >> index bb7c594..aea3650 100644
>
>> >>
>
>> >> --- a/framework/status.py
>
>> >>
>
>> >> +++ b/framework/status.py
>
>> >>
>
>> >> @@ -158,10 +158,10 @@ class Status(object):
>
>> >>
>
>> >> return unicode(self.name)
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __lt__(self, other):
>
>> >>
>
>> >> - return int(self) < int(other)
>
>> >>
>
>> >> + return not self.__ge__(other)
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __le__(self, other):
>
>> >>
>
>> >> - return int(self) <= int(other)
>
>> >>
>
>> >> + return not self.__gt__(other)
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __eq__(self, other):
>
>> >>
>
>> >> # This must be int or status, since status comparisons are done
>
>> >>
>
>> >> using @@ -173,13 +173,14 @@ class Status(object):
>
>> >>
>
>> >> raise TypeError("Cannot compare type: {}".format(type(other)))
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __ne__(self, other):
>
>> >>
>
>> >> - return int(self) != int(other)
>
>> >>
>
>> >> + return self.fraction != other.fraction or int(self) != int(other)
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __ge__(self, other):
>
>> >>
>
>> >> - return int(self) >= int(other)
>
>> >>
>
>> >> + return self.fraction[1] > other.fraction[1] or (
>
>> >>
>
>> >> + self.fraction[1] == other.fraction[1] and int(self) >=
>
>> >>
>
>> >> int(other))
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __gt__(self, other):
>
>> >>
>
>> >> - return int(self) > int(other)
>
>> >>
>
>> >> + return self.fraction[1] > other.fraction[1] or int(self) >
>
>> >>
>
>> >> int(other)
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __int__(self):
>
>> >>
>
>> >> return self.value
>
>> >>
>
>> >> @@ -195,12 +196,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 +206,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..a4aa136 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)
>
>> >>
>
>> >> + self.tests['changes'].add(test)
>
>> >>
>
>> >> + elif first > last:
>
>> >>
>
>> >> + self.tests['fixes'].add(test)
>
>> >>
>
>> >> self.tests['changes'].add(test)
>
>> >>
>
>> >>
>
>> >>
>
>> >> def __find_totals(self):
>
>> >>
>
>> >> diff --git a/framework/tests/status_tests.py
>
>> >>
>
>> >> b/framework/tests/status_tests.py index ad0630a..ec6bf18 100644
>
>> >>
>
>> >> --- a/framework/tests/status_tests.py
>
>> >>
>
>> >> +++ b/framework/tests/status_tests.py
>
>> >>
>
>> >> @@ -33,13 +33,18 @@ import framework.status as status
>
>> >>
>
>> >> # tested separately because of upcoming features for it
>
>> >>
>
>> >> STATUSES = ["pass", "dmesg-warn", "warn", "dmesg-fail", "fail",
>> >> "crash"]
>
>> >>
>
>> >>
>
>> >>
>
>> >> +# all statuses except pass are problems
>
>> >>
>
>> >> +PROBLEMS = STATUSES[1:]
>
>> >>
>
>> >> +
>
>> >>
>
>> >> # Create lists of fixes and regressions programmatically based on the
>
>> >>
>
>> >> STATUSES # list. This means less code, and easier expansion changes.
>
>> >>
>
>> >> -REGRESSIONS = itertools.combinations(STATUSES, 2)
>
>> >>
>
>> >> -FIXES = itertools.combinations(reversed(STATUSES), 2)
>
>> >>
>
>> >> +REGRESSIONS = list(itertools.combinations(STATUSES, 2)) + \
>
>> >>
>
>> >> + list(itertools.combinations(["skip"] + PROBLEMS, 2))
>
>> >>
>
>> >> +FIXES = list(itertools.combinations(reversed(STATUSES), 2)) + \
>
>> >>
>
>> >> + list(itertools.combinations(list(reversed(PROBLEMS)) + ["skip"],
>
>> >>
>
>> >> 2))
>
>> >
>
>> > is there a reason to list the second part twice?
>
>>
>
>> Not sure which second part you're talking about... just making sure
>
>> that transitions from skip -> warn counts as a "problem" and warn ->
>
>> skip counts as a "fix" at least as far as lt and gt are concerned. The
>
>> actual policy of what to show on the various pages is left in
>
>> summary.py.
>
>
>
> list(itertools.combinations(list(reversed(PROBLEMS)) + ["skip"]

Oh, can itertools.combinations take an iter? I'll check. I just
assumed it couldn't. [Hm, it clearly can, based on the code 1 line
above it. Oops. I'll change that before pushing.]

  -ilia