http://build.webkit.org/builders/Chromium%20Win%20Release%20%28Tests%29/builds/25106/steps/webkitpy-test/logs/stdio Looks like it's been failing since it was added in http://trac.webkit.org/changeset/113037 . Should we skip this test on Windows?
Oh sorry about that! Yeah I guess it's safe to not run this test on non-linux platforms.
Created attachment 135618 [details] Patch
Comment on attachment 135618 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135618&action=review > Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py:72 > + if sys.platform != "linux2": What about linux3 (I think newer kernels show up as linux3)? I normally just do sys.platform.startswith("linux").
Committed r113210: <http://trac.webkit.org/changeset/113210>
(In reply to comment #3) > (From update of attachment 135618 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=135618&action=review > > > Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py:72 > > + if sys.platform != "linux2": > > What about linux3 (I think newer kernels show up as linux3)? I normally just do sys.platform.startswith("linux"). Oh I didn't know about that linux3 :) I landed the modified patch. Thanks!
I'm generally not a fan of skipping unit tests on different platforms; if this test is failing on windows, it suggests that it's either not correctly written or it depends on something in the environment or operating system. If it's the latter, we should probably rename this to integration_test_get_crash_log() to be clearer ...
(In reply to comment #6) > I'm generally not a fan of skipping unit tests on different platforms; if this test is failing on windows, it suggests that it's either not correctly written or it depends on something in the environment or operating system. If it's the latter, we should probably rename this to integration_test_get_crash_log() to be clearer ... Indeed I was too quick skipping this on non-linux, the mock crash log used in the test is using %(core_directory)s/core-pid_%%p-_-process_%%e instead of building the path with os.path.join(). I'll send a new patch.
Created attachment 135771 [details] Patch
Comment on attachment 135771 [details] Patch If GtkPort._get_crash_log used FileSystem.join and you used MockFileSystem in the test, you would always get / when running the test since MockFileSystem always uses / as the separator. This seems fine too.
Comment on attachment 135771 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=135771&action=review > Tools/Scripts/webkitpy/layout_tests/port/gtk_unittest.py:60 > + core_pattern = os.path.join(core_directory, "core-pid_%p-_-process_%e") This should probably be port._filesystem.join() (and moved below the self.make_port() call), else you'll get diffs on windows from backslashes in the path, I expect.
Comment on attachment 135771 [details] Patch Clearing flags on attachment: 135771 Committed r113339: <http://trac.webkit.org/changeset/113339>
All reviewed patches have been landed. Closing bug.