[webkit-reviews] review denied: [Bug 60605] [NRWT] Reftests should run even when pixel tests are disabled. : [Attachment 115206] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied Tony Chang <tony at chromium.org>'s
request for review:
Bug 60605: [NRWT] Reftests should run even when pixel tests are disabled.
https://bugs.webkit.org/show_bug.cgi?id=60605

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

------- Additional Comments from Dirk Pranke <dpranke at chromium.org>
Almost there. This feels pretty clean to me, now.

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


> Tools/Scripts/webkitpy/layout_tests/port/chromium.py:402
> +    def __init__(self, port, worker_number, pixel_tests=False):

Nit: I think you should remove the default value for pixel_tests.

> Tools/Scripts/webkitpy/layout_tests/port/dryrun.py:103
> +	   return DryrunDriver(self, worker_number, pixel_tests=False)

I think you can just delete this method and implement _driver_class(). I'm
pretty sure this change would cause pixel tests to be ignored, which would be
wrong.

> Tools/Scripts/webkitpy/layout_tests/port/webkit.py:443
> +    def __init__(self, port, worker_number, pixel_tests=False):

Nit: again, remove the default value.

> Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_integrationtest.py:155
> +	       TestDriver.__init__(self, port, worker_number,
pixel_tests=False)

Nit: pass in pixel_tests instead, maybe?