2010-10-18 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Oct 2010 00:40:58 +0000 (00:40 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Oct 2010 00:40:58 +0000 (00:40 +0000)
        Reviewed by Eric Seidel.

        Re-submit a revised version of r69638 - enabling new-run-webkit-tests
        under cygwin. The initial version had a bug in base:uri_to_test_name
        that was causing tests to fail. This version corrects that bug, but
        also makes the code safer by calling cygpath more reliably, and
        leaving a long-running cygpath process open.

        This patch also corrects a couple of minor bugs in http_lock_unittest,
        chromium_unittest, and dedpulicate_tests_unittest that showed up
        while testing this.

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

        * Scripts/webkitpy/common/system/path.py:
        * Scripts/webkitpy/common/system/path_unittest.py:
        * Scripts/webkitpy/layout_tests/deduplicate_tests_unittest.py:
        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
        * Scripts/webkitpy/layout_tests/port/chromium.py:
        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
        * Scripts/webkitpy/layout_tests/port/http_lock_unittest.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:

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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/common/system/path.py
WebKitTools/Scripts/webkitpy/common/system/path_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/deduplicate_tests_unittest.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/port/chromium_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/http_lock_unittest.py

index 23c991cb7177f3780d58c5d1923da80c54a8a389..2a9a3f76bc0581ff85245df1e2dc413b2cd42d48 100644 (file)
@@ -1,3 +1,30 @@
+2010-10-18  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Seidel.
+
+        Re-submit a revised version of r69638 - enabling new-run-webkit-tests
+        under cygwin. The initial version had a bug in base:uri_to_test_name
+        that was causing tests to fail. This version corrects that bug, but
+        also makes the code safer by calling cygpath more reliably, and
+        leaving a long-running cygpath process open.
+
+        This patch also corrects a couple of minor bugs in http_lock_unittest,
+        chromium_unittest, and dedpulicate_tests_unittest that showed up
+        while testing this.
+
+        https://bugs.webkit.org/show_bug.cgi?id=47220
+
+        * Scripts/webkitpy/common/system/path.py:
+        * Scripts/webkitpy/common/system/path_unittest.py:
+        * Scripts/webkitpy/layout_tests/deduplicate_tests_unittest.py:
+        * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py:
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/chromium.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/http_lock_unittest.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+
 2010-10-18  Eric Seidel  <eric@webkit.org>
 
         Reviewed by Adam Barth.
index 937247ef0acc1919de0ed7e6b70d4ca8171c7110..43c6410bc8081d5b551bcbd5dca3fd5a050044a1 100644 (file)
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 """generic routines to convert platform-specific paths to URIs."""
+from __future__ import with_statement
+
+import atexit
+import subprocess
 import sys
+import threading
 import urllib
 
 
-def abspath_to_uri(path, executive, platform=None):
+def abspath_to_uri(path, platform=None):
     """Converts a platform-specific absolute path to a file: URL."""
     if platform is None:
         platform = sys.platform
-    return "file:" + _escape(_convert_path(path, executive, platform))
+    return "file:" + _escape(_convert_path(path, platform))
+
+
+def cygpath(path):
+    """Converts a cygwin path to Windows path."""
+    return _CygPath.convert_using_singleton(path)
+
+
+# Note that this object is not threadsafe and must only be called
+# from multiple threads under protection of a lock (as is done in cygpath())
+class _CygPath(object):
+    """Manages a long-running 'cygpath' process for file conversion."""
+    _lock = None
+    _singleton = None
+
+    @staticmethod
+    def stop_cygpath_subprocess():
+        if not _CygPath._lock:
+            return
+
+        with _CygPath._lock:
+            if _CygPath._singleton:
+                _CygPath._singleton.stop()
+
+    @staticmethod
+    def convert_using_singleton(path):
+        if not _CygPath._lock:
+            _CygPath._lock = threading.Lock()
+
+        with _CygPath._lock:
+            if not _CygPath._singleton:
+                _CygPath._singleton = _CygPath()
+                # Make sure the cygpath subprocess always gets shutdown cleanly.
+                atexit.register(_CygPath.stop_cygpath_subprocess)
+
+            return _CygPath._singleton.convert(path)
+
+    def __init__(self):
+        self._child_process = None
+
+    def start(self):
+        assert(self._child_process is None)
+        args = ['cygpath', '-f', '-', '-wa']
+        self._child_process = subprocess.Popen(args,
+                                               stdin=subprocess.PIPE,
+                                               stdout=subprocess.PIPE)
+
+    def is_running(self):
+        if not self._child_process:
+            return False
+        return self._child_process.returncode is None
+
+    def stop(self):
+        if self._child_process:
+            self._child_process.stdin.close()
+            self._child_process.wait()
+        self._child_process = None
+
+    def convert(self, path):
+        if not self.is_running():
+            self.start()
+        self._child_process.stdin.write("%s\r\n" % path)
+        self._child_process.stdin.flush()
+        return self._child_process.stdout.readline().rstrip()
 
 
 def _escape(path):
@@ -47,12 +115,12 @@ def _escape(path):
     return urllib.quote(path, safe='/+:')
 
 
-def _convert_path(path, executive, platform):
+def _convert_path(path, platform):
     """Handles any os-specific path separators, mappings, etc."""
     if platform == 'win32':
         return _winpath_to_uri(path)
     if platform == 'cygwin':
-        return _winpath_to_uri(_cygpath(path, executive))
+        return _winpath_to_uri(cygpath(path))
     return _unixypath_to_uri(path)
 
 
@@ -61,12 +129,6 @@ def _winpath_to_uri(path):
     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 3c1021dcad7d5a7a8ed245d34dfb11578da4ba58..4dbd38a1e2a6bf514cc5aab3f1dbe354fda52f26 100644 (file)
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import unittest
+import sys
 
 import path
 
-
 class AbspathTest(unittest.TestCase):
-    def run_command(self, args, **kwargs):
-        self.assertEqual(args[0], 'cygpath')
-        return self.cygpath_result
-
     def assertMatch(self, test_path, expected_uri,
-                    platform, cygpath_result=None):
-        if platform == 'cygwin':
-            self.cygpath_result = cygpath_result
-        self.assertEqual(path.abspath_to_uri(test_path, executive=self,
-                                             platform=platform),
+                    platform=None):
+        if platform == 'cygwin' and sys.platform != 'cygwin':
+            return
+        self.assertEqual(path.abspath_to_uri(test_path, platform=platform),
                          expected_uri)
 
     def test_abspath_to_uri_cygwin(self):
+        if sys.platform != 'cygwin':
+            return
+
         self.assertMatch('/cygdrive/c/foo/bar.html',
-                         'file:///c:/foo/bar.html',
-                         platform='cygwin',
-                         cygpath_result='c:\\foo\\bar.html\n')
+                         'file:///C:/foo/bar.html',
+                         platform='cygwin')
         self.assertEqual(path.abspath_to_uri('/cygdrive/c/foo/bar.html',
-                                             executive=self,
                                              platform='cygwin'),
-                         'file:///c:/foo/bar.html')
+                         'file:///C:/foo/bar.html')
 
     def test_abspath_to_uri_darwin(self):
         self.assertMatch('/foo/bar.html',
                          'file:///foo/bar.html',
                          platform='darwin')
         self.assertEqual(path.abspath_to_uri("/foo/bar.html",
-                                             executive=self,
                                              platform='darwin'),
                          "file:///foo/bar.html")
 
