[Webkit-unassigned] [Bug 52267] Change results.json format to the one used by unexpected_results.json

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=52267


Tony Chang <tony at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #83778|review?                     |review+
               Flag|                            |




--- Comment #12 from Tony Chang <tony at chromium.org>  2011-02-25 10:07:04 PST ---
(From update of attachment 83778)
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.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.