Changeset 100231 in webkit

Timestamp:

Nov 14, 2011, 6:33:51 PM (15 years ago)

Author:

eric@webkit.org

Message:

check-webkit-style broken by r99773: "Could not determine the port"
​https://bugs.webkit.org/show_bug.cgi?id=72275

Reviewed by Adam Barth.

The TestExpectationsChecker was using a generic try/except block
which caught all exceptions, so we didn't notice that failing
to pass a Host to PortFactory was causing an exception in port instantiation.
I've factored out the "lookup the port" logic into a separate function
which I've now unittested. This should fix the bug and prevent
others like it from occuring the the future.

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

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) (2 diffs)

trunk/Tools/ChangeLog

@@ +1 @@
+2011-11-14  Eric Seidel  <eric@webkit.org>
+
+        check-webkit-style broken by r99773: "Could not determine the port"
+        https://bugs.webkit.org/show_bug.cgi?id=72275
+
+        Reviewed by Adam Barth.
+
+        The TestExpectationsChecker was using a generic try/except block
+        which caught all exceptions, so we didn't notice that failing
+        to pass a Host to PortFactory was causing an exception in port instantiation.
+        I've factored out the "lookup the port" logic into a separate function
+        which I've now unittested.  This should fix the bug and prevent
+        others like it from occuring the the future.
+
+        * Scripts/webkitpy/style/checkers/test_expectations.py:
+        * Scripts/webkitpy/style/checkers/test_expectations_unittest.py:
+
 2011-11-14  Julien Chaffraix  <jchaffraix@webkit.org>
 

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

@@ -35 +35 @@
 
 from common import TabChecker
-from webkitpy.layout_tests.port.factory import PortFactory
+from webkitpy.common.host import Host
 from webkitpy.layout_tests.models import test_expectations
 
@@ -53 +53 @@
     categories = set(['test/expectations'])
 
+    def _determine_port_from_exepectations_path(self, host, expectations_path):
+        try:
+            # I believe what this is trying to do is "when the port name is chromium,
+            # get the chromium-port for this platform".  Unclear why that's needed??
+            port_name = expectations_path.split(host.filesystem.sep)[-2]
+            if port_name == "chromium":
+                return host.port_factory.get(options=ChromiumOptions())
+            # Passing port_name=None to the factory would just return the current port, which isn't what we want, I don't think.
+            if not port_name:
+                return None
+            return host.port_factory.get(port_name)
+        except Exception, e:
+            _log.warn("Exception while getting port for path %s" % expectations_path)
+            return None
+
     def __init__(self, file_path, handle_style_error):
         self._file_path = file_path
@@ -60 +75 @@
         self._output_regex = re.compile('Line:(?P<line>\d+)\s*(?P<message>.+)')
 
-        # FIXME: This should get the PortFactory from a Host object!
-        port_factory = PortFactory()
+        # FIXME: A host should be passed to the constructor instead!
+        host = Host()
+        host._initialize_scm()
 
         # Determining the port of this expectations.
-        try:
-            port_name = self._file_path.split(os.sep)[-2]
-            if port_name == "chromium":
-                self._port_obj = port_factory.get(options=ChromiumOptions())
-            else:
-                self._port_obj = port_factory.get(port_name)
-        except:
-            # Using 'test' port when we couldn't determine the port for 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 = port_factory.get('test')
-        # Suppress error messages of test_expectations module since they will be
-        # reported later.
+            self._port_obj = host.port_factory.get('test')
+        # Suppress error messages of test_expectations module since they will be reported later.
         log = logging.getLogger("webkitpy.layout_tests.layout_package.test_expectations")
         log.setLevel(logging.CRITICAL)

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

@@ -33 +33 @@
 
 from test_expectations import TestExpectationsChecker
-from webkitpy.layout_tests import port
+from webkitpy.common.host_mock import MockHost
 
 
@@ -64 +64 @@
         self._test_file = 'passes/text.html'
 
-    def process_expectations(self, expectations, overrides=None):
-        self._checker = TestExpectationsChecker()
+    def _expect_port_for_expectations_path(self, expected_port_or_port_class, expectations_path):
+        host = MockHost()
+        checker = TestExpectationsChecker(expectations_path, ErrorCollector())
+        port = checker._determine_port_from_exepectations_path(host, expectations_path)
+        if port:
+            self.assertEquals(port.__class__.__name__, expected_port_or_port_class)
+        else:
+            self.assertEquals(port, expected_port_or_port_class)
+
+    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 assert_lines_lint(self, lines, expected):