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 <[email protected]> Cc: Dylan Baker <[email protected]> Signed-off-by: Ilia Mirkin <[email protected]> --- 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)) -# all statuses except pass are problems -PROBLEMS = STATUSES[1:] +# The statuses that don't cause changes when transitioning from one another +NO_OPS = ('skip', 'notrun') def initialize_status(): @@ -147,13 +152,20 @@ def test_not_change(): """ Skip and NotRun should not count as changes """ yieldable = check_not_change - for nochange, stat in itertools.product(['skip', 'notrun'], STATUSES): + for nochange, stat in itertools.permutations(NO_OPS, 2): yieldable.description = "{0} -> {1} should not be a change".format( nochange, stat) yield yieldable, status.status_lookup(nochange), status.status_lookup(stat) -def test_compare_statuses(): - """ Compare NOTRUN -> PASS returns false """ - nt.assert_equal(False, status.NOTRUN < status.PASS, - msg="NOTRUN -> PASS returned True but should return False") +def test_max_statuses(): + """ Verify that max() works between skip and non-skip statuses """ + for nochange, stat in itertools.product(NO_OPS, STATUSES): + nochange = status.status_lookup(nochange) + stat = status.status_lookup(stat) + nt.assert_equal( + stat, max(nochange, stat), + msg="max({nochange}, {stat}) = {stat}".format(**locals())) + nt.assert_equal( + stat, max(stat, nochange), + msg="max({stat}, {nochange}) = {stat}".format(**locals())) diff --git a/framework/tests/summary_tests.py b/framework/tests/summary_tests.py index d6856a9..1ad51b5 100644 --- a/framework/tests/summary_tests.py +++ b/framework/tests/summary_tests.py @@ -48,7 +48,10 @@ def test_summary_add_to_set(): ('skip', 'dmesg-warn', 'enabled'), ('fail', 'notrun', 'disabled'), ('crash', 'skip', 'disabled'), - ('skip', 'skip', 'skipped')]: + ('skip', 'skip', 'skipped'), + ('notrun', 'fail', 'problems'), + ('fail', 'notrun', 'problems'), + ('pass', 'fail', 'problems')]: check_sets.description = "{0} -> {1} should be added to {2}".format( ostat, nstat, set_) -- 1.8.3.2 _______________________________________________ Piglit mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/piglit
