webkitpy: Ref tests don't respect platform specific expectations
authorjbedard@apple.com <jbedard@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Dec 2018 21:48:13 +0000 (21:48 +0000)
committerjbedard@apple.com <jbedard@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Mon, 10 Dec 2018 21:48:13 +0000 (21:48 +0000)
https://bugs.webkit.org/show_bug.cgi?id=192515
<rdar://problem/46564839>

Reviewed by Lucas Forschler.

* Scripts/webkitpy/port/base.py:
(Port._expected_baselines_for_suffixes): Accept multiple suffixes so ref tests can use this function.
(Port.expected_baselines): Move implementation to _expected_baselines_for_suffixes.
(Port.expected_filename): Remove irrelevant FIXME, code clean-up.
(Port.reference_files): Instead of just searching a single directory, use _expected_baselines_for_suffixes to
search all platform expectations as well.
* Scripts/webkitpy/port/base_unittest.py:
(test_ref_tests_platform_directory):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@239048 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/port/base.py
Tools/Scripts/webkitpy/port/base_unittest.py

index 04c14c2..493478b 100644 (file)
@@ -1,3 +1,20 @@
+2018-12-10  Jonathan Bedard  <jbedard@apple.com>
+
+        webkitpy: Ref tests don't respect platform specific expectations
+        https://bugs.webkit.org/show_bug.cgi?id=192515
+        <rdar://problem/46564839>
+
+        Reviewed by Lucas Forschler.
+
+        * Scripts/webkitpy/port/base.py:
+        (Port._expected_baselines_for_suffixes): Accept multiple suffixes so ref tests can use this function.
+        (Port.expected_baselines): Move implementation to _expected_baselines_for_suffixes.
+        (Port.expected_filename): Remove irrelevant FIXME, code clean-up.
+        (Port.reference_files): Instead of just searching a single directory, use _expected_baselines_for_suffixes to
+        search all platform expectations as well.
+        * Scripts/webkitpy/port/base_unittest.py:
+        (test_ref_tests_platform_directory):
+
 2018-12-10  Chris Dumez  <cdumez@apple.com>
 
         Add SPI to allow the client to set the user-agent at main frame level, from the UIProcess
index d01b20b..bd139fd 100644 (file)
@@ -408,6 +408,26 @@ class Port(object):
         """Returns a tuple of all of the non-reftest baseline extensions we use. The extensions include the leading '.'."""
         return ('.wav', '.webarchive', '.txt', '.png')
 
+    def _expected_baselines_for_suffixes(self, test_name, suffixes, all_baselines=False):
+        baseline_search_path = self.baseline_search_path() + [self.layout_tests_dir()]
+
+        baselines = []
+        for platform_dir in baseline_search_path:
+            for suffix in suffixes:
+                baseline_filename = self._filesystem.splitext(test_name)[0] + '-expected' + suffix
+                if self._filesystem.exists(self._filesystem.join(platform_dir, baseline_filename)):
+                    baselines.append((platform_dir, baseline_filename))
+
+            if not all_baselines and baselines:
+                return baselines
+
+        if baselines:
+            return baselines
+
+        for suffix in suffixes:
+            baselines.append((None, self._filesystem.splitext(test_name)[0] + '-expected' + suffix))
+        return baselines
+
     def expected_baselines(self, test_name, suffix, all_baselines=False):
         """Given a test name, finds where the baseline results are located.
 
@@ -435,27 +455,7 @@ class Port(object):
         conjunction with the other baseline and filename routines that are
         platform specific.
         """
