[webkit-reviews] review denied: [Bug 72841] NRWT: option --skip-pixel-test-if-no-baseline support on DRT : [Attachment 130591] Patch

[bugzilla-daemon at webkit.org]

Dirk Pranke <dpranke at chromium.org> has denied Nandor Huszka
<Huszka.Nandor at stud.u-szeged.hu>'s request for review:
Bug 72841: NRWT: option --skip-pixel-test-if-no-baseline support on DRT
https://bugs.webkit.org/show_bug.cgi?id=72841

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

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


Technology in new-run-webkit-tests has evolved over the past few months, and I
think we should use a different approach for this patch. We now have the
ability to run different tests with different command line flags (using the
code in DriverProxy in webkitpy/layout_tests/port/driver.py), and given that we
know whether we should run pixel tests or not in worker.py (as your change
indicates), we should combine pass test_input.should_run_pixel_test to
driver_input and pass run_pixel_test to driver_input and just use a different
DRT instance to run the test in this case. 

If we do that, then you don't need to modify DRT at all and the logic of
needing pixel tests or not stays where it should be, in worker.py and
single_test_runner.py.

Let me know if that's enough for you to work on, or if you would like me to
work up a patch to illustrate this.

Thanks!

> Tools/Scripts/webkitpy/layout_tests/controllers/worker.py:126
> +	       if (driver_name == "WebKitTestRunner" or driver_name ==
"DumpRenderTree") and self._port.get_option('skip_pixel_test_if_no_baseline')
and self._port.get_option('pixel_tests'):

drivers are basically always either named 'WebKitTestRunner' or
'DumpRenderTree'. You can remove these checks and just check the
port.get_option(X) values.