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

[Dylan Baker]

Quoting Michel Dänzer (2016-10-24 23:10:04)
> On 13/10/16 05:59 AM, Dylan Baker wrote:
> > 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 
> 
> This change broke HTML summary generation for me. Interestingly, two
> possible invocations fail slightly differently:
> 
> > ./piglit summary html summary/quick_cl results/quick_cl.20161024-baseline 
> > results/quick_cl.20161025-baseline
> usage: piglit [-h] {print-cmd,run,resume,summary} ...
> 
> positional arguments:
>   {print-cmd,run,resume,summary}
> print-cmd   Print piglit commands, one per line.
> run Run a piglit test
> resume  resume an interrupted piglit run
> summary summary generators
> 
> optional arguments:
>   -h, --helpshow this help message and exit

Yeah, there's a patch in the really big series that fixes an "except" catching
things it shouldn't and printing the help. It's probably actually the same bug
in the same place. I'll take a look.

> 
> 
> > ./piglit-summary-html.py summary/quick_cl 
> > results/quick_cl.20161024-baseline results/quick_cl.20161025-baseline
> /home/daenzer/src/piglit-git/piglit/framework/test/base.py:77: UserWarning: 
> Timeouts are not available
>   warnings.warn('Timeouts are not available')
> Traceback (most recent call last):
>   File "./piglit-summary-html.py", line 35, in 
> html([i.decode('utf-8') for i in sys.argv[1:]])
>   File "/home/daenzer/src/piglit-git/piglit/framework/exceptions.py", line 
> 51, in _inner
> func(*args, **kwargs)
>   File "/home/daenzer/src/piglit-git/piglit/framework/programs/summary.py", 
> line 117, in html
> summary.html(args.resultsFiles, args.summaryDir, args.exclude_details)
>   File "/home/daenzer/src/piglit-git/piglit/framework/summary/html_.py", line 
> 178, in html
> _make_testrun_info(results, destination, exclude)
>   File "/home/daenzer/src/piglit-git/piglit/framework/summary/html_.py", line 
> 100, in _make_testrun_info
> time=each.time_elapsed.delta,
> AttributeError: 'dict' object has no attribute 'delta'
> 
> 
> -- 
> Earthling Michel Dänzer   |   http://www.amd.com
> Libre software enthusiast | Mesa and X developer


signature.asc
Description: signature
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[Michel Dänzer]

On 13/10/16 05:59 AM, Dylan Baker wrote:
> 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 

This change broke HTML summary generation for me. Interestingly, two
possible invocations fail slightly differently:

> ./piglit summary html summary/quick_cl results/quick_cl.20161024-baseline 
> results/quick_cl.20161025-baseline
usage: piglit [-h] {print-cmd,run,resume,summary} ...

positional arguments:
  {print-cmd,run,resume,summary}
print-cmd   Print piglit commands, one per line.
run Run a piglit test
resume  resume an interrupted piglit run
summary summary generators

optional arguments:
  -h, --helpshow this help message and exit


> ./piglit-summary-html.py summary/quick_cl results/quick_cl.20161024-baseline 
> results/quick_cl.20161025-baseline
/home/daenzer/src/piglit-git/piglit/framework/test/base.py:77: UserWarning: 
Timeouts are not available
  warnings.warn('Timeouts are not available')
Traceback (most recent call last):
  File "./piglit-summary-html.py", line 35, in 
html([i.decode('utf-8') for i in sys.argv[1:]])
  File "/home/daenzer/src/piglit-git/piglit/framework/exceptions.py", line 51, 
in _inner
func(*args, **kwargs)
  File "/home/daenzer/src/piglit-git/piglit/framework/programs/summary.py", 
line 117, in html
summary.html(args.resultsFiles, args.summaryDir, args.exclude_details)
  File "/home/daenzer/src/piglit-git/piglit/framework/summary/html_.py", line 
178, in html
_make_testrun_info(results, destination, exclude)
  File "/home/daenzer/src/piglit-git/piglit/framework/summary/html_.py", line 
100, in _make_testrun_info
time=each.time_elapsed.delta,
AttributeError: 'dict' object has no attribute 'delta'


-- 
Earthling Michel Dänzer   |   http://www.amd.com
Libre software enthusiast | Mesa and X developer
___
Piglit mailing list
Piglit@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/piglit

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

[Dylan Baker]

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 
---
 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