-        baseline_filename = self._filesystem.splitext(test_name)[0] + '-expected' + suffix
-        baseline_search_path = self.baseline_search_path()
-
-        baselines = []
-        for platform_dir in baseline_search_path:
-            if self._filesystem.exists(self._filesystem.join(platform_dir, baseline_filename)):
-                baselines.append((platform_dir, baseline_filename))
-
-            if not all_baselines and baselines:
-                return baselines
-
-        # If it wasn't found in a platform directory, return the expected
-        # result in the test directory, even if no such file actually exists.
-        platform_dir = self.layout_tests_dir()
-        if self._filesystem.exists(self._filesystem.join(platform_dir, baseline_filename)):
-            baselines.append((platform_dir, baseline_filename))
-
-        if baselines:
-            return baselines
-
-        return [(None, baseline_filename)]
+        return self._expected_baselines_for_suffixes(test_name, [suffix], all_baselines=all_baselines)
 
     def expected_filename(self, test_name, suffix, return_default=True):
         """Given a test name, returns an absolute path to its expected results.
@@ -477,13 +477,9 @@ class Port(object):
         This routine is generic but is implemented here to live alongside
         the other baseline and filename manipulation routines.
         """
-        # FIXME: The [0] here is very mysterious, as is the destructured return.
         platform_dir, baseline_filename = self.expected_baselines(test_name, suffix)[0]
-        if platform_dir:
-            return self._filesystem.join(platform_dir, baseline_filename)
-
-        if return_default:
-            return self._filesystem.join(self.layout_tests_dir(), baseline_filename)
+        if platform_dir or return_default:
+            return self._filesystem.join(platform_dir or self.layout_tests_dir(), baseline_filename)
         return None
 
     def expected_checksum(self, test_name):
@@ -553,17 +549,23 @@ class Port(object):
         if self.get_option('treat_ref_tests_as_pixel_tests'):
             return []
 
-        reftest_list = self._get_reftest_list(test_name)
-        if not reftest_list:
-            reftest_list = []
-            for expectation, prefix in (('==', ''), ('!=', '-mismatch')):
-                for extention in Port._supported_reference_extensions:
-                    path = self.expected_filename(test_name, prefix + extention)
-                    if self._filesystem.exists(path):
-                        reftest_list.append((expectation, path))
-            return reftest_list
-
-        return reftest_list.get(self._filesystem.join(self.layout_tests_dir(), test_name), [])  # pylint: disable=E1103
+        result = self._get_reftest_list(test_name)
+        if result:
+            return result.get(self._filesystem.join(self.layout_tests_dir(), test_name), [])
+
+        result = []
+        suffixes = []
+        for part1 in ['', '-mismatch']:
+            for part2 in self._supported_reference_extensions:
+                suffixes.append(part1 + part2)
+        for platform_dir, baseline_filename in self._expected_baselines_for_suffixes(test_name, suffixes):
+            if not platform_dir:
+                continue
+            result.append((
+                '!=' if '-mismatch.' in baseline_filename else '==',
+                self._filesystem.join(platform_dir, baseline_filename),
+            ))
+        return result
 
     def potential_test_names_from_expected_file(self, path):
         """Return potential test names if any from a potential expected file path, relative to LayoutTests directory."""
index 0a5a947..73c24b7 100644 (file)
@@ -411,6 +411,23 @@ class PortTest(unittest.TestCase):
         self.assertTrue(port._filesystem.isdir(jhbuild_path))
         self.assertTrue(port._should_use_jhbuild())
 
+    def test_ref_tests_platform_directory(self):
+        port = self.make_port(port_name='foo')
+        port.default_baseline_search_path = lambda: ['/mock-checkout/LayoutTests/platform/foo']
+        port._filesystem.write_text_file('/mock-checkout/LayoutTests/fast/ref-expected.html', 'foo')
+
+        # No platform directory
+        self.assertEqual(
+            [('==', '/mock-checkout/LayoutTests/fast/ref-expected.html')],
+            port.reference_files('fast/ref.html'),
+        )
+
+        port._filesystem.write_text_file('/mock-checkout/LayoutTests/platform/foo/fast/ref-expected-mismatch.html', 'foo-plat')
+        self.assertEqual(
+            [('!=', '/mock-checkout/LayoutTests/platform/foo/fast/ref-expected-mismatch.html')],
+            port.reference_files('fast/ref.html'),
+        )
+
 
 class NaturalCompareTest(unittest.TestCase):
     def setUp(self):