2010-10-12 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Oct 2010 04:39:56 +0000 (04:39 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 13 Oct 2010 04:39:56 +0000 (04:39 +0000)
        Reviewed by Eric Seidel.

        This patch enables new-run-webkit-tests (in particular the
        chromium-win port) to run under Cygwin as well as Win32. Mostly
        this just required some conversions from cygwin paths to Win32
        paths when we spawn off Win32 binaries like test_shell.

        https://bugs.webkit.org/show_bug.cgi?id=47220

        * Scripts/webkitpy/common/system/path.py:
        - Expose the cygpath() function for path conversion

        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
        - shift filename->uri conversion in the TestInfo objects to the
          dump_render_tree thread

        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/chromium.py:
        - use cygpath()

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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/common/system/path.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

index e16f4b26ef6d9575869fa1e7eb2801d2c130b44b..e9bd3107e5183a21a8a2af5a7615be68263dc2d7 100644 (file)
@@ -1,3 +1,26 @@
+2010-10-12  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        This patch enables new-run-webkit-tests (in particular the
+        chromium-win port) to run under Cygwin as well as Win32. Mostly
+        this just required some conversions from cygwin paths to Win32
+        paths when we spawn off Win32 binaries like test_shell.
+
+        https://bugs.webkit.org/show_bug.cgi?id=47220
+
+        * Scripts/webkitpy/common/system/path.py:
+        - Expose the cygpath() function for path conversion
+
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
+        - shift filename->uri conversion in the TestInfo objects to the
+          dump_render_tree thread
+
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        - use cygpath()
+
 2010-10-12  Yuta Kitamura  <yutak@chromium.org>
 
         Unreviewed. Add Yuta Kitamura (yutak) to the committers list.
index 937247ef0acc1919de0ed7e6b70d4ca8171c7110..528210a06391898436e64615469e688fedfc13fa 100644 (file)
@@ -38,6 +38,21 @@ def abspath_to_uri(path, executive, platform=None):
     return "file:" + _escape(_convert_path(path, executive, platform))
 
 
+def cygpath(path, executive):
+    """Converts a cygwin path to Windows path."""
+    # FIXME: this may not be correct in every situation, but forking
+    # cygpath is very slow. More importantly, there is a bug in Python
+    # where launching subprocesses and communicating with PIPEs (which
+    # is what run_command() does) can lead to deadlocks when running in
+    # multiple threads.
+    if path.startswith("/cygdrive"):
+        path = path[10] + ":" + path[11:]
+        path = path.replace("/", "\\")
+        return path
+    return executive.run_command(['cygpath', '-wa', path],
+                                 decode_output=False).rstrip()
+
+
 def _escape(path):
     """Handle any characters in the path that should be escaped."""
     # FIXME: web browsers don't appear to blindly quote every character
@@ -52,7 +67,7 @@ def _convert_path(path, executive, platform):
     if platform == 'win32':
         return _winpath_to_uri(path)
     if platform == 'cygwin':
-        return _winpath_to_uri(_cygpath(path, executive))
+        return _winpath_to_uri(cygpath(path, executive))
     return _unixypath_to_uri(path)
 
 
@@ -60,13 +75,6 @@ def _winpath_to_uri(path):
     """Converts a window absolute path to a file: URL."""
     return "///" + path.replace("\\", "/")
 
-
-def _cygpath(path, executive):
-    """Converts a cygwin path to Windows path."""
-    return executive.run_command(['cygpath', '-wa', path],
-                                 decode_output=False).rstrip()
-
-
 def _unixypath_to_uri(path):
     """Converts a unix-style path to a file: URL."""
     return "//" + path
index e0fd1b6bbd6ee895542bf2803e16d8b790d26738..d6e7bc482c1c5f90fd8a09f784737d606b6a888e 100644 (file)
@@ -83,7 +83,7 @@ def _process_output(port, options, test_info, test_types, test_args,
       port: port-specific hooks
       options: command line options argument from optparse
       proc: an active DumpRenderTree process
-      test_info: Object containing the test filename, uri and timeout
+      test_info: Object containing the test filename and timeout
       test_types: list of test types to subject the output to
       test_args: arguments to be passed to each test
 
@@ -172,7 +172,7 @@ class SingleTestThread(threading.Thread):
         Args:
           port: object implementing port-specific hooks
           options: command line argument object from optparse
-          test_info: Object containing the test filename, uri and timeout
+          test_info: Object containing the test filename and timeout
           test_types: A list of TestType objects to run the test output
               against.
           test_args: A TestArguments object to pass to each TestType.
@@ -193,12 +193,13 @@ class SingleTestThread(threading.Thread):
         # FIXME: this is a separate routine to work around a bug
         # in coverage: see http://bitbucket.org/ned/coveragepy/issue/85.
         test_info = self._test_info
+        uri = self._port.filename_to_uri(test_info.filename)
         self._driver = self._port.create_driver(self._test_args.png_path,
                                                 self._options)
         self._driver.start()
         start = time.time()
         crash, timeout, actual_checksum, output, error = \
-            self._driver.run_test(test_info.uri.strip(), test_info.timeout,
+            self._driver.run_test(uri, test_info.timeout,
                                   test_info.image_hash())
         end = time.time()
         self._test_result = _process_output(self._port, self._options,
@@ -255,7 +256,7 @@ class TestShellThread(WatchableThread):
           port: interface to port-specific hooks
           options: command line options argument from optparse
           filename_list_queue: A thread safe Queue class that contains lists
-              of tuples of (filename, uri) pairs.
+              of (filename, TestInfo) pairs.
           result_queue: A thread safe Queue class that will contain tuples of
               (test, failure lists) for the test results.
           test_types: A list of TestType objects to run the test output
@@ -459,7 +460,7 @@ class TestShellThread(WatchableThread):
         files singly.
 
         Args:
-          test_info: Object containing the test filename, uri and timeout
+          test_info: Object containing the test filename and timeout
 
         Returns:
           A TestResult
@@ -507,7 +508,7 @@ class TestShellThread(WatchableThread):
         """Run a single test file using a shared DumpRenderTree process.
 
         Args:
-          test_info: Object containing the test filename, uri and timeout
+          test_info: Object containing the test filename and timeout
 
         Returns:
           A list of TestFailure objects describing the error.
@@ -529,8 +530,9 @@ class TestShellThread(WatchableThread):
              _pad_timeout(int(test_info.timeout)))
         self._next_timeout = start + thread_timeout
 
+        uri = self._port.filename_to_uri(test_info.filename)
         crash, timeout, actual_checksum, output, error = \
-           self._driver.run_test(test_info.uri, test_info.timeout, image_hash)
+           self._driver.run_test(uri, test_info.timeout, image_hash)
         end = time.time()
 
         result = _process_output(self._port, self._options,
index 38b982b551d82076015043f8990126f126a48c9a..042f0aafc971e408faf15282d96833222295748b 100644 (file)
@@ -49,6 +49,7 @@ import websocket_server
 
 from webkitpy.common.system import logutils
 from webkitpy.common.system.executive import Executive, ScriptError
+from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.common.system.user import User
 
 
@@ -312,17 +313,7 @@ class Port(object):
                 protocol = "http"
             return "%s://127.0.0.1:%u/%s" % (protocol, port, relative_path)
 
-        abspath = os.path.abspath(filename)
-
-        # On Windows, absolute paths are of the form "c:\foo.txt". However,
-        # all current browsers (except for Opera) normalize file URLs by
-        # prepending an additional "/" as if the absolute path was
-        # "/c:/foo.txt". This means that all file URLs end up with "file:///"
-        # at the beginning.
-        if sys.platform == 'win32':
-            abspath = '/' + abspath.replace('\\', '/')
-
-        return "file://" + abspath
+        return abspath_to_uri(os.path.abspath(filename), self._executive)
 
     def tests(self, paths):
         """Return the list of tests found (relative to layout_tests_dir()."""
@@ -371,9 +362,12 @@ class Port(object):
         """
         test = uri
         if uri.startswith("file:///"):
+            # FIXME: need an inverse of uri_to_abspath()
             if sys.platform == 'win32':
                 test = test.replace('file:///', '')
                 test = test.replace('/', '\\')
+            elif sys.platform == 'cygwin':
+                test = '/cygdrive/' + uri[8] + '/' + uri[11:]
             else:
                 test = test.replace('file://', '')
             return self.relative_test_filename(test)
index e66c64dbe87dd47eb86117dac28de43e644518a3..d9cb4678c35ddb44965338b1e5e14f02b29350d3 100644 (file)
@@ -34,6 +34,7 @@ import tempfile
 import unittest
 
 from webkitpy.common.system.executive import Executive, ScriptError
+from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.thirdparty.mock import Mock
 
 
@@ -227,25 +228,11 @@ class PortTest(unittest.TestCase):
         self.assertTrue('css2.1' in dirs)
 
     def test_filename_to_uri(self):
-
         port = base.Port()
         layout_test_dir = port.layout_tests_dir()
         test_file = os.path.join(layout_test_dir, "foo", "bar.html")
-
-        # On Windows, absolute paths are of the form "c:\foo.txt". However,
-        # all current browsers (except for Opera) normalize file URLs by
-        # prepending an additional "/" as if the absolute path was
-        # "/c:/foo.txt". This means that all file URLs end up with "file:///"
-        # at the beginning.
-        if sys.platform == 'win32':
-            prefix = "file:///"
-            path = test_file.replace("\\", "/")
-        else:
-            prefix = "file://"
-            path = test_file
-
         self.assertEqual(port.filename_to_uri(test_file),
-                         prefix + path)
+                         abspath_to_uri(test_file, Executive()))
 
 
 
index 301d4b189b09c83d2706e360a73e8823fc1b0808..1f784dc3b8750793f61d6939066f4ab674166936 100644 (file)
@@ -43,12 +43,13 @@ import tempfile
 import time
 import webbrowser
 
-import base
-import http_server
-
 from webkitpy.common.system.executive import Executive
+from webkitpy.common.system.path import abspath_to_uri, cygpath
 from webkitpy.layout_tests.layout_package import test_expectations
 
+import base
+import http_server
+
 # Chromium DRT on OSX uses WebKitDriver.
 if sys.platform == 'darwin':
     import webkit
@@ -143,11 +144,21 @@ class ChromiumPort(base.Port):
         with open(actual_filename, 'w+b') as file:
             file.write(actual_contents)
 
+        # We use convert_path if there's a chance that the launched
+        # executable needs filename arguments in a different format than
+        # the normal format provided by the python runtime. The main
+        # example of this is running under Cygwin on Windows but
+        # launching a Win32 binary, where we need to convert the path
+        # from /cygdrive/c/foo.txt to c:\foo.txt.
         if diff_filename:
-            cmd = [executable, '--diff', expected_filename,
-                   actual_filename, diff_filename]
+            cmd = [executable, '--diff',
+                   self._convert_path(expected_filename),
+                   self._convert_path(actual_filename),
+                   self._convert_path(diff_filename)]
         else:
-            cmd = [executable, expected_filename, actual_filename]
+            cmd = [executable,
+                   self._convert_path(expected_filename),
+                   self._convert_path(actual_filename)]
 
         result = True
         try:
@@ -340,6 +351,13 @@ class ChromiumPort(base.Port):
             platform = self.name()
         return self.path_from_webkit_base('LayoutTests', 'platform', platform)
 
+    def _convert_path(self, path):
+        """Handles filename conversion for subprocess command line args."""
+        # See note above in diff_image() for why we need this.
+        if sys.platform == 'cygwin':
+            return cygpath(path, self._executive)
+        return path
+
     def _path_to_image_diff(self):
         binary_name = 'image_diff'
         if self._options.use_drt:
@@ -359,7 +377,10 @@ class ChromiumDriver(base.Driver):
     def _driver_args(self):
         driver_args = []
         if self._image_path:
-            driver_args.append("--pixel-tests=" + self._image_path)
+            # See note above in diff_image() for why we need
+            # _convert_path().
+            driver_args.append("--pixel-tests=" +
+                               self._port._convert_path(self._image_path))
 
         if self._options.use_drt:
             driver_args.append('--test-shell')
index 9cc7895f3aa8e38a7a61338198062a65f23c6c94..efac7f26ee4fb1e618ef3b17ba842ab847af40a2 100755 (executable)
@@ -91,14 +91,13 @@ class TestInfo:
     """Groups information about a test for easy passing of data."""
 
     def __init__(self, port, filename, timeout):
-        """Generates the URI and stores the filename and timeout for this test.
+        """
         Args:
           filename: Full path to the test.
-          timeout: Timeout for running the test in TestShell.
+          timeout: Timeout for running the test in DRT.
           """
         self.filename = filename
         self._port = port
-        self.uri = port.filename_to_uri(filename)
         self.timeout = timeout
         self._image_checksum = -1