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

[Dylan Baker]

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?
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/piglit/attachments/20140408/3887c6cc/attachment-0001.html>
-------------- 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/20140408/3887c6cc/attachment-0001.sig>