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>

3 files changed, 36 insertions, 64 deletions

diff --git a/unittests/framework/backends/test_json.py b/unittests/framework/backends/test_json.py
index 056e2c16c..da1015c9f 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 db45c1b1e..c8e3ee6a9 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 b2db8703c..9fce9d5e5 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."""