2010-10-07 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Oct 2010 00:48:39 +0000 (00:48 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 8 Oct 2010 00:48:39 +0000 (00:48 +0000)
        Reviewed by Adam Barth.

        rebaseline-chromium-webkit-tests relied on the filename_to_uri()
        hook in the Port infrastructure to generate URIs for the files
        in its summary HTML report; however, that method is supposed to only
        be used for test files (and should really be renamed), so this would
        crash.

        This change adds a new "path" module to the system package with a
        routine called abspath_to_uri() that handles converting paths to
        file: URIs independently of anything in the layout_tests package,
        and changes the code to use this. At some point in the near future
        the layout_tests/port/* code should use this as well.

        This change also deletes a bunch of unused code and fixes some
        comments in rebaseline_chromium_webkit_tests.py.

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

        * Scripts/webkitpy/common/system/path.py: Added.
        * Scripts/webkitpy/common/system/path_unittest.py: Added.
        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py:

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

WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/common/system/path.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/common/system/path_unittest.py [new file with mode: 0644]
WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py

index 9108ea135b76f7a224f69a93129af5009af87eeb..0bdeb801b932374852ff81ad53c6ceacf9caa66d 100644 (file)
@@ -1,3 +1,29 @@
+2010-10-07  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Adam Barth.
+
+        rebaseline-chromium-webkit-tests relied on the filename_to_uri()
+        hook in the Port infrastructure to generate URIs for the files
+        in its summary HTML report; however, that method is supposed to only
+        be used for test files (and should really be renamed), so this would
+        crash.
+
+        This change adds a new "path" module to the system package with a
+        routine called abspath_to_uri() that handles converting paths to
+        file: URIs independently of anything in the layout_tests package,
+        and changes the code to use this. At some point in the near future
+        the layout_tests/port/* code should use this as well.
+
+        This change also deletes a bunch of unused code and fixes some
+        comments in rebaseline_chromium_webkit_tests.py.
+
+        https://bugs.webkit.org/show_bug.cgi?id=47319
+
+        * Scripts/webkitpy/common/system/path.py: Added.
+        * Scripts/webkitpy/common/system/path_unittest.py: Added.
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py:
+
 2010-10-07  Martin Robinson  <mrobinson@igalia.com>
 
         Reviewed by Gustavo Noronha Silva.
diff --git a/WebKitTools/Scripts/webkitpy/common/system/path.py b/WebKitTools/Scripts/webkitpy/common/system/path.py
new file mode 100644 (file)
index 0000000..937247e
--- /dev/null
@@ -0,0 +1,72 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#     * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#     * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#     * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+"""generic routines to convert platform-specific paths to URIs."""
+import sys
+import urllib
+
+
+def abspath_to_uri(path, executive, 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))
+
+
+def _escape(path):
+    """Handle any characters in the path that should be escaped."""
+    # FIXME: web browsers don't appear to blindly quote every character
+    # when converting filenames to files. Instead of using urllib's default
+    # rules, we allow a small list of other characters through un-escaped.
+    # It's unclear if this is the best possible solution.
+    return urllib.quote(path, safe='/+:')
+
+
+def _convert_path(path, executive, 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 _unixypath_to_uri(path)
+
+
+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
diff --git a/WebKitTools/Scripts/webkitpy/common/system/path_unittest.py b/WebKitTools/Scripts/webkitpy/common/system/path_unittest.py
new file mode 100644 (file)
index 0000000..3c1021d
--- /dev/null
@@ -0,0 +1,100 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#    * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#    * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#    * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+
+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),
+                         expected_uri)
+
+    def test_abspath_to_uri_cygwin(self):
+        self.assertMatch('/cygdrive/c/foo/bar.html',
+                         'file:///c:/foo/bar.html',
+                         platform='cygwin',
+                         cygpath_result='c:\\foo\\bar.html\n')
+        self.assertEqual(path.abspath_to_uri('/cygdrive/c/foo/bar.html',
+                                             executive=self,
+                                             platform='cygwin'),
+                         '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")
+
+    def test_abspath_to_uri_linux2(self):
+        self.assertMatch('/foo/bar.html',
+                         'file:///foo/bar.html',
+                         platform='darwin')
+        self.assertEqual(path.abspath_to_uri("/foo/bar.html",
+                                             executive=self,
+                                             platform='linux2'),
+                         "file:///foo/bar.html")
+
+    def test_abspath_to_uri_win(self):
+        self.assertMatch('c:\\foo\\bar.html',
+                         '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")
+
+    def test_abspath_to_uri_escaping(self):
+        self.assertMatch('/foo/bar + baz%?.html',
+                         'file:///foo/bar%20+%20baz%25%3F.html',
+                         platform='darwin')
+        self.assertMatch('/foo/bar + baz%?.html',
+                         'file:///foo/bar%20+%20baz%25%3F.html',
+                         platform='linux2')
+
+        # 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')
+
+
+if __name__ == '__main__':
+    unittest.main()
index e57ceb21700e5afb7f5ec03315d284241bb86fed..c20b90b9cdccc24e931ba57c5ea03e88c5a858df 100644 (file)
@@ -57,8 +57,9 @@ import time
 import urllib
 import zipfile
 
+from webkitpy.common.system import path
 from webkitpy.common.system import user
-from webkitpy.common.system.executive import run_command, ScriptError
+from webkitpy.common.system.executive import Executive, ScriptError
 import webkitpy.common.checkout.scm as scm
 
 import port
@@ -81,58 +82,6 @@ ARCHIVE_DIR_NAME_DICT = {'win': 'webkit-rel',
                          'linux-canary': 'webkit-rel-linux-webkit-org'}
 
 
-# FIXME: Should be rolled into webkitpy.Executive
-def run_shell_with_return_code(command, print_output=False):
-    """Executes a command and returns the output and process return code.
-
-    Args:
-      command: program and arguments.
-      print_output: if true, print the command results to standard output.
-
-    Returns:
-      command output, return code
-    """
-
-    # Use a shell for subcommands on Windows to get a PATH search.
-    # FIXME: shell=True is a trail of tears, and should be removed.
-    use_shell = sys.platform.startswith('win')
-    # Note: Not thread safe: http://bugs.python.org/issue2320
-    p = subprocess.Popen(command, stdout=subprocess.PIPE,
-                         stderr=subprocess.STDOUT, shell=use_shell)
-    if print_output:
-        output_array = []
-        while True:
-            line = p.stdout.readline()
-            if not line:
-                break
-            if print_output:
-                print line.strip('\n')
-            output_array.append(line)
-        output = ''.join(output_array)
-    else:
-        output = p.stdout.read()
-    p.wait()
-    p.stdout.close()
-
-    return output, p.returncode
-
-
-# FIXME: Should be rolled into webkitpy.Executive
-def run_shell(command, print_output=False):
-    """Executes a command and returns the output.
-
-    Args:
-      command: program and arguments.
-      print_output: if true, print the command results to standard output.
-
-    Returns:
-      command output
-    """
-
-    output, return_code = run_shell_with_return_code(command, print_output)
-    return output
-
-
 def log_dashed_string(text, platform, logging_level=logging.INFO):
     """Log text message with dashes on both sides."""
 
@@ -628,7 +577,6 @@ class Rebaseliner(object):
         base_file = get_result_file_fullpath(self._options.html_directory,
                                              baseline_filename, self._platform,
                                              'old')
-        # FIXME: This assumes run_shell returns a byte array.
         # We should be using an explicit encoding here.
         with open(base_file, "wb") as file:
             file.write(output)
@@ -642,7 +590,6 @@ class Rebaseliner(object):
                 diff_file = get_result_file_fullpath(
                     self._options.html_directory, baseline_filename,
                     self._platform, 'diff')
-                # FIXME: This assumes run_shell returns a byte array, not unicode()
                 with open(diff_file, 'wb') as file:
                     file.write(output)
                 _log.info('  Html: created baseline diff file: "%s".',
@@ -694,14 +641,20 @@ class HtmlGenerator(object):
                         '<img style="width: 200" src="%(uri)s" /></a></td>')
     HTML_TR = '<tr>%s</tr>'
 
-    def __init__(self, target_port, options, platforms, rebaselining_tests):
+    def __init__(self, target_port, options, platforms, rebaselining_tests,
+                 executive):
         self._html_directory = options.html_directory
         self._target_port = target_port
         self._platforms = platforms
         self._rebaselining_tests = rebaselining_tests
+        self._executive = executive
         self._html_file = os.path.join(options.html_directory,
                                        'rebaseline.html')
 
+    def abspath_to_uri(self, filename):
+        """Converts an absolute path to a file: URI."""
+        return path.abspath_to_uri(filename, self._executive)
+
     def generate_html(self):
         """Generate html file for rebaselining result comparison."""
 
@@ -769,14 +722,13 @@ class HtmlGenerator(object):
         links = ''
         if os.path.exists(old_file):
             links += html_td_link % {
-                'uri': self._target_port.filename_to_uri(old_file),
+                'uri': self.abspath_to_uri(old_file),
                 'name': baseline_filename}
         else:
             _log.info('    No old baseline file: "%s"', old_file)
             links += self.HTML_TD_NOLINK % ''
 
-        links += html_td_link % {'uri': self._target_port.filename_to_uri(
-                                     new_file),
+        links += html_td_link % {'uri': self.abspath_to_uri(new_file),
                                  'name': baseline_filename}
 
         diff_file = get_result_file_fullpath(self._html_directory,
@@ -784,8 +736,8 @@ class HtmlGenerator(object):
                                              'diff')
         _log.info('    Baseline diff file: "%s"', diff_file)
         if os.path.exists(diff_file):
-            links += html_td_link % {'uri': self._target_port.filename_to_uri(
-                diff_file), 'name': 'Diff'}
+            links += html_td_link % {'uri': self.abspath_to_uri(diff_file),
+                                     'name': 'Diff'}
         else:
             _log.info('    No baseline diff file: "%s"', diff_file)
             links += self.HTML_TD_NOLINK % ''
@@ -825,8 +777,7 @@ class HtmlGenerator(object):
         if rows:
             test_path = os.path.join(self._target_port.layout_tests_dir(),
                                      test)
-            html = self.HTML_TR_TEST % (
-                self._target_port.filename_to_uri(test_path), test)
+            html = self.HTML_TR_TEST % (self.abspath_to_uri(test_path), test)
             html += self.HTML_TEST_DETAIL % ' '.join(rows)
 
             _log.debug('    html for test: %s', html)
@@ -867,7 +818,7 @@ def get_host_port_object(options):
     return port_obj
 
 
-def main():
+def main(executive=Executive()):
     """Main function to produce new baselines."""
 
     option_parser = optparse.OptionParser()
@@ -992,7 +943,8 @@ def main():
     html_generator = HtmlGenerator(target_port_obj,
                                    options,
                                    rebaseline_platforms,
-                                   rebaselining_tests)
+                                   rebaselining_tests,
+                                   executive=executive)
     html_generator.generate_html()
     if not options.quiet:
         html_generator.show_html()
index 9ba3d6bd37ad42d8198978366d60f7bf5d3ad167..fce6e82caef835a73398084b69148008db26ae68 100644 (file)
 """Unit tests for rebaseline_chromium_webkit_tests.py."""
 
 import os
+import sys
 import unittest
 
 from webkitpy.layout_tests import port
 from webkitpy.layout_tests import rebaseline_chromium_webkit_tests
+from webkitpy.common.system.executive import Executive, ScriptError
 
 
 class MockPort(object):
@@ -45,8 +47,9 @@ class MockPort(object):
 
 
 class MockOptions(object):
-    def __init__(self):
-        self.configuration = None
+    def __init__(self, configuration=None, html_directory=None):
+        self.configuration = configuration
+        self.html_directory = html_directory
 
 
 def get_mock_get(config_expectations):
@@ -118,5 +121,31 @@ class TestRebaseliner(unittest.TestCase):
         self.assertFalse(rebaseliner._diff_baselines(image, image,
                                                      is_image=True))
 
+
+class TestHtmlGenerator(unittest.TestCase):
+    def make_generator(self, tests):
+        return rebaseline_chromium_webkit_tests.HtmlGenerator(
+            target_port=None,
+            options=MockOptions(html_directory="/tmp"),
+            platforms=['mac'],
+            rebaselining_tests=tests,
+            executive=Executive())
+
+    def test_generate_baseline_links(self):
+        orig_platform = sys.platform
+        orig_exists = os.path.exists
+
+        try:
+            sys.platform = 'darwin'
+            os.path.exists = lambda x: True
+            generator = self.make_generator(["foo.txt"])
+            links = generator._generate_baseline_links("foo", ".txt", "mac")
+            expected_links = '<td align=center><a href="file:///tmp/foo-expected-mac-old.txt">foo-expected.txt</a></td><td align=center><a href="file:///tmp/foo-expected-mac-new.txt">foo-expected.txt</a></td><td align=center><a href="file:///tmp/foo-expected-mac-diff.txt">Diff</a></td>'
+            self.assertEqual(links, expected_links)
+        finally:
+            sys.platform = orig_platform
+            os.path.exists = orig_exists
+
+
 if __name__ == '__main__':
     unittest.main()