Created attachment 202755 [details] Screenshot of example output from NRWT. When running ORWT on the Apple Windows port, several test results 'fall back' to the mac port for expected results. NRWT also appears to be checking the 'mac' port output folder, but registers a number of items as failures even though there are existing 'win' results that should be overriding these expected results.
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>