2010-10-04 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Oct 2010 04:19:16 +0000 (04:19 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 5 Oct 2010 04:19:16 +0000 (04:19 +0000)
        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:

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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py

index 3e8be7140f0fc4d75ea5185cccebfdbaecdc4d13..632a6e2724f58b7b97b10e54c8a9f9125a4338ac 100644 (file)
@@ -1,3 +1,28 @@
+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>
 
         Unreviewed, build fix.
index f3db68367089dbe3195f4735669a7160c1ac5c7a..4073b82df23b4370ed4797b8805dcbff73b8ea14 100644 (file)
@@ -281,7 +281,7 @@ class Port(object):
         return text.strip("\r\n").replace("\r\n", "\n") + "\n"
 
     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/"
 
@@ -307,9 +307,17 @@ class Port(object):
                 protocol = "http"
             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):
         """Return the list of tests found (relative to layout_tests_dir()."""
@@ -378,13 +386,6 @@ class Port(object):
 
         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):
         """Return the absolute path to the top of the LayoutTests directory."""
         return self.path_from_webkit_base('LayoutTests')
index 71877b3e2e46dba61833a7e4f9dd4b3da29a416d..e66c64dbe87dd47eb86117dac28de43e644518a3 100644 (file)
@@ -226,6 +226,29 @@ class PortTest(unittest.TestCase):
         self.assertTrue('canvas' in dirs)
         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."""
     def assertVirtual(self, method, *args, **kwargs):
index d2b02650b2e9bd7d2c55df1dce675ac8780bc9d7..67d2ab2f1dc9074fdaa627ec2a4d09f33b8cb560 100644 (file)
@@ -84,11 +84,6 @@ class ChromiumWinPort(chromium.ChromiumPort):
                        '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):
         path = filename[len(self.layout_tests_dir()) + 1:]
         return path.replace('\\', '/')
index 31a42f3c8c63bc3f5c88d6ec4d1967af6495d335..2868a5ed0e71aac6a2c64223fce31f5074f5f95b 100755 (executable)
@@ -1439,13 +1439,10 @@ def _set_up_derived_options(port_obj, options):
     if not options.use_apache:
         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()
 
     if not options.time_out_ms:
index b350cb42988d32fec36d991703bb45cca878cb8d..8bb90e84d5d5d56d131ef32dd81c416f1a34c630 100644 (file)
@@ -233,6 +233,31 @@ class MainTest(unittest.TestCase):
         self.assertFalse(err.empty())
         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):
     def _wrapper(name, mode, encoding):