[webkit-reviews] review denied: [Bug 67253] NRWT should show the size of the pixel image delta in results.html : [Attachment 108554] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Fri Sep 23 17:29:43 PDT 2011


Dirk Pranke <dpranke at chromium.org> has denied Simon Fraser (smfr)
<simon.fraser at apple.com>'s request for review:
Bug 67253: NRWT should show the size of the pixel image delta in results.html
https://bugs.webkit.org/show_bug.cgi?id=67253

Attachment 108554: Patch
https://bugs.webkit.org/attachment.cgi?id=108554&action=review

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
View in context: https://bugs.webkit.org/attachment.cgi?id=108554&action=review


R-'ing this for now until we can add an integration test and I get a chance to
run through the patch by hand. You probably also need to update diff_image in
port/test.py.

>> Tools/Scripts/webkitpy/layout_tests/models/test_failures.py:171
>> +	    self.diff_percent = diff_percent
> 
> I feel like I've tried to add non-static data to these before and others have
gotten sad.  The test_failures.py design is bad/broken.  We'll have to consult
Ojan/Dirk here for suggestions.

I think this should mostly work. We may have to double-check the
serialization/pickling code to make sure it transfers across.

>> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:184
>> +	    diff_percent = 0;
> 
> multiple statements on one line (semicolon)  [pep8/E702] [5]

Python doesn't use semicolons :)

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:583
> +		   return [True, 0]

We should probably have at least one integration test that returns a non-zero
diff value. I will look into what it would take to make this work.


More information about the webkit-reviews mailing list