Bug 52267 – Change results.json format to the one used by unexpected_results.json

• Status: RESOLVED
• Resolution: FIXED
• Component: Tools / Tests

[Mihai Parparita]

Right now results.json uses a JSON-like format that's meant for the flakiness dashboard (it has a custom compression scheme, wraps the JSON in an ADD_RESULT method, etc.).

Since we don't actually need it for the flakiness dashboard anymore (it uses incremental_results.json), we should repurpose results.json to have a format closed to unexpected_results.json (which is a straightforward mapping from test name to expected and actual result). That would then make it easier to programatically parse the results of a NRWT run (the way webkitpy.common.net.layouttestresults.LayoutTestResults parses results.html for old-RWT - we should make that class handle either result type).

[Eric Seidel (no email)]

sgtm.

[Dirk Pranke]

This sounds reasonable. Who do you imagine parsing the results files? Do the webkit.org bots parse the results files directly?

[Mihai Parparita]

Besides the various rebaselining tools (which should let you pick amongst failing tests), there are bunch of webkit-patch commands that use webkitpy.common.net.layouttestresults.LayoutTestResults (failure-reason, find-flaky-tests, etc.). Right now they don't work with bots that use NRWT, but if we output an easily parseable results.json then it should be reasonable to load it into the Python LayoutTestResults representation too.

[Ojan Vafai]

Eventually, I'd also like to see results.html pull in results.json and use that to generate that page from JS. That would let us make that page a richer experience.

[Dirk Pranke]

I have on my TODO list somewhere the task to unify results.html with the html generated by ORWT, so it seems reasonable to do that and pull in results.json at the same time.

[Eric Seidel (no email)]

(In reply to comment #4)
> Eventually, I'd also like to see results.html pull in results.json and use that to generate that page from JS. That would let us make that page a richer experience.

I am *strong* in favor of this.  If for no other reason than it separates model and view.  It would also make it trivial to unify the results.html experience between ORWT and NRWT.

[Mihai Parparita]

(In reply to comment #6)
> I am *strong* in favor of this.  If for no other reason than it separates model and view.  It would also make it trivial to unify the results.html experience between ORWT and NRWT.

+1. Is there a bug filed?

Dirk, mind if I work on this bug in the meantime?

[Dirk Pranke]

Not at all. Feel free to do the other thing as well ;)

[Dirk Pranke]

I think bug 37736 is Maciej's original request to unify the html output. There isn't a bug to have that file pull results.json, obviously.

[Ojan Vafai]

*** Bug 55116 has been marked as a duplicate of this bug. ***

[Ojan Vafai]

Created attachment 83778 [details]
Patch

[Tony Chang]

Comment on attachment 83778 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83778&action=review

Looks fine to me, although Mihai is more familiar with this code so maybe he wants to take a look too.

> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:174
> +    def generator_full_results_file(self):

Nit: Maybe generate_full_results_file to match generate_json_output?

> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:177
> +        # For now we only include the times as this is only used for treemaps and
> +        # expected/actual don't make test for gtests.

Nit: don't make sense for gtests?

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:98
> +    test_timings_map = dict((test_tuple.filename, test_tuple.test_run_time) for test_tuple in test_timings)

Nit: test_tuple -> test_result?

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:115
>      for k, v in TestExpectationsFile.EXPECTATIONS.iteritems():
>          keywords[v] = k.upper()
>  
> +    for k, v in TestExpectationsFile.MODIFIERS.iteritems():
> +        keywords[v] = k.upper()

Nit: k -> modifier? v-> code_string?  Even key, value would be better.

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:151
> +            time_seconds = test_timings_map[filename]
> +            tests[test]['time'] = int(1000 * time_seconds)

Can we name the dict key time_ms to make the units clear?

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:815
> +    def _dump_summarized_result(self, filename, results):

Nit: docstring (mainly, it's not clear what results is).  Also, should there be a unittest for this?

> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:820
> +        # Compact the results since we'll be uploading this to the test-results server.
> +        # This shrinks the file size by ~20%.
> +        # actual --> a

In a future patch, maybe we can just zip the file.  I bet you get a > 20% savings.

[Ojan Vafai]

Comment on attachment 83778 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=83778&action=review

>> Tools/Scripts/webkitpy/layout_tests/layout_package/json_results_generator.py:174
>> +    def generator_full_results_file(self):
> 
> Nit: Maybe generate_full_results_file to match generate_json_output?

whoops. just a typo.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:115
>> +        keywords[v] = k.upper()
> 
> Nit: k -> modifier? v-> code_string?  Even key, value would be better.

Sorry. Was just mimicking the code above. Will fix both.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:151
>> +            tests[test]['time'] = int(1000 * time_seconds)
> 
> Can we name the dict key time_ms to make the units clear?

yes.

>> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:820
>> +        # actual --> a
> 
> In a future patch, maybe we can just zip the file.  I bet you get a > 20% savings.

The downside is that we'd have to unzip it on the receiving end since we're ultimately serving up a JS file to the browser.

[Ojan Vafai]

Committed r79837: <http://trac.webkit.org/changeset/79837>

[WebKit Review Bot]

http://trac.webkit.org/changeset/79837 might have broken SnowLeopard Intel Release (Tests)

[Dirk Pranke]

(In reply to comment #13)
> >> Tools/Scripts/webkitpy/layout_tests/layout_package/test_runner.py:820
> >> +        # actual --> a
> > 
> > In a future patch, maybe we can just zip the file.  I bet you get a > 20% savings.
> 
> The downside is that we'd have to unzip it on the receiving end since we're ultimately serving up a JS file to the browser.

Can we gzip the file and then just respond to the raw script URL with the gzipped content?