Bug 116698

Created attachment 202756 [details] Screenshot of ORWT output for same build

Created attachment 203076 [details] Patch

Delaying processing of this patch until I run local tests on my Windows builder.

Patch seems to work properly. There were originally 336 failures (with 258 of them coming from the platform/mac folder). Now there are 78 failures.

Comment on attachment 203076 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203076&action=review > Tools/Scripts/webkitpy/port/win.py:144 > + return dirs_to_skip There's too much duplicated code in these routines, I think. What I would probably try to do is change the implementation of _tests_for_other_platforms() in base.py to call a new test_search_path() in base.py (like you have here), define test_search_path() in base.py as calling baseline_search_path() (to get the same logic by default), and then override test_search_path() in win.py to call the super method, but filter out the mac ports. Does that make sense?

Created attachment 203094 [details] Patch

Comment on attachment 203094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203094&action=review > Tools/Scripts/webkitpy/port/win.py:124 > + # Remove the mac tests, which we never want to execute > + regex = re.compile('mac.*') We don't need to use a regex for this. Also, this regex is going to match anything that happens to start with "mac"; e.g. maclaine. > Tools/Scripts/webkitpy/port/win.py:125 > + test_fallback_names = [x for x in self.baseline_search_path() if not regex.match(x)] Please don't use one letter variable like x. spell out path. We can do path.startswith('mac/') instead.

Comment on attachment 203094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203094&action=review >> Tools/Scripts/webkitpy/port/win.py:125 >> + test_fallback_names = [x for x in self.baseline_search_path() if not regex.match(x)] > > Please don't use one letter variable like x. spell out path. We can do path.startswith('mac/') instead. I was also going to suggest x.startswith('mac'). You can't use 'mac/', since the slash isn't in the path. However, since baseline_search_path is just a list of dirs, I don't think you need to worry about 'maclaine' or other likely conflicts.

Comment on attachment 203094 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=203094&action=review >>> Tools/Scripts/webkitpy/port/win.py:125 >>> + test_fallback_names = [x for x in self.baseline_search_path() if not regex.match(x)] >> >> Please don't use one letter variable like x. spell out path. We can do path.startswith('mac/') instead. > > I was also going to suggest x.startswith('mac'). You can't use 'mac/', since the slash isn't in the path. However, since baseline_search_path is just a list of dirs, I don't think you need to worry about 'maclaine' or other likely conflicts. I see.

(In reply to comment #7) > (From update of attachment 203094 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203094&action=review > > > Tools/Scripts/webkitpy/port/win.py:124 > > + # Remove the mac tests, which we never want to execute > > + regex = re.compile('mac.*') > > We don't need to use a regex for this. Also, this regex is going to match anything that happens to start with "mac"; e.g. maclaine. That's the desired behavior --we want to exclude mac, mac-lion, mac-future, etc.

(In reply to comment #8) > (From update of attachment 203094 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=203094&action=review > > >> Tools/Scripts/webkitpy/port/win.py:125 > >> + test_fallback_names = [x for x in self.baseline_search_path() if not regex.match(x)] > > > > Please don't use one letter variable like x. spell out path. We can do path.startswith('mac/') instead. > > I was also going to suggest x.startswith('mac'). You can't use 'mac/', since the slash isn't in the path. However, since baseline_search_path is just a list of dirs, I don't think you need to worry about 'maclaine' or other likely conflicts. The result of calling self.baseline_search_path() is an expanded set of directories, such as: ['/cygdrive/c/Projects/WebKit/OpenSource/LayoutTests/platform/win', '/cygdrive/c/Projects/WebKit/OpenSource/LayoutTests/platform/mac-lion', '/cygdrive/c/Projects/WebKit/OpenSource/LayoutTests/platform/mac'] Consequently, we can't just search on 'starts with'. The regex allowed me to get rid of the mac and mac-lion cases here, though I suppose it might have clobbered some future path.

Created attachment 203101 [details] Uses dpranke's suggested syntax for excluding mac paths.

Created attachment 203102 [details] Uses dpranke's suggested syntax for excluding the mac test folders.

Comment on attachment 203102 [details] Uses dpranke's suggested syntax for excluding the mac test folders. View in context: https://bugs.webkit.org/attachment.cgi?id=203102&action=review > Tools/Scripts/webkitpy/port/win.py:123 > + # Remove the mac tests, which we never want to execute This comment more or less repeats the code. Please remove it.

Committed r150850: <http://trac.webkit.org/changeset/150850>