[webkit-reviews] review denied: [Bug 67269] Provides a stand alone CSSWG reftest runner which can be used internally. : [Attachment 105948] Patch

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Thu Sep 1 18:15:57 PDT 2011


Hayato Ito <hayato at chromium.org> has denied Ai Makabi <makabi at google.com>'s
request for review:
Bug 67269: Provides a stand alone CSSWG reftest runner which can be used
internally.
https://bugs.webkit.org/show_bug.cgi?id=67269

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

------- Additional Comments from Hayato Ito <hayato at chromium.org>
Hi Ai-san. Thank you for addressing comments!
Could you address some minor comments again? Sorry for the back and forth.

I don't think we need a comprehensive unittest for this type of script because
this test runner can be used only internally.
So we could land this patch after you addressing minor comments and Hamaji-san
reviews and give r+ to a next patch. 


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


> Tools/Scripts/webkitpy/layout_tests/reftests/test_csswg_reftest.py:72
> +	       return

Could you remove this line? We don't need 'return' here.

> Tools/Scripts/webkitpy/layout_tests/reftests/test_csswg_reftest.py:101
> +		   self.test_files[test_file].crash = True

You might want to replace L100-101 with one line as:
    self.test_files[test_file].crash = output.crash

> Tools/Scripts/webkitpy/layout_tests/reftests/test_csswg_reftest.py:137
> +	       if self.test_files[test_file].crash:

Could you print a message to notify a crash?
like:
   print "test_file: %s is crashed." % test_file


More information about the webkit-reviews mailing list