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 they're short comings 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.ba...@intel.com>
---
 framework/backends/json.py                       | 61 +++++------------
 framework/results.py                             | 19 +++--
 unittests/framework/backends/test_json.py        | 26 +------
 unittests/framework/backends/test_json_update.py | 55 ++++++---------
 unittests/framework/test_results.py              | 19 +----
 5 files changed, 69 insertions(+), 111 deletions(-)

diff --git a/framework/backends/json.py b/framework/backends/json.py
index 7181841..17002ed 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 f9ddcb4..5634df1 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()
 
diff --git a/unittests/framework/backends/test_json.py 
b/unittests/framework/backends/test_json.py
index 056e2c1..da1015c 100644
--- a/unittests/framework/backends/test_json.py
+++ b/unittests/framework/backends/test_json.py
@@ -250,6 +250,13 @@ class TestLoadResults(object):
             f.write(json.dumps(shared.JSON))
         backends.json.load_results(six.text_type(p), 'none')
 
+    def test_inst(self, tmpdir):
+        p = tmpdir.join('my file')
+        with p.open('w') as f:
+            f.write(json.dumps(shared.JSON))
+        assert isinstance(backends.json.load_results(six.text_type(p), 'none'),
+                          results.TestrunResult)
+
 
 class TestLoad(object):
     """Tests for the _load function."""
@@ -261,22 +268,3 @@ class TestLoad(object):
         with p.open('r') as f:
             with pytest.raises(exceptions.PiglitFatalError):
                 backends.json._load(f)
-
-
-class TestPiglitDecooder(object):
-    """Tests for the piglit_decoder function."""
-
-    def test_result(self):
-        """backends.json.piglit_decoder: turns results into TestResults."""
-        test = json.loads(
-            '{"foo": {"result": "pass", "__type__": "TestResult"}}',
-            object_hook=backends.json.piglit_decoder)
-        assert isinstance(test['foo'], results.TestResult)
-
-    def test_old_result(self):
-        """backends.json.piglit_decoder: does not turn old results into
-        TestResults.
-        """
-        test = json.loads('{"foo": {"result": "pass"}}',
-                          object_hook=backends.json.piglit_decoder)
-        assert isinstance(test['foo'], dict)
diff --git a/unittests/framework/backends/test_json_update.py 
b/unittests/framework/backends/test_json_update.py
index db45c1b..c8e3ee6 100644
--- a/unittests/framework/backends/test_json_update.py
+++ b/unittests/framework/backends/test_json_update.py
@@ -37,7 +37,6 @@ import jsonschema
 import pytest
 
 from framework import backends
-from framework import results
 
 # pylint: disable=protected-access,no-self-use
 
@@ -67,13 +66,13 @@ class TestV7toV8(object):
             "test_count": 0,
             "exclude_tests": [],
             "exclude_filter": [],
-            "env": {
-                "lspci": "stuff",
-                "uname": "more stuff",
-                "glxinfo": "and stuff",
-                "wglinfo": "stuff"
-            }
+            "env": {},
         },
+        "lspci": "stuff",
+        "uname": "more stuff",
+        "glxinfo": "and stuff",
+        "wglinfo": "stuff",
+        "clinfo": "stuff",
         "tests": {
             'a@test': {
                 'time': 1.2,
@@ -94,6 +93,7 @@ class TestV7toV8(object):
             }
         },
         "time_elapsed": 1.2,
+        '__type__': 'TestrunResult',
     }
 
     @pytest.fixture
@@ -107,27 +107,15 @@ class TestV7toV8(object):
         """backends.json.update_results (7 -> 8): test time is stored as start
         and end.
         """
-        assert result.tests['a@test'].time.start == 0.0
-        assert result.tests['a@test'].time.end == 1.2
-
-    def test_time_inst(self, result):
-        """backends.json.update_results (7 -> 8): test time is a TimeAttribute
-        instance.
-        """
-        assert isinstance(result.tests['a@test'].time, results.TimeAttribute)
-
-    def test_time_elapsed_inst(self, result):
-        """backends.json.update_results (7 -> 8): total time is stored as
-        TimeAttribute.
-        """
-        assert isinstance(result.time_elapsed, results.TimeAttribute)
+        assert result['tests']['a@test']['time']['start'] == 0.0
+        assert result['tests']['a@test']['time']['end'] == 1.2
 
     def test_time_elapsed(self, result):
         """backends.json.update_results (7 -> 8): total time is stored as start
         and end.
         """
