[Piglit] [PATCH v2 3/3] framework/backends/json: Don't convert to TestrunResult while updating

Dylan Baker dylan at pnwbakers.com
Tue Oct 11 19:33:00 UTC 2016


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.

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.
---
 framework/backends/json.py                       | 37 +++++------
 unittests/framework/backends/test_json.py        |  7 ++-
 unittests/framework/backends/test_json_update.py | 55 ++++++-----------
 3 files changed, 48 insertions(+), 51 deletions(-)

diff --git a/framework/backends/json.py b/framework/backends/json.py
index 7181841..ced3852 100644
--- a/framework/backends/json.py
+++ b/framework/backends/json.py
@@ -260,12 +260,12 @@ 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):
     """Set json specific metadata on a TestrunResult."""
-    results.results_version = CURRENT_JSON_VERSION
+    results['results_version'] = CURRENT_JSON_VERSION
 
 
 def _load(results_file):
@@ -275,16 +275,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):
@@ -336,20 +334,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 +379,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 +399,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/unittests/framework/backends/test_json.py b/unittests/framework/backends/test_json.py
index 056e2c1..dbf04f0 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."""
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 at 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 at test'].time.start == 0.0
-        assert result.tests['a at 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 at 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 at test']['time']['start'] == 0.0
+        assert result['tests']['a at 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 at 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 at test'].pid == [5]
+        assert result['tests']['a at test']['pid'] == [5]
 
     def test_valid(self, result):
         with open(os.path.join(os.path.dirname(__file__), 'schema',
-- 
git-series 0.8.10


More information about the Piglit mailing list