framework/backends/json: Don't convert to TestrunResult while updating

This changes the way updates are done in the backend, instead of
converting to a TestrunResult immediately, all of the transformations
are done to the JSON data in it's rawest form, ie, as dicts and lists,
and then transform to a TestrunResult (and children) after that.

This makes the loading code more robust and simpler, since it's
decoupled from the representation, making the transformations easier to
test and manage.

Part of this change is fixing the .to_json and .from_dict methods, many
of which "worked" because their shortcomings were papered over by using
json.load with a custom decoder. This patch fixes them to actually work
correctly. Despite my best attempts I couldn't decouple this work for
this patch because of the close coupling of the JSON loading code and
the class representations.

There are a number of fixups to the tests in this patch, since a number
of issues were being covered by the TestrunResult.to_json() method
filling it missing values.

Signed-off-by: Dylan Baker <dylanx.c.baker@intel.com>

2 files changed, 33 insertions, 47 deletions

diff --git a/framework/backends/json.py b/framework/backends/json.py
index 718184152..17002ed58 100644
--- a/framework/backends/json.py
+++ b/framework/backends/json.py
@@ -61,14 +61,6 @@ MINIMUM_SUPPORTED_VERSION = 7
 # The level to indent a final file
 INDENT = 4
 
-_DECODER_TABLE = {
-    'Subtests': results.Subtests,
-    'TestResult': results.TestResult,
-    'TestrunResult': results.TestrunResult,
-    'TimeAttribute': results.TimeAttribute,
-    'Totals': results.Totals,
-}
-
 
 def piglit_encoder(obj):
     """ Encoder for piglit that can transform additional classes into json
@@ -85,16 +77,6 @@ def piglit_encoder(obj):
     return obj
 
 
-def piglit_decoder(obj):
-    """Json decoder for piglit that can load TestResult objects."""
-    if isinstance(obj, dict):
-        try:
-            return _DECODER_TABLE[obj['__type__']].from_dict(obj)
-        except KeyError:
-            pass
-    return obj
-
-
 class JSONBackend(FileBackend):
     """ Piglit's native JSON backend
 
@@ -167,8 +149,7 @@ class JSONBackend(FileBackend):
                     # work.
                     try:
                         with open(test, 'r') as f:
-                            data['tests'].update(
-                                json.load(f, object_hook=piglit_decoder))
+                            data['tests'].update(json.load(f))
                     except ValueError:
                         pass
             assert data['tests']
@@ -204,8 +185,7 @@ class JSONBackend(FileBackend):
                             if os.path.isfile(test):
                                 try:
                                     with open(test, 'r') as f:
-                                        a = json.load(
-                                            f, object_hook=piglit_decoder)
+                                        a = json.load(f)
                                 except ValueError:
                                     continue
 
@@ -260,7 +240,7 @@ def load_results(filename, compression_):
     with compression.DECOMPRESSORS[compression_](filepath) as f:
         testrun = _load(f)
 
-    return _update_results(testrun, filepath)
+    return results.TestrunResult.from_dict(_update_results(testrun, filepath))
 
 
 def set_meta(results):
@@ -275,16 +255,14 @@ def _load(results_file):
 
     """
     try:
-        result = json.load(results_file, object_hook=piglit_decoder)
+        result = json.load(results_file)
     except ValueError as e:
         raise exceptions.PiglitFatalError(
             'While loading json results file: "{}",\n'
             'the following error occurred:\n{}'.format(results_file.name,
                                                        six.text_type(e)))
 
-    if isinstance(result, results.TestrunResult):
-        return result
-    return results.TestrunResult.from_dict(result, _no_totals=True)
+    return result
 
 
 def _resume(results_dir):
@@ -307,7 +285,7 @@ def _resume(results_dir):
     for file_ in os.listdir(os.path.join(results_dir, 'tests')):
         with open(os.path.join(results_dir, 'tests', file_), 'r') as f:
             try:
-                meta['tests'].update(json.load(f, object_hook=piglit_decoder))
+                meta['tests'].update(json.load(f))
             except ValueError:
                 continue
 
@@ -336,20 +314,20 @@ def _update_results(results, filepath):
             8: _update_eight_to_nine,
         }
 
-        while results.results_version < CURRENT_JSON_VERSION:
-            results = updates[results.results_version](results)
+        while results['results_version'] < CURRENT_JSON_VERSION:
+            results = updates[results['results_version']](results)
 
         return results
 
-    if results.results_version < MINIMUM_SUPPORTED_VERSION:
+    if results['results_version'] < MINIMUM_SUPPORTED_VERSION:
         raise exceptions.PiglitFatalError(
             'Unsupported version "{}", '
             'minimum supported version is "{}"'.format(
-                results.results_version, MINIMUM_SUPPORTED_VERSION))
+                results['results_version'], MINIMUM_SUPPORTED_VERSION))
 
     # If the results version is the current version there is no need to
     # update, just return the results
-    if results.results_version == CURRENT_JSON_VERSION:
+    if results['results_version'] == CURRENT_JSON_VERSION:
         return results
 
     results = loop_updates(results)
@@ -381,12 +359,15 @@ def _update_seven_to_eight(result):
     This value is used for both TestResult.time and TestrunResult.time_elapsed.
 
     """
