Title: [100231] trunk/Tools
- Revision
- 100231
- Author
- [email protected]
- Date
- 2011-11-14 18:33:51 -0800 (Mon, 14 Nov 2011)
Log 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:
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (100230 => 100231)
--- trunk/Tools/ChangeLog 2011-11-15 02:18:38 UTC (rev 100230)
+++ trunk/Tools/ChangeLog 2011-11-15 02:33:51 UTC (rev 100231)
@@ -1,3 +1,20 @@
+2011-11-14 Eric Seidel <[email protected]>
+
+ 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 <[email protected]>
Add --css-grid-layout to build-webkit and the build systems
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py (100230 => 100231)
--- trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py 2011-11-15 02:18:38 UTC (rev 100230)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py 2011-11-15 02:33:51 UTC (rev 100231)
@@ -34,7 +34,7 @@
import sys
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
@@ -52,6 +52,21 @@
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
self._handle_style_error = handle_style_error
@@ -59,25 +74,20 @@
self._tab_checker = TabChecker(file_path, handle_style_error)
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)
Modified: trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py (100230 => 100231)
--- trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py 2011-11-15 02:18:38 UTC (rev 100230)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations_unittest.py 2011-11-15 02:33:51 UTC (rev 100231)
@@ -32,7 +32,7 @@
import unittest
from test_expectations import TestExpectationsChecker
-from webkitpy.layout_tests import port
+from webkitpy.common.host_mock import MockHost
class ErrorCollector(object):
@@ -63,9 +63,19 @@
self._error_collector = ErrorCollector()
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):
self._error_collector.reset_errors()
checker = TestExpectationsChecker('test/test_expectations.txt',
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes
