Changeset 107124 in webkit

Timestamp:

Feb 8, 2012 1:02:53 PM (12 years ago)

Author:

dpranke@chromium.org

Message:

check-webkit-style failing with "Path does not exist."
​https://bugs.webkit.org/show_bug.cgi?id=77873

Reviewed by Ojan Vafai.

This change fixes the way the style checker determines which
Port class to use for a given test_expectations.txt path; the
previous version used a heuristic that didn't really work in the
first place.

• Scripts/webkitpy/style/checkers/test_expectations.py:

(TestExpectationsChecker._determine_port_from_expectations_path):
(TestExpectationsChecker.init):

• Scripts/webkitpy/style/checkers/test_expectations_unittest.py:

(TestExpectationsTestCase._expect_port_for_expectations_path):
(TestExpectationsTestCase.test_determine_port_from_expectations_path):

Location:

trunk/Tools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/style/checkers/test_expectations.py (modified) (3 diffs)
• Scripts/webkitpy/style/checkers/test_expectations_unittest.py (modified) (1 diff)

trunk/Tools/ChangeLog

@@ +1 @@
+2012-02-08  Dirk Pranke  <dpranke@chromium.org>
+
+        check-webkit-style failing with "Path does not exist."
+        https://bugs.webkit.org/show_bug.cgi?id=77873
+
+        Reviewed by Ojan Vafai.
+
+        This change fixes the way the style checker determines which
+        Port class to use for a given test_expectations.txt path; the
+        previous version used a heuristic that didn't really work in the
+        first place.
+
+        * Scripts/webkitpy/style/checkers/test_expectations.py:
+        (TestExpectationsChecker._determine_port_from_expectations_path):
+        (TestExpectationsChecker.__init__):
+        * Scripts/webkitpy/style/checkers/test_expectations_unittest.py:
+        (TestExpectationsTestCase._expect_port_for_expectations_path):
+        (TestExpectationsTestCase.test_determine_port_from_expectations_path):
+
 2012-02-08  Fehér Zsolt  <feherzs@inf.u-szeged.hu>
 

trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py

@@ -48 +48 @@
     categories = set(['test/expectations'])
 
-    def _determine_port_from_exepectations_path(self, host, expectations_path):
-        try:
-            port_name = expectations_path.split(host.filesystem.sep)[-2]
-            if not port_name:
-                return None
-
-            # Pass a configuration to avoid calling default_configuration() when initializing the port (takes 0.5 seconds on a Mac Pro!).
-            return host.port_factory.get(port_name, options=DummyOptions(configuration="Release"))
-        except Exception, e:
-            _log.warn("Exception while getting port for path %s" % expectations_path)
-            return None
+    def _determine_port_from_expectations_path(self, host, expectations_path):
+        # Pass a configuration to avoid calling default_configuration() when initializing the port (takes 0.5 seconds on a Mac Pro!).
+        options = DummyOptions(configuration='Release')
+        for port_name in host.port_factory.all_port_names():
+            port = host.port_factory.get(port_name, options=options)
+            if port.path_to_test_expectations_file().replace(port.path_from_webkit_base() + host.filesystem.sep, '') == expectations_path:
+                return port
+        return None
 
     def __init__(self, file_path, handle_style_error, host=None):
@@ -71 +68 @@
         host._initialize_scm()
 
-        # Determining the port of this expectations.
-        self._port_obj = self._determine_port_from_exepectations_path(host, file_path)
-        # Using 'test' port when we couldn't determine the port for this
-        # expectations.
-        if not self._port_obj:
-            _log.warn("Could not determine the port for %s. "
-                      "Using 'test' port, but platform-specific expectations "
-                      "will fail the check." % self._file_path)
-            self._port_obj = host.port_factory.get('test')
+        self._port_obj = self._determine_port_from_expectations_path(host, file_path)
+
         # Suppress error messages of test_expectations module since they will be reported later.
         log = logging.getLogger("webkitpy.layout_tests.layout_package.test_expectations")
@@ -113 +103 @@
         overrides = self._port_obj.test_expectations_overrides()
         expectations = '\n'.join(lines)
-        self.check_test_expectations(expectations_str=expectations,
-                                     tests=None,
-                                     overrides=overrides)
+        if self._port_obj:
+            self.check_test_expectations(expectations_str=expectations,
+                                         tests=None,
+                                         overrides=overrides)
+        else:
+            self._handle_style_error(1, 'test/expectations', 5,
+                                     'No port uses path %s for test_expectations' % self._file_path)
         # Warn tabs in lines as well
         self.check_tabs(lines)

trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py

@@ -64 +64 @@
         self._test_file = 'passes/text.html'
 
-    def _expect_port_for_expectations_path(self, expected_port_or_port_class, expectations_path):
+    def _expect_port_for_expectations_path(self, expected_port_implementation, expectations_path):
         host = MockHost()
         checker = TestExpectationsChecker(expectations_path, ErrorCollector(), host=host)
-        port = checker._determine_port_from_exepectations_path(host, expectations_path)
+        port = checker._determine_port_from_expectations_path(host, expectations_path)
         if port:
-            self.assertEquals(port.__class__.__name__, expected_port_or_port_class)
+            self.assertTrue(port.name().startswith(expected_port_implementation))
         else:
-            self.assertEquals(port, expected_port_or_port_class)
+            self.assertEquals(None, expected_port_implementation)
 
-    def test_determine_port_from_exepectations_path(self):
-        self._expect_port_for_expectations_path(None, "/")
-        self._expect_port_for_expectations_path("ChromiumMacPort", "/mock-checkout/LayoutTests/chromium-mac/test_expectations.txt")
+    def test_determine_port_from_expectations_path(self):
+        self._expect_port_for_expectations_path(None, '/')
+        self._expect_port_for_expectations_path(None, 'LayoutTests/chromium-mac/test_expectations.txt')
+        self._expect_port_for_expectations_path('chromium', 'LayoutTests/platform/chromium/test_expectations.txt')
+        self._expect_port_for_expectations_path(None, '/mock-checkout/LayoutTests/platform/win/test_expectations.txt')
+        self._expect_port_for_expectations_path('win', 'LayoutTests/platform/win/test_expectations.txt')
 
     def assert_lines_lint(self, lines, should_pass, expected_output=None):
         self._error_collector.reset_errors()
+
+        host = MockHost()
         checker = TestExpectationsChecker('test/test_expectations.txt',
-                                          self._error_collector, host=MockHost())
+                                          self._error_collector, host=host)
+
+        # We should have failed to find a valid port object for that path.
+        self.assertEquals(checker._port_obj, None)
+
+        # Now use a test port so we can check the lines.
+        checker._port_obj = host.port_factory.get('test-mac-leopard')
         checker.check_test_expectations(expectations_str='\n'.join(lines),
                                         tests=[self._test_file],