-    for test in compat.viewvalues(result.tests):
-        test.time = results.TimeAttribute(end=test.time)
+    for test in compat.viewvalues(result['tests']):
+        test['time'] = {'start': 0.0, 'end': float(test['time']),
+                        '__type__': 'TimeAttribute'}
 
-    result.time_elapsed = results.TimeAttribute(end=result.time_elapsed)
+    result['time_elapsed'] = {'start': 0.0, 'end':
+                              float(result['time_elapsed']),
+                              '__type__': 'TimeAttribute'}
 
-    result.results_version = 8
+    result['results_version'] = 8
 
     return result
 
@@ -398,10 +379,10 @@ def _update_eight_to_nine(result):
     null rather than a single integer or null.
 
     """
-    for test in compat.viewvalues(result.tests):
-        test.pid = [test.pid]
+    for test in compat.viewvalues(result['tests']):
+        test['pid'] = [test['pid']]
 
-    result.results_version = 9
+    result['results_version'] = 9
 
     return result
 
diff --git a/framework/results.py b/framework/results.py
index f9ddcb4cd..5634df1c6 100644
--- a/framework/results.py
+++ b/framework/results.py
@@ -200,8 +200,8 @@ class TestResult(object):
             'out': self.out,
             'result': self.result,
             'returncode': self.returncode,
-            'subtests': self.subtests,
-            'time': self.time,
+            'subtests': self.subtests.to_json(),
+            'time': self.time.to_json(),
             'exception': self.exception,
             'traceback': self.traceback,
             'dmesg': self.dmesg,
@@ -225,18 +225,19 @@ class TestResult(object):
         inst = cls()
 
         for each in ['returncode', 'command', 'exception', 'environment',
-                     'time', 'traceback', 'result', 'dmesg', 'pid']:
+                     'traceback', 'dmesg', 'pid', 'result']:
             if each in dict_:
                 setattr(inst, each, dict_[each])
 
+        # Set special instances
         if 'subtests' in dict_:
-            for name, value in six.iteritems(dict_['subtests']):
-                inst.subtests[name] = value
+            inst.subtests = Subtests.from_dict(dict_['subtests'])
+        if 'time' in dict_:
+            inst.time = TimeAttribute.from_dict(dict_['time'])
 
         # out and err must be set manually to avoid replacing the setter
         if 'out' in dict_:
             inst.out = dict_['out']
-
         if 'err' in dict_:
             inst.err = dict_['err']
 
@@ -344,6 +345,7 @@ class TestrunResult(object):
         if not self.totals:
             self.calculate_group_totals()
         rep = copy.copy(self.__dict__)
+        rep['tests'] = {k: t.to_json() for k, t in six.iteritems(self.tests)}
         rep['__type__'] = 'TestrunResult'
         return rep
 
@@ -360,12 +362,15 @@ class TestrunResult(object):
         """
         res = cls()
         for name in ['name', 'uname', 'options', 'glxinfo', 'wglinfo', 'lspci',
-                     'time_elapsed', 'tests', 'totals', 'results_version',
+                     'time_elapsed', 'totals', 'results_version',
                      'clinfo']:
             value = dict_.get(name)
             if value:
                 setattr(res, name, value)
 
+        res.tests = {n: TestResult.from_dict(t)
+                     for n, t in six.iteritems(dict_['tests'])}
+
         if not res.totals and not _no_totals:
             res.calculate_group_totals()