-        assert result.time_elapsed.start == 0.0
-        assert result.time_elapsed.end == 1.2
+        assert result['time_elapsed']['start'] == 0.0
+        assert result['time_elapsed']['end'] == 1.2
 
     def test_valid(self, result):
         with open(os.path.join(os.path.dirname(__file__), 'schema',
@@ -143,7 +131,7 @@ class TestV8toV9(object):
     """Tests for Version 8 to version 9."""
 
     data = {
-        "results_version": 9,
+        "results_version": 8,
         "name": "test",
         "options": {
             "profile": ['quick'],
@@ -157,16 +145,16 @@ class TestV8toV9(object):
             "test_count": 0,
             "exclude_tests": [],
             "exclude_filter": [],
-            "env": {
-                "lspci": "stuff",
-                "uname": "more stuff",
-                "glxinfo": "and stuff",
-                "wglinfo": "stuff"
-            }
+            "env": {},
         },
+        "lspci": "stuff",
+        "uname": "more stuff",
+        "glxinfo": "and stuff",
+        "wglinfo": "stuff",
+        "clinfo": "stuff",
         "tests": {
             'a@test': {
-                "time_elapsed": {
+                "time": {
                     'start': 1.2,
                     'end': 1.8,
                     '__type__': 'TimeAttribute'
@@ -191,7 +179,8 @@ class TestV8toV9(object):
             'start': 1.2,
             'end': 1.8,
             '__type__': 'TimeAttribute'
-        }
+        },
+        '__type__': 'TestrunResult',
     }
 
     @pytest.fixture
@@ -202,7 +191,7 @@ class TestV8toV9(object):
             return backends.json._update_eight_to_nine(backends.json._load(f))
 
     def test_pid(self, result):
-        assert result.tests['a@test'].pid == [5]
+        assert result['tests']['a@test']['pid'] == [5]
 
     def test_valid(self, result):
         with open(os.path.join(os.path.dirname(__file__), 'schema',
diff --git a/unittests/framework/test_results.py 
b/unittests/framework/test_results.py
index b2db870..9fce9d5 100644
--- a/unittests/framework/test_results.py
+++ b/unittests/framework/test_results.py
@@ -113,7 +113,7 @@ class TestTestResult(object):
                     'result': 'crash',
                     'exception': 'an exception',
                     'dmesg': 'this is dmesg',
-                    'pid': 1934,
+                    'pid': [1934],
                 }
 
                 cls.test = results.TestResult.from_dict(cls.dict)
@@ -142,8 +142,8 @@ class TestTestResult(object):
 
                 """
                 # pylint: disable=unsubscriptable-object
-                assert self.test.time['start'] == self.dict['time']['start']
-                assert self.test.time['end'] == self.dict['time']['end']
+                assert self.test.time.start == self.dict['time']['start']
+                assert self.test.time.end == self.dict['time']['end']
 
             def test_environment(self):
                 """sets environment properly."""
@@ -234,13 +234,6 @@ class TestTestResult(object):
             cls.test = test
             cls.json = test.to_json()
 
-            # the TimeAttribute needs to be dict-ified as well. There isn't
-            # really a good way to do this that doesn't introduce a lot of
-            # complexity, such as:
-            # json.loads(json.dumps(test, default=piglit_encoder),
-            #            object_hook=piglit_decoder)
-            cls.json['time'] = cls.json['time'].to_json()
-
         def test_returncode(self):
             """results.TestResult.to_json: sets the returncode correctly"""
             assert self.test.returncode == self.json['returncode']
@@ -269,7 +262,9 @@ class TestTestResult(object):
 
         def test_subtests(self):
             """results.TestResult.to_json: sets the subtests correctly"""
-            assert self.test.subtests == self.json['subtests']
+            assert self.test.subtests['a'] == self.json['subtests']['a']
+            assert self.test.subtests['b'] == self.json['subtests']['b']
+            assert self.json['subtests']['__type__']
 
         def test_type(self):
             """results.TestResult.to_json: adds the __type__ hint"""
@@ -515,7 +510,7 @@ class TestTestrunResult(object):
 
         def test_tests(self):
             """tests is properly encoded."""
-            assert self.test['tests']['a test'].result == 'pass'
+            assert self.test['tests']['a test']['result'] == 'pass'
 
         def test_type(self):
             """__type__ is added."""
-- 
git-series 0.8.10
_______________________________________________
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

Reply via email to