[Webkit-unassigned] [Bug 70484] [WK2] add flag to only check pixel results if png files exist

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=70484


Dirk Pranke <dpranke at chromium.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #117175|review?, commit-queue?      |review-, commit-queue-
               Flag|                            |




--- Comment #24 from Dirk Pranke <dpranke at chromium.org>  2011-11-30 12:36:45 PST ---
(From update of attachment 117175)
View in context: https://bugs.webkit.org/attachment.cgi?id=117175&action=review

> Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py:203
> +        self.assertEquals(png_list, self.png_list)

Okay, inlining the function made it clear that we weren't actually testing anything interesting by testing just that skip_pixel_test() returns the correct value. Further, writing tests in webkit_unittest that are so heavily dependent on test.py indicated that there was something wrong. I'd just delete this whole routine.

What we need to test is that the actual change in worker.py is working correctly. The best way to do that is to is to add another test to run_webkit_tests_integrationtest.py with the correct flags. See something like test_repeat_each() for an example of how to check that we only ran the tests you expected to run.

Of course, the integration tests all run using the TestPort, not the WebKitPort, and so they won't be able to check the restriction on this only working with WebKitTestRunner. We can either get rid of the generic restriction on this flag (in line 67 of run_webkit_test.py), or we can add a test-wk2 port implementation to port/test.py and override port.driver_name() accordingly.

I would probably vote for just removing the check in run_webkit_tests.py; I think you're being overly cautious since the command line flag is kind of obscure, you're probably the only one to use it, and you're planning to add this to DRT as well, but if you would rather be safe than sorry, adding the test-wk2 flag is fine as well.

Sorry for the back-and-forth on this :(.

The patch looks fine otherwise.

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.