Changeset 69065 in webkit

Timestamp:

Oct 4, 2010 9:19:16 PM (14 years ago)

Author:

dpranke@chromium.org

Message:

2010-10-04 Dirk Pranke <​dpranke@chromium.org>

Reviewed by Adam Barth.

aroben's change in r68792 actually broke new-run-webkit-tests when
running the DRT code path. His change was intended to fix the
way we were converting windows paths to URIs when running under
Cygwin (the paths were getting one too many "/" on the front).
However, the change ended up breaking the chromium_win port, which
had slightly different logic.

This patch removes the port-specific code and adds tests to make
sure we're getting the behavior we expect. The Port object no longer
exposes a get_absolute_path() method that can be used outside of
of converting test filenames, because it's unreliable otherwise
(we don't have the right context to know which conversion is intended).

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

• Scripts/webkitpy/layout_tests/port/base.py:
• Scripts/webkitpy/layout_tests/port/base_unittest.py:
• Scripts/webkitpy/layout_tests/port/chromium_win.py:
• Scripts/webkitpy/layout_tests/run_webkit_tests.py:
• Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:

Location:

trunk/WebKitTools

Files:

• ChangeLog (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/base.py (modified) (3 diffs)
• Scripts/webkitpy/layout_tests/port/base_unittest.py (modified) (1 diff)
• Scripts/webkitpy/layout_tests/port/chromium_win.py (modified) (1 diff)
• Scripts/webkitpy/layout_tests/run_webkit_tests.py (modified) (1 diff)
• Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py (modified) (1 diff)

trunk/WebKitTools/ChangeLog

@@ +1 @@
+2010-10-04  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        aroben's change in r68792 actually broke new-run-webkit-tests when
+        running the DRT code path. His change was intended to fix the
+        way we were converting windows paths to URIs when running under
+        Cygwin (the paths were getting one too many "/" on the front).
+        However, the change ended up breaking the chromium_win port, which
+        had slightly different logic.
+
+        This patch removes the port-specific code and adds tests to make
+        sure we're getting the behavior we expect. The Port object no longer
+        exposes a get_absolute_path() method that can be used outside of
+        of converting test filenames, because it's unreliable otherwise
+        (we don't have the right context to know which conversion is intended).
+
+        https://bugs.webkit.org/show_bug.cgi?id=47140
+
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
+
 2010-10-04  Dirk Pranke  <dpranke@chromium.org>
 

trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py

@@ -282 +282 @@
 
     def filename_to_uri(self, filename):
-        """Convert a test file to a URI."""
+        """Convert a test file (which is an absolute path) to a URI."""
         LAYOUTTEST_HTTP_DIR = "http/tests/"
         LAYOUTTEST_WEBSOCKET_DIR = "http/tests/websocket/tests/"
@@ -308 +308 @@
             return "%s://127.0.0.1:%u/%s" % (protocol, port, relative_path)
 
-        if sys.platform is 'win32':
-            return "file:///" + self.get_absolute_path(filename)
-        return "file://" + self.get_absolute_path(filename)
+        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
 
     def tests(self, paths):
@@ -378 +386 @@
 
         raise NotImplementedError('unknown url type: %s' % uri)
-
-    def get_absolute_path(self, filename):
-        """Return the absolute path in unix format for the given filename.
-
-        This routine exists so that platforms that don't use unix filenames
-        can convert accordingly."""
-        return os.path.abspath(filename)
 
     def layout_tests_dir(self):

trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py

@@ -227 +227 @@
         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)
+
+
+
 class VirtualTest(unittest.TestCase):
     """Tests that various methods expected to be virtual are."""

trunk/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py

@@ -84 +84 @@
                        'build-instructions-windows')
         return result
-
-    def get_absolute_path(self, filename):
-        """Return the absolute path in unix format for the given filename."""
-        abspath = os.path.abspath(filename)
-        return abspath.replace('\\', '/')
 
     def relative_test_filename(self, filename):

trunk/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py

@@ -1440 +1440 @@
         options.use_apache = sys.platform in ('darwin', 'linux2')
 
-    if options.results_directory.startswith("/"):
-        # Assume it's an absolute path and normalize.
-        options.results_directory = port_obj.get_absolute_path(
-            options.results_directory)
-    else:
-        # If it's a relative path, make the output directory relative to
-        # Debug or Release.
+    if not os.path.isabs(options.results_directory):
+        # This normalizes the path to the build dir.
+        # FIXME: how this happens is not at all obvious; this is a dumb
+        # interface and should be cleaned up.
         options.results_directory = port_obj.results_directory()
 

trunk/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py

@@ -234 +234 @@
         self.assertEqual(user.url, '/tmp/layout-test-results/results.html')
 
+    def test_results_directory_absolute(self):
+        # We run a configuration that should fail, to generate output, then
+        # look for what the output results url was.
+
+        res, out, err, user = logging_run(['--results-directory=/tmp-results'],
+                                          tests_included=True)
+        self.assertEqual(user.url, '/tmp-results/results.html')
+
+    def test_results_directory_default(self):
+        # We run a configuration that should fail, to generate output, then
+        # look for what the output results url was.
+
+        # This is the default location.
+        res, out, err, user = logging_run(tests_included=True)
+        self.assertEqual(user.url, '/tmp/layout-test-results/results.html')
+
+    def test_results_directory_relative(self):
+        # We run a configuration that should fail, to generate output, then
+        # look for what the output results url was.
+
+        res, out, err, user = logging_run(['--results-directory=foo'],
+                                          tests_included=True)
+        self.assertEqual(user.url, '/tmp/foo/results.html')
+
+
 
 def _mocked_open(original_open, file_list):