[Piglit] [Patch V2 3/8] framework/status.py: Replace subclasses with constants

Dylan Baker baker.dylan.c at gmail.com
Mon Mar 10 18:25:29 PDT 2014


This patch replaces the flyweight children of Status with a single class
that is initialized with arguments, and provides constants. These
constants are mostly immutable in the cleanest way possible (they're not
if you really want to go after them, but short of Cython or a native C
extension it's impossible to make a truly immutable object in python).

What this provides for us is a simple, clean interface, a massive
reduction in code, and better compatibility for python 3 (the previous
implementation relied on a behavior of __slots__ that changed in python
3), and minimal API changes.

v2: - Replace patch that used abc.ABCMeta with this constant aproach

Signed-off-by: Dylan Baker <baker.dylan.c at gmail.com>
---
 framework/status.py  | 152 +++++++++++++++++++++++----------------------------
 framework/summary.py |  20 +++----
 2 files changed, 77 insertions(+), 95 deletions(-)

diff --git a/framework/status.py b/framework/status.py
index 2c6b692..5170847 100644
--- a/framework/status.py
+++ b/framework/status.py
@@ -62,6 +62,14 @@ The formula for determining fixes is:
 
 """
 
+__all__ = ['NOTRUN',
+           'PASS',
+           'FAIL',
+           'WARN',
+           'CRASH',
+           'DMESG_WARN',
+           'DMESG_FAIL',
+           'SKIP']
 
 def status_lookup(status):
     """ Provided a string return a status object instance
@@ -71,17 +79,17 @@ def status_lookup(status):
     does not correspond to a key it will raise an exception
 
     """
-    status_dict = {'skip': Skip,
-                   'pass': Pass,
-                   'warn': Warn,
-                   'fail': Fail,
-                   'crash': Crash,
-                   'dmesg-warn': DmesgWarn,
-                   'dmesg-fail': DmesgFail,
-                   'notrun': NotRun}
+    status_dict = {'skip': SKIP,
+                   'pass': PASS,
+                   'warn': WARN,
+                   'fail': FAIL,
+                   'crash': CRASH,
+                   'dmesg-warn': DMESG_WARN,
+                   'dmesg-fail': DMESG_FAIL,
+                   'notrun': NOTRUN}
 
     try:
-        return status_dict[status]()
+        return status_dict[status]
     except KeyError:
         # Raise a StatusException rather than a key error
         raise StatusException
@@ -101,26 +109,50 @@ class StatusException(LookupError):
 
 
 class Status(object):
-    """
-    A simple class for representing the output values of tests.
-
-    This class implements a flyweight pattern, limiting the memory footprint of
-    initializing thousands of these objects.
-
-    This is a base class, and should not be directly called. Instead a child
-    class should be created and called, there are many provided in this module.
+    """ A simple class for representing the output values of tests.
+
+    This class creatse the necessary magic values to use python's rich
+    comparison methods. This allows two objects to be compared using common
+    operators like <. >, ==, etc. It also alows them to be looked up in
+    containers using ``for x in []`` syntax.
+
+    This class is meant to be immutable, it (ab)uses two tools to provide this
+    psuedo-immutability: the property decorator, and the __slots__ attribute to
+    1: make the three attributes getters, therefor unwritable, and 2: make
+    adding additional attributes impossible
+
+    Arguments:
+    name -- the name of the status
+    value -- an int used to sort statuses from best to worst (0 -> inf)
+    fraction -- a tuple with two ints representing
+                [0]: 1 if the status is 'passing', else 0
+                [1]: 1 if the status counts toward the total number of tests,
+                     else 0
 
     """
-    # Using __slots__ allows us to implement the flyweight pattern, limiting
-    # the memory consumed for creating tens of thousands of these objects.
-    __slots__ = ['name', 'value', 'fraction']
-
-    name = None
-    value = None
-    fraction = (0, 1)
-
-    def __init__(self):
-        raise NotImplementedError
+    __slots__ = ['__name', '__value', '__fraction']
+
+    def __init__(self, name, value, fraction=(0, 1)):
+        # The object is immutable, so calling self.foo = foo will raise a
+        # TypeError. Using setattr from the parrent object works around this.
+        self.__name = name
+        self.__value = value
+        self.__fraction = fraction
+
+    @property
+    def name(self):
+        """ Return the value of name """
+        return self.__name
+
+    @property
+    def value(self):
+        """ Return the sorting value """
+        return self.__value
+
+    @property
+    def fraction(self):
+        """ Return the totals of the test as a tuple: (<pass>. <total>) """
+        return self.__fraction
 
     def __repr__(self):
         return self.name
@@ -153,68 +185,18 @@ class Status(object):
         return self.value
 
 
-class NotRun(Status):
-    name = 'Not Run'
-    value = 0
-    fraction = (0, 0)
-
-    def __init__(self):
-        pass
-
-
-class Skip(Status):
-    name = 'skip'
-    value = 5
-    fraction = (0, 0)
-
-    def __init__(self):
-        pass
-
-
-class Pass(Status):
-    name = 'pass'
-    value = 10
-    fraction = (1, 1)
-
-    def __init__(self):
-        pass
-
-
-class DmesgWarn(Status):
-    name = 'dmesg-warn'
-    value = 20
-
-    def __init__(self):
-        pass
-
-
-class Warn(Status):
-    name = 'warn'
-    value = 25
-
-    def __init__(self):
-        pass
-
-
-class DmesgFail(Status):
-    name = 'dmesg-fail'
-    value = 30
+NOTRUN = Status('Not Run', 0, (0, 0))
 
-    def __init__(self):
-        pass
+SKIP = Status('skip', 10, (0, 0))
 
+PASS = Status('pass', 20, (1, 1))
 
-class Fail(Status):
-    name = 'fail'
-    value = 35
+DMESG_WARN = Status('dmesg-warn', 30)
 
-    def __init__(self):
-        pass
+WARN = Status('warn', 40)
 
+DMESG_FAIL = Status('dmesg-fail', 50)
 
-class Crash(Status):
-    name = 'crash'
-    value = 40
+FAIL = Status('fail', 60)
 
-    def __init__(self):
-        pass
+CRASH = Status('crash', 70)
diff --git a/framework/summary.py b/framework/summary.py
index 2fc16ce..51b5463 100644
--- a/framework/summary.py
+++ b/framework/summary.py
@@ -194,7 +194,7 @@ class HTMLIndex(list):
         HTML summary file.
         """
         # "Not Run" is not a valid css class replace it with skip
-        if isinstance(css, so.NotRun):
+        if css == so.NOTRUN:
             css = 'skip'
 
         self.append({'type': "groupResult",
@@ -263,7 +263,7 @@ class Summary:
             """ Helper for updating the fractions and status lists """
             fraction[test] = tuple(
                 [sum(i) for i in zip(fraction[test], result.fraction)])
-            if result != so.Skip() and status[test] < result:
+            if result != so.SKIP and status[test] < result:
                 status[test] = result
 
         for results in self.results:
@@ -274,7 +274,7 @@ class Summary:
             # a default value instead of a key error.
             # This default key must be callable
             self.fractions[results.name] = collections.defaultdict(lambda: (0, 0))
-            self.status[results.name] = collections.defaultdict(so.NotRun)
+            self.status[results.name] = collections.defaultdict(lambda: so.NOTRUN)
 
             # short names
             fraction = self.fractions[results.name]
@@ -308,24 +308,24 @@ class Summary:
                 try:
                     status.append(each.tests[test]['result'])
                 except KeyError:
-                    status.append(so.NotRun())
+                    status.append(so.NOTRUN)
 
             # Problems include: warn, dmesg-warn, fail, dmesg-fail, and crash.
             # Skip does not go on this page, it has the 'skipped' page
-            if max(status) > so.Pass():
+            if max(status) > so.PASS:
                 self.tests['problems'].add(test)
 
             # Find all tests with a status of skip
-            if so.Skip() in status:
+            if so.SKIP in status:
                 self.tests['skipped'].add(test)
 
             # find fixes, regressions, and changes
             for i in xrange(len(status) - 1):
                 first = status[i]
                 last = status[i + 1]
-                if max(first, so.Pass()) < last:
+                if max(first, so.PASS) < last:
                     self.tests['regressions'].add(test)
-                if first > max(last, so.Pass()):
+                if first > max(last, so.PASS):
                     self.tests['fixes'].add(test)
                 # Changes cannot be added in the fixes and regressions passes
                 # becasue NotRun is a change, but not a regression or fix
@@ -463,12 +463,12 @@ class Summary:
             if diff:
                 for test in self.tests['changes']:
                     print "%(test)s: %(statuses)s" % {'test': test, 'statuses':
-                          ' '.join([str(i.tests.get(test, {'result': so.Skip()})
+                          ' '.join([str(i.tests.get(test, {'result': so.SKIP})
                                     ['result']) for i in self.results])}
             else:
                 for test in self.tests['all']:
                     print "%(test)s: %(statuses)s" % {'test': test, 'statuses':
-                          ' '.join([str(i.tests.get(test, {'result': so.Skip()})
+                          ' '.join([str(i.tests.get(test, {'result': so.SKIP})
                                     ['result']) for i in self.results])}
 
         # Print the summary
-- 
1.9.0



More information about the Piglit mailing list