[webkit-reviews] review granted: [Bug 137229] LayoutTestResults and ExpectedFailures should know about the interrupted flag from the json results file : [Attachment 238881] Fixes style issues

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Mon Sep 29 14:50:34 PDT 2014


Alexey Proskuryakov <ap at webkit.org> has granted Jake Nielsen
<jake.nielsen.webkit at gmail.com>'s request for review:
Bug 137229: LayoutTestResults and ExpectedFailures should know about the
interrupted flag from the json results file
https://bugs.webkit.org/show_bug.cgi?id=137229

Attachment 238881: Fixes style issues
https://bugs.webkit.org/attachment.cgi?id=238881&action=review

------- Additional Comments from Alexey Proskuryakov <ap at webkit.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=238881&action=review


> Tools/Scripts/webkitpy/common/net/layouttestresults.py:66
> +    def interrupted(self):
> +	   return self._result_dict["interrupted"]
>  
>      def test_results(self):
> -	   return self._test_results
> +	   return self._result_dict['tests']

It may be nicer to split the dictionary into separate member variables when
constructing the LayoutTestResults object. There doesn't seem to be any reason
to keep them together.

> Tools/Scripts/webkitpy/common/net/resultsjsonparser.py:156
> +	   # Because we want to keep all of the metadata stored in the json
dict, but we want already-processed test results,

I'd state this slightly differently:

# We want to keep all of the metadata stored in the json dictionary as is, but
we want already-processed test results.

As long as comment explains why something is done, the reader is supposed to
read how the code achieves this.


More information about the webkit-reviews mailing list