@@ -68,7 +63,6 @@ class AbspathTest(unittest.TestCase):
                          'file:///foo/bar.html',
                          platform='darwin')
         self.assertEqual(path.abspath_to_uri("/foo/bar.html",
-                                             executive=self,
                                              platform='linux2'),
                          "file:///foo/bar.html")
 
@@ -77,7 +71,6 @@ class AbspathTest(unittest.TestCase):
                          'file:///c:/foo/bar.html',
                          platform='win32')
         self.assertEqual(path.abspath_to_uri("c:\\foo\\bar.html",
-                                             executive=self,
                                              platform='win32'),
                          "file:///c:/foo/bar.html")
 
@@ -91,10 +84,22 @@ class AbspathTest(unittest.TestCase):
 
         # Note that you can't have '?' in a filename on windows.
         self.assertMatch('/cygdrive/c/foo/bar + baz%.html',
-                         'file:///c:/foo/bar%20+%20baz%25.html',
-                         platform='cygwin',
-                         cygpath_result='c:\\foo\\bar + baz%.html\n')
+                         'file:///C:/foo/bar%20+%20baz%25.html',
+                         platform='cygwin')
+
+    def test_stop_cygpath_subprocess(self):
+        if sys.platform != 'cygwin':
+            return
+
+        # Call cygpath to ensure the subprocess is running.
+        path.cygpath("/cygdrive/c/foo.txt")
+        self.assertTrue(path._CygPath._singleton.is_running())
+
+        # Stop it.
+        path._CygPath.stop_cygpath_subprocess()
 
