test-webkitpy fails on chromium win
Created attachment 100053 [details] Patch
Comment on attachment 100053 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=100053&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py:37 > + # FIXME: We don't test some of the combinations on win32 because paths are returned using the Win32 "\\" conventions > + # and the tests will fail. It's unclear if there's a good solution to this. I'm confused by this statement. > Tools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py:40 > + if sys.platform != 'win32': A constant on the class or in the file would read better than checking in each place.
Created attachment 100054 [details] clarify comments per eseidel's review
Comment on attachment 100054 [details] clarify comments per eseidel's review View in context: https://bugs.webkit.org/attachment.cgi?id=100054&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py:40 > + # FIXME: We can't test some of the mac and linux code when running on > + # Windows because the filename paths returned from test_files.find() > + # have the Win32 "\\" filenames and # the code in the mac and linux ports' > + # implementation of 'relative_test_filename()' expect them to have forward slashes. > + FILES_HAVE_FORWARD_SLASHES = sys.platform != 'win32' This is more clear, but I'm still confused why os.path.sep isn't our friend here?
Comment on attachment 100054 [details] clarify comments per eseidel's review View in context: https://bugs.webkit.org/attachment.cgi?id=100054&action=review >> Tools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py:40 >> + FILES_HAVE_FORWARD_SLASHES = sys.platform != 'win32' > > This is more clear, but I'm still confused why os.path.sep isn't our friend here? Maybe it isn't worth the effort to make this platform-agnostic. I don't know. That's your judgment call to make. But it's not immediately obvious to me why it's not platform-agnostic automatically if we're using os.path.sep where we're supposed to. I don't see any '/' in these tests themselves. Should we be making this check inside assertOverridesWorked instead of in each test?
What does the failure look like?
Comment on attachment 100054 [details] clarify comments per eseidel's review I think the real bug here is that these tests should talk to a MockFileSystem. Since they aren't, they arent' unit tests and the file should be renamed _integrationtest.py appropriately.
Created attachment 100060 [details] make it clearer that there are integration tests going on
Comment on attachment 100060 [details] make it clearer that there are integration tests going on View in context: https://bugs.webkit.org/attachment.cgi?id=100060&action=review > Tools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py:50 > + def integration_test_chromium_gpu_win(self): Do integration_test methods get run?
The GPU failures (prior to the patch, obviously) looked like: ====================================================================== ERROR: test_get_chromium_gpu__on_linux (webkitpy.layout_tests.port.chromium_gpu_unittest.ChromiumGp Test) ---------------------------------------------------------------------- Traceback (most recent call last): File "d:\src\dev\src\third_party\WebKit\Tools\Scripts\webkitpy\layout_tests\port\chromium_gpu_uni test.py", line 45, in test_get_chromium_gpu__on_linux self.assertOverridesWorked('chromium-gpu-linux', 'chromium-gpu', 'linux2') File "d:\src\dev\src\third_party\WebKit\Tools\Scripts\webkitpy\layout_tests\port\chromium_gpu_uni test.py", line 91, in assertOverridesWorked self.assertTrue(path in files) File "d:\src\dev\src\third_party\WebKit\Tools\Scripts\webkitpy\layout_tests\port\chromium_gpu_uni test.py", line 91, in assertOverridesWorked self.assertTrue(path in files) File "d:\src\depot_tools\python_bin\lib\bdb.py", line 46, in trace_dispatch return self.dispatch_line(frame) File "d:\src\depot_tools\python_bin\lib\bdb.py", line 65, in dispatch_line if self.quitting: raise BdbQuit BdbQuit
(In reply to comment #9) > (From update of attachment 100060 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=100060&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py:50 > > + def integration_test_chromium_gpu_win(self): > > Do integration_test methods get run? Not by default. When you run the test files directly you can run them by specifying the -i flag, e.g.: pythom chromium_gpu_unittest.py -i I would have put a lot of money on me either having added the -i flag to test-webkitpy, or at least filed a bug and posted a patch for it, but I'm not finding it now, so I'll file another bug for it.
Filed bug 64137 for adding integration test support to test-wekbitpy. Note that I did not rename the file in this patch, just so you can actually see the diff.
Comment on attachment 100060 [details] make it clearer that there are integration tests going on OK.
Comment on attachment 100060 [details] make it clearer that there are integration tests going on Clearing flags on attachment: 100060 Committed r90607: <http://trac.webkit.org/changeset/90607>
All reviewed patches have been landed. Closing bug.