Attachment 356864 Details for Bug 192515 – Patch

[patch] Patch

bug-192515-20181207222329.patch (text/plain), 7.62 KB, created by Jonathan Bedard on 2018-12-07 22:23:30 PST

(hide)

Description:

Filename:

MIME Type:

Creator: Jonathan Bedard

Created: 2018-12-07 22:23:30 PST

Size: 7.62 KB

patch

obsolete

>Index: Tools/ChangeLog
>===================================================================
>--- Tools/ChangeLog	(revision 239004)
>+++ Tools/ChangeLog	(working copy)
>@@ -1,3 +1,20 @@
>+2018-12-07  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 NOBODY (OOPS!).
>+
>+        * 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-07  Chris Dumez  <cdumez@apple.com>
> 
>         ProcessSwap.UseSessionCookiesAfterProcessSwapInPrivateBrowsing API test is failing
>Index: Tools/Scripts/webkitpy/port/base.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/base.py	(revision 238966)
>+++ Tools/Scripts/webkitpy/port/base.py	(working copy)
>@@ -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: Tools/Scripts/webkitpy/port/base_unittest.py
>===================================================================
>--- Tools/Scripts/webkitpy/port/base_unittest.py	(revision 238966)
>+++ Tools/Scripts/webkitpy/port/base_unittest.py	(working copy)
>@@ -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):

Actions: View | Formatted Diff | Diff

Attachments on bug 192515: 356851 | 356856 | 356857 | 356859 | 356860 | 356861 | 356864