+        # Ensure that it is stopped.
+        self.assertFalse(path._CygPath._singleton.is_running())
 
 if __name__ == '__main__':
     unittest.main()
index bb9604f5c502babf7132b0007099238ef5598a9a..309bf8d9e7ab1a08ca3dddeb86148c6cacd132ac 100644 (file)
@@ -205,3 +205,6 @@ class ListDuplicatesTest(unittest.TestCase):
         for expected, inputs in test_cases:
             self.assertEquals(expected,
                               deduplicate_tests.get_relative_test_path(*inputs))
+
+if __name__ == '__main__':
+    unittest.main()
index b15edb246468bf82dc95520e43d49db881bbe8ae..cb96913582d6f177cf2abbdda875ee7ce6b6174e 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
 
 
@@ -336,17 +337,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))
 
     def tests(self, paths):
         """Return the list of tests found (relative to layout_tests_dir()."""
@@ -395,12 +386,8 @@ class Port(object):
         """
         test = uri
         if uri.startswith("file:///"):
-            if sys.platform == 'win32':
-                test = test.replace('file:///', '')
-                test = test.replace('/', '\\')
-            else:
-                test = test.replace('file://', '')
-            return self.relative_test_filename(test)
+            prefix = abspath_to_uri(self.layout_tests_dir()) + "/"
+            return test[len(prefix):]
 
         if uri.startswith("http://127.0.0.1:8880/"):
             # websocket tests
index 40eb984cb914a59509699020809a3b0d6022a10c..baecad9d4ab2571e876f21e3636d54c6d5b1012c 100644 (file)
@@ -33,6 +33,7 @@ import sys
 import tempfile
 import unittest
 
+from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.common.system.executive import Executive, ScriptError
 from webkitpy.thirdparty.mock import Mock
 
@@ -244,8 +245,7 @@ class PortTest(unittest.TestCase):
             path = test_file
 
         self.assertEqual(port.filename_to_uri(test_file),
-                         prefix + path)
-
+                         abspath_to_uri(test_file))
 
 
 class VirtualTest(unittest.TestCase):
index 0fae62f3a231e60dd664992e75d0d18d35a3ac26..8356bd90dfb066583d63713cfbc30771b76761b4 100644 (file)
@@ -43,6 +43,10 @@ import tempfile
 import time
 import webbrowser
 
+from webkitpy.common.system.executive import Executive
+from webkitpy.common.system.path import cygpath
+from webkitpy.layout_tests.layout_package import test_expectations
+
 import base
 import http_server
 
@@ -344,6 +348,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)
+        return path
+
     def _path_to_image_diff(self):
         binary_name = 'image_diff'
         if self._options.use_drt:
@@ -363,7 +374,9 @@ 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 e3858f0c4fc672b389bc9188f9ad6068e11cee66..2e9461a3f470ab1a28df8ee8a523972fc3f8f529 100644 (file)
@@ -176,7 +176,9 @@ DEFER LINUX WIN : fast/js/very-good.js = TIMEOUT PASS"""
                             return_exit_code=False,
                             return_stderr=True,
                             decode_output=False):
-                return self._result
+                if return_exit_code:
+                    return self._result
+                return ''
 
         options = EmptyOptions()
         port = ChromiumPortTest.TestLinuxPort(options)
index f2e4ce57cbd1a1d04423af056e9610f10795e0fa..85c760ac5fa0436991d486387120c87fbd62c904 100644 (file)
@@ -43,7 +43,7 @@ class HttpLockTest(unittest.TestCase):
 
     def clean_all_lockfile(self):
         if os.path.exists(self.guard_lock_file):
-            os.unlink(guard_lock_file)
+            os.unlink(self.guard_lock_file)
         lock_list = glob.glob(self.lock_file_path_prefix + '*')
         for file_name in lock_list:
             os.unlink(file_name)