2011-01-18 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Jan 2011 21:10:33 +0000 (21:10 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 18 Jan 2011 21:10:33 +0000 (21:10 +0000)
        Reviewed by Mihai Parparita.

        Update rebaseline-chromium-webkit-tests to use filesystem objects
        instead of direct references to os.path, shutil, tempfile, etc.

        This patch doesn't change anything, but will allow subsequent
        patches to change the unit tests to no longer use the real
        filesystem.

        This patch adds a bunch more methods to the filesystem object as
        well.

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

        * Scripts/webkitpy/common/system/filesystem.py:
        * Scripts/webkitpy/common/system/filesystem_mock.py:
        * Scripts/webkitpy/tool/commands/queues.py:
        * 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@76050 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/common/system/filesystem.py
Tools/Scripts/webkitpy/common/system/filesystem_mock.py
Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py
Tools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py
Tools/Scripts/webkitpy/tool/commands/queues.py

index 2a97e83a529b644f8cd281ca51a411b769cff54d..3a011c7539dba1c560fd148ae8e1b521406ced74 100644 (file)
@@ -1,3 +1,25 @@
+2011-01-18  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Mihai Parparita.
+
+        Update rebaseline-chromium-webkit-tests to use filesystem objects
+        instead of direct references to os.path, shutil, tempfile, etc.
+
+        This patch doesn't change anything, but will allow subsequent
+        patches to change the unit tests to no longer use the real
+        filesystem.
+
+        This patch adds a bunch more methods to the filesystem object as
+        well.
+
+        https://bugs.webkit.org/show_bug.cgi?id=52482
+
+        * Scripts/webkitpy/common/system/filesystem.py:
+        * Scripts/webkitpy/common/system/filesystem_mock.py:
+        * Scripts/webkitpy/tool/commands/queues.py:
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py:
+
 2011-01-18  Dirk Pranke  <dpranke@chromium.org>
 
         Reviewed by Mihai Parparita.
index 527b6bdea5ad68da2530f3069fe7f87bf2eb6eb2..53e9796f15e357fff21925298444077ffdb2579a 100644 (file)
@@ -44,10 +44,27 @@ class FileSystem(object):
     Unless otherwise noted, all paths are allowed to be either absolute
     or relative."""
 
+    def basename(self, path):
+        return os.path.basename(path)
+
+    def copyfile(self, source, destination):
+        """Copies the contents of the file at the given path to the destination
+        path."""
+        shutil.copyfile(source, destination)
+
+    def dirname(self, path):
+        return os.path.dirname(path)
+
     def exists(self, path):
         """Return whether the path exists in the filesystem."""
         return os.path.exists(path)
 
+    def files_under(self, path):
+        """Return the list of all files under the given path."""
+        return [self.join(path_to_file, filename)
+            for (path_to_file, _, filenames) in os.walk(path)
+            for filename in filenames]
+
     def isfile(self, path):
         """Return whether the path refers to a file."""
         return os.path.isfile(path)
@@ -71,14 +88,20 @@ class FileSystem(object):
         the directory will self-delete at the end of the block (if the
         directory is empty; non-empty directories raise errors). The
         directory can be safely deleted inside the block as well, if so
-        desired."""
+        desired.
+
+        Note that the object returned is not a string and does not support all of the string
+        methods. If you need a string, coerce the object to a string and go from there.
+        """
         class TemporaryDirectory(object):
             def __init__(self, **kwargs):
                 self._kwargs = kwargs
-                self._directory_path = None
+                self._directory_path = tempfile.mkdtemp(**self._kwargs)
+
+            def __str__(self):
+                return self._directory_path
 
             def __enter__(self):
-                self._directory_path = tempfile.mkdtemp(**self._kwargs)
                 return self._directory_path
 
             def __exit__(self, type, value, traceback):
@@ -98,6 +121,30 @@ class FileSystem(object):
             if e.errno != errno.EEXIST:
                 raise
 
+    def move(self, src, dest):
+        shutil.move(src, dest)
+
+    def normpath(self, path):
+        return os.path.normpath(path)
+
+    def open_binary_tempfile(self, suffix):
+        """Create, open, and return a binary temp file. Returns a tuple of the file and the name."""
+        temp_fd, temp_name = tempfile.mkstemp(suffix)
+        f = os.fdopen(temp_fd, 'wb')
+        return f, temp_name
+
+    def read_binary_file(self, path):
+        """Return the contents of the file at the given path as a byte string."""
+        with file(path, 'rb') as f:
+            return f.read()
+
+    def read_text_file(self, path):
+        """Return the contents of the file at the given path as a Unicode string.
+
+        The file is read assuming it is a UTF-8 encoded file with no BOM."""
+        with codecs.open(path, 'r', 'utf8') as f:
+            return f.read()
+
     class _WindowsError(exceptions.OSError):
         """Fake exception for Linux and Mac."""
         pass
@@ -124,8 +171,9 @@ class FileSystem(object):
                 if retry_timeout_sec < 0:
                     raise e
 
-    def remove_tree(self, path, ignore_errors=False):
-        shutil.rmtree(path, ignore_errors)
+    def rmtree(self, path):
+        """Delete the directory rooted at path, empty or no."""
+        shutil.rmtree(path, ignore_errors=True)
 
     def read_binary_file(self, path):
         """Return the contents of the file at the given path as a byte string."""
@@ -139,6 +187,10 @@ class FileSystem(object):
         with codecs.open(path, 'r', 'utf8') as f:
             return f.read()
 
+    def splitext(self, path):
+        """Return (dirname, basename + ext)."""
+        return os.path.splitext(path)
+
     def write_binary_file(self, path, contents):
         """Write the contents to the file at the given location."""
         with file(path, 'wb') as f:
@@ -150,14 +202,3 @@ class FileSystem(object):
         The file is written encoded as UTF-8 with no BOM."""
         with codecs.open(path, 'w', 'utf8') as f:
             f.write(contents)
-
-    def copyfile(self, source, destination):
-        """Copies the contents of the file at the given path to the destination
-        path."""
-        shutil.copyfile(source, destination)
-
-    def files_under(self, path):
-        """Return the list of all files under the given path."""
-        return [self.join(path_to_file, filename)
-            for (path_to_file, _, filenames) in os.walk(path)
-            for filename in filenames]
index 809c4c6fbaabac056565a7d63e6cbc9353b9b1fc..10acc3b42deb50e54932c1c67a4723ddfe83284a 100644 (file)
@@ -43,10 +43,39 @@ class MockFileSystem(object):
                 not exist.
         """
         self.files = files or {}
+        self._current_tmpno = 0
+
+    def _raise_not_found(self, path):
+        raise IOError(errno.ENOENT, path, os.strerror(errno.ENOENT))
+
+    def _split(self, path):
+        idx = path.rfind('/')
+        return (path[0:idx], path[idx + 1:])
+
+    def basename(self):
+        return self._split(path)[1]
+
+    def copyfile(self, source, destination):
+        if not self.exists(source):
+            self._raise_not_found(source)
+        if self.isdir(source):
+            raise IOError(errno.EISDIR, source, os.strerror(errno.ISDIR))
+        if self.isdir(destination):
+            raise IOError(errno.EISDIR, destination, os.strerror(errno.ISDIR))
+
+        self.files[destination] = self.files[source]
+
+    def dirname(self, path):
+        return self._split(path)[0]
 
     def exists(self, path):
         return self.isfile(path) or self.isdir(path)
 
+    def files_under(self, path):
+        if not path.endswith('/'):
+            path += '/'
+        return [file for file in self.files if file.startswith(path)]
+
     def isfile(self, path):
         return path in self.files and self.files[path] is not None
 
@@ -80,42 +109,105 @@ class MockFileSystem(object):
                     files.append(remaining)
         return dirs + files
 
+    def _mktemp(self, suffix='', prefix='tmp', dir=None, **kwargs):
+        if dir is None:
+            dir = '/__im_tmp'
+        curno = self.current_tmpno
+        self.current_tmpno += 1
+        return self.join(dir, "%s_%u_%s" % (prefix, curno, suffix))
+
+    def mkdtemp(self, **kwargs):
+        class TemporaryDirectory(object):
+            def __init__(self, fs, **kwargs):
+                self._kwargs = kwargs
+                self._filesystem = fs
+                self._directory_path = fs._mktemp(**kwargs)
+                fs.maybe_make_directory(self._directory_path)
+
+            def __str__(self):
+                return self._directory_path
+
+            def __enter__(self):
+                return self._directory_path
+
+            def __exit__(self, type, value, traceback):
+                # Only self-delete if necessary.
+
+                # FIXME: Should we delete non-empty directories?
+                if self._filesystem.exists(self._directory_path):
+                    self._filesystem.rmdir(self._directory_path)
+
+        return TemporaryDirectory(fs=self, **kwargs)
+
     def maybe_make_directory(self, *path):
         # FIXME: Implement such that subsequent calls to isdir() work?
         pass
 
+    def move(self, src, dst):
+        if self.files[src] is None:
+            self._raise_not_found(src)
+        self.files[dst] = self.files[src]
+        self.files[src] = None
+
+    def normpath(self, path):
+        return path
+
+    def open_binary_tempfile(self, suffix):
+        path = self._mktemp(suffix)
+        return WritableFileObject(self, path), path
+
     def read_text_file(self, path):
         return self.read_binary_file(path)
 
     def read_binary_file(self, path):
         if path in self.files:
             if self.files[path] is None:
-                raise IOError(errno.ENOENT, path, os.strerror(errno.ENOENT))
+                self._raise_not_found(path)
             return self.files[path]
 
+    def remove(self, path):
+        if self.files[path] is None:
+            self._raise_not_found(path)
+        self.files[path] = None
+
+    def rmtree(self, path):
+        if not path.endswith('/'):
+            path += '/'
+
+        for f in self.files:
+            if f.startswith(path):
+                self.files[f] = None
+
+    def splitext(self, path):
+        idx = path.rfind('.')
+        if idx == -1:
+            idx = 0
+        return (path[0:idx], path[idx:])
+
     def write_text_file(self, path, contents):
         return self.write_binary_file(path, contents)
 
     def write_binary_file(self, path, contents):
         self.files[path] = contents
 
-    def copyfile(self, source, destination):
-        if not self.exists(source):
-            raise IOError(errno.ENOENT, source, os.strerror(errno.ENOENT))
-        if self.isdir(source):
-            raise IOError(errno.EISDIR, source, os.strerror(errno.ISDIR))
-        if self.isdir(destination):
-            raise IOError(errno.EISDIR, destination, os.strerror(errno.ISDIR))
 
-        self.files[destination] = self.files[source]
+class WritableFileObject(object):
+    def __init__(self, fs, path, append=False, encoding=None):
+        self.fs = fs
+        self.name = name
+        self.closed = False
+        if path not in self.fs.files or not append:
+            self.fs.files[path] = ""
 
-    def files_under(self, path):
-        if not path.endswith('/'):
-            path += '/'
-        return [file for file in self.files if file.startswith(path)]
+    def __enter__(self):
+        return self
 
-    def remove(self, path):
-        del self.files[path]
+    def __exit__(self, type, value, traceback):
+        self.close()
+
+    def close(self):
+        self.closed = True
 
-    def remove_tree(self, path, ignore_errors=False):
-        self.files = [file for file in self.files if not file.startswith(path)]
+    def write(self, str):
+        self.fs.files[self.name] += str
+        self.fs.written_files[self.name] = self.fs.files[self.name]
index 4d8b7c9f5a2bd4c74ed56fd6312ba195c16c8a6d..7641f55d2fa266d35ed1621463fa376e21a94955 100644 (file)
@@ -41,26 +41,18 @@ The script does the following for each platform specified:
 At the end, the script generates a html that compares old and new baselines.
 """
 
-from __future__ import with_statement
-
-import codecs
 import copy
 import logging
 import optparse
-import os
 import re
-import shutil
-import subprocess
 import sys
-import tempfile
 import time
 import urllib
 import zipfile
 
+from webkitpy.common.checkout import scm
 from webkitpy.common.system import path
-from webkitpy.common.system import user
-from webkitpy.common.system.executive import Executive, ScriptError
-import webkitpy.common.checkout.scm as scm
+from webkitpy.common.system.executive import ScriptError
 
 import port
 from layout_package import test_expectations
@@ -100,41 +92,35 @@ def log_dashed_string(text, platform, logging_level=logging.INFO):
         _log.info(msg)
 
 
-def setup_html_directory(html_directory):
+def setup_html_directory(filesystem, parent_directory):
     """Setup the directory to store html results.
 
-       All html related files are stored in the "rebaseline_html" subdirectory.
-
-    Args:
-      html_directory: parent directory that stores the rebaselining results.
-                      If None, a temp directory is created.
-
-    Returns:
-      the directory that stores the html related rebaselining results.
+       All html related files are stored in the "rebaseline_html" subdirectory of
+       the parent directory. The path to the created directory is returned.
     """
 
-    if not html_directory:
-        html_directory = tempfile.mkdtemp()
-    elif not os.path.exists(html_directory):
-        os.mkdir(html_directory)
+    if not parent_directory:
+        parent_directory = str(filesystem.mkdtemp())
+    else:
+        filesystem.maybe_make_directory(parent_directory)
 
-    html_directory = os.path.join(html_directory, 'rebaseline_html')
+    html_directory = filesystem.join(parent_directory, 'rebaseline_html')
     _log.info('Html directory: "%s"', html_directory)
 
-    if os.path.exists(html_directory):
-        shutil.rmtree(html_directory, True)
-        _log.info('Deleted file at html directory: "%s"', html_directory)
+    if filesystem.exists(html_directory):
+        filesystem.rmtree(html_directory)
+        _log.info('Deleted html directory: "%s"', html_directory)
 
-    if not os.path.exists(html_directory):
-        os.mkdir(html_directory)
+    filesystem.maybe_make_directory(html_directory)
     return html_directory
 
 
-def get_result_file_fullpath(html_directory, baseline_filename, platform,
+def get_result_file_fullpath(filesystem, html_directory, baseline_filename, platform,
                              result_type):
     """Get full path of the baseline result file.
 
     Args:
+      filesystem: wrapper object
       html_directory: directory that stores the html related files.
       baseline_filename: name of the baseline file.
       platform: win, linux or mac
@@ -144,9 +130,9 @@ def get_result_file_fullpath(html_directory, baseline_filename, platform,
       Full path of the baseline file for rebaselining result comparison.
     """
 
-    base, ext = os.path.splitext(baseline_filename)
+    base, ext = filesystem.splitext(baseline_filename)
     result_filename = '%s-%s-%s%s' % (base, platform, result_type, ext)
-    fullpath = os.path.join(html_directory, result_filename)
+    fullpath = filesystem.join(html_directory, result_filename)
     _log.debug('  Result file full path: "%s".', fullpath)
     return fullpath
 
@@ -168,6 +154,7 @@ class Rebaseliner(object):
         self._platform = platform
         self._options = options
         self._port = running_port
+        self._filesystem = running_port._filesystem
         self._target_port = target_port
         self._rebaseline_port = port.get(
             self._target_port.test_platform_name_to_name(platform), options)
@@ -370,7 +357,7 @@ class Rebaseliner(object):
 
             found = False
             scm_error = False
-            test_basename = os.path.splitext(test)[0]
+            test_basename = self._filesystem.splitext(test)[0]
             for suffix in BASELINE_SUFFIXES:
                 archive_test_name = ('layout-test-results/%s-actual%s' %
                                       (test_basename, suffix))
@@ -385,15 +372,14 @@ class Rebaseliner(object):
 
                 # Extract new baseline from archive and save it to a temp file.
                 data = zip_file.read(archive_test_name)
-                temp_fd, temp_name = tempfile.mkstemp(suffix)
-                f = os.fdopen(temp_fd, 'wb')
-                f.write(data)
-                f.close()
+                tempfile, temp_name = self._filesystem.open_binary_tempfile(suffix)
+                tempfile.write(data)
+                tempfile.close()
 
                 expected_filename = '%s-expected%s' % (test_basename, suffix)
-                expected_fullpath = os.path.join(
+                expected_fullpath = self._filesystem.join(
                     self._rebaseline_port.baseline_path(), expected_filename)
-                expected_fullpath = os.path.normpath(expected_fullpath)
+                expected_fullpath = self._filesystem.normpath(expected_fullpath)
                 _log.debug('  Expected file full path: "%s"',
                            expected_fullpath)
 
@@ -407,16 +393,13 @@ class Rebaseliner(object):
                                         test,
                                         suffix,
                                         self._platform):
-                    os.remove(temp_name)
+                    self._filesystem.remove(temp_name)
                     self._delete_baseline(expected_fullpath)
                     continue
 
-                # Create the new baseline directory if it doesn't already
-                # exist.
-                self._port.maybe_make_directory(
-                    os.path.dirname(expected_fullpath))
+                self._filesystem.maybe_make_directory(self._filesystem.dirname(expected_fullpath))
 
-                shutil.move(temp_name, expected_fullpath)
+                self._filesystem.move(temp_name, expected_fullpath)
 
                 if 0 != self._scm.add(expected_fullpath, return_exit_code=True):
                     # FIXME: print detailed diagnose messages
@@ -436,7 +419,7 @@ class Rebaseliner(object):
             test_no += 1
 
         zip_file.close()
-        os.remove(archive_file)
+        self._filesystem.remove(archive_file)
 
         return self._rebaselined_tests
 
@@ -458,21 +441,17 @@ class Rebaseliner(object):
           True if the baseline is unnecessary.
           False otherwise.
         """
-        test_filepath = os.path.join(self._target_port.layout_tests_dir(),
+        test_filepath = self._filesystem.join(self._target_port.layout_tests_dir(),
                                      test)
         all_baselines = self._rebaseline_port.expected_baselines(
             test_filepath, suffix, True)
         for (fallback_dir, fallback_file) in all_baselines:
             if fallback_dir and fallback_file:
-                fallback_fullpath = os.path.normpath(
-                    os.path.join(fallback_dir, fallback_file))
+                fallback_fullpath = self._filesystem.normpath(
+                    self._filesystem.join(fallback_dir, fallback_file))
                 if fallback_fullpath.lower() != baseline_path.lower():
-                    with codecs.open(new_baseline, "r",
-                                     None) as file_handle1:
-                        new_output = file_handle1.read()
-                    with codecs.open(fallback_fullpath, "r",
-                                     None) as file_handle2:
-                        fallback_output = file_handle2.read()
+                    new_output = self._filesystem.read_binary_file(new_baseline)
+                    fallback_output = self._filesystem.read_binary_file(fallback_fullpath)
                     is_image = baseline_path.lower().endswith('.png')
                     if not self._diff_baselines(new_output, fallback_output,
                                                 is_image):
@@ -507,7 +486,7 @@ class Rebaseliner(object):
           filename: full path of the file to delete.
         """
 
-        if not filename or not os.path.isfile(filename):
+        if not filename or not self._filesystem.isfile(filename):
             return
         self._scm.delete(filename)
 
@@ -530,14 +509,12 @@ class Rebaseliner(object):
                 date_suffix = time.strftime('%Y%m%d%H%M%S',
                                             time.localtime(time.time()))
                 backup_file = ('%s.orig.%s' % (path, date_suffix))
-                if os.path.exists(backup_file):
-                    os.remove(backup_file)
+                if self._filesystem.exists(backup_file):
+                    self._filesystem.remove(backup_file)
                 _log.info('Saving original file to "%s"', backup_file)
-                os.rename(path, backup_file)
-            # FIXME: What encoding are these files?
-            # Or is new_expectations always a byte array?
-            with open(path, "w") as file:
-                file.write(new_expectations)
+                self._filesystem.move(path, backup_file)
+
+            self._filesystem.write_text_file(path, new_expectations)
             # self._scm.add(path)
         else:
             _log.info('No test was rebaselined so nothing to remove.')
@@ -551,15 +528,15 @@ class Rebaseliner(object):
           baseline_fullpath: full path of the expected baseline file.
         """
 
-        if not baseline_fullpath or not os.path.exists(baseline_fullpath):
+        if not baseline_fullpath or not self._filesystem.exists(baseline_fullpath):
             return
 
         # Copy the new baseline to html directory for result comparison.
-        baseline_filename = os.path.basename(baseline_fullpath)
-        new_file = get_result_file_fullpath(self._options.html_directory,
+        baseline_filename = self._filesystem.basename(baseline_fullpath)
+        new_file = get_result_file_fullpath(self._filesystem, self._options.html_directory,
                                             baseline_filename, self._platform,
                                             'new')
-        shutil.copyfile(baseline_fullpath, new_file)
+        self._filesystem.copyfile(baseline_fullpath, new_file)
         _log.info('  Html: copied new baseline file from "%s" to "%s".',
                   baseline_fullpath, new_file)
 
@@ -574,7 +551,7 @@ class Rebaseliner(object):
             'NO SUCH FILE OR DIRECTORY')):
             _log.info('  No base file: "%s"', baseline_fullpath)
             return
-        base_file = get_result_file_fullpath(self._options.html_directory,
+        base_file = get_result_file_fullpath(self._filesystem, self._options.html_directory,
                                              baseline_filename, self._platform,
                                              'old')
         # We should be using an explicit encoding here.
@@ -587,7 +564,7 @@ class Rebaseliner(object):
         if baseline_filename.upper().endswith('.TXT'):
             output = self._scm.diff_for_file(baseline_fullpath, log=_log)
             if output:
-                diff_file = get_result_file_fullpath(
+                diff_file = get_result_file_fullpath(self._filesystem,
                     self._options.html_directory, baseline_filename,
                     self._platform, 'diff')
                 with open(diff_file, 'wb') as file:
@@ -642,19 +619,19 @@ 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,
-                 executive):
+    def __init__(self, port, target_port, options, platforms, rebaselining_tests):
         self._html_directory = options.html_directory
+        self._port = port
         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')
+        self._filesystem = port._filesystem
+        self._html_file = self._filesystem.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)
+        return path.abspath_to_uri(filename, self._port._executive)
 
     def generate_html(self):
         """Generate html file for rebaselining result comparison."""
@@ -677,9 +654,7 @@ class HtmlGenerator(object):
                                         'body': html_body})
         _log.debug(html)
 
-        with codecs.open(self._html_file, "w", "utf-8") as file:
-            file.write(html)
-
+        self._filesystem.write_text_file(self._html_file, html)
         _log.info('Baseline comparison html generated at "%s"',
                   self._html_file)
 
@@ -687,7 +662,7 @@ class HtmlGenerator(object):
         """Launch the rebaselining html in brwoser."""
 
         _log.info('Launching html: "%s"', self._html_file)
-        user.User().open_url(self._html_file)
+        self._port._user.open_url(self._html_file)
         _log.info('Html launched.')
 
     def _generate_baseline_links(self, test_basename, suffix, platform):
@@ -705,14 +680,14 @@ class HtmlGenerator(object):
         baseline_filename = '%s-expected%s' % (test_basename, suffix)
         _log.debug('    baseline filename: "%s"', baseline_filename)
 
-        new_file = get_result_file_fullpath(self._html_directory,
+        new_file = get_result_file_fullpath(self._filesystem, self._html_directory,
                                             baseline_filename, platform, 'new')
         _log.info('    New baseline file: "%s"', new_file)
-        if not os.path.exists(new_file):
+        if not self._filesystem.exists(new_file):
             _log.info('    No new baseline file: "%s"', new_file)
             return ''
 
-        old_file = get_result_file_fullpath(self._html_directory,
+        old_file = get_result_file_fullpath(self._filesystem, self._html_directory,
                                             baseline_filename, platform, 'old')
         _log.info('    Old baseline file: "%s"', old_file)
         if suffix == '.png':
@@ -721,7 +696,7 @@ class HtmlGenerator(object):
             html_td_link = self.HTML_TD_LINK
 
         links = ''
-        if os.path.exists(old_file):
+        if self._filesystem.exists(old_file):
             links += html_td_link % {
                 'uri': self.abspath_to_uri(old_file),
                 'name': baseline_filename}
@@ -732,11 +707,11 @@ class HtmlGenerator(object):
         links += html_td_link % {'uri': self.abspath_to_uri(new_file),
                                  'name': baseline_filename}
 
-        diff_file = get_result_file_fullpath(self._html_directory,
+        diff_file = get_result_file_fullpath(self._filesystem, self._html_directory,
                                              baseline_filename, platform,
                                              'diff')
         _log.info('    Baseline diff file: "%s"', diff_file)
-        if os.path.exists(diff_file):
+        if self._filesystem.exists(diff_file):
             links += html_td_link % {'uri': self.abspath_to_uri(diff_file),
                                      'name': 'Diff'}
         else:
@@ -755,7 +730,7 @@ class HtmlGenerator(object):
           html that compares baseline results for the test.
         """
 
-        test_basename = os.path.basename(os.path.splitext(test)[0])
+        test_basename = self._filesystem.basename(self._filesystem.splitext(test)[0])
         _log.info('  basename: "%s"', test_basename)
         rows = []
         for suffix in BASELINE_SUFFIXES:
@@ -776,8 +751,7 @@ class HtmlGenerator(object):
                     rows.append(self.HTML_TR % row)
 
         if rows:
-            test_path = os.path.join(self._target_port.layout_tests_dir(),
-                                     test)
+            test_path = self._filesystem.join(self._target_port.layout_tests_dir(), test)
             html = self.HTML_TR_TEST % (self.abspath_to_uri(test_path), test)
             html += self.HTML_TEST_DETAIL % ' '.join(rows)
 
@@ -883,7 +857,7 @@ def parse_options(args):
     return (options, target_options)
 
 
-def main(executive=Executive()):
+def main():
     """Main function to produce new baselines."""
 
     (options, target_options) = parse_options(sys.argv[1:])
@@ -929,7 +903,7 @@ def main(executive=Executive()):
         if platform in platforms:
             rebaseline_platforms.append(platform)
 
-    options.html_directory = setup_html_directory(options.html_directory)
+    options.html_directory = setup_html_directory(host_port_obj._filesystem, options.html_directory)
 
     rebaselining_tests = set()
     backup = options.backup
@@ -950,11 +924,11 @@ def main(executive=Executive()):
 
     _log.info('')
     log_dashed_string('Rebaselining result comparison started', None)
-    html_generator = HtmlGenerator(target_port_obj,
+    html_generator = HtmlGenerator(host_port_obj,
+                                   target_port_obj,
                                    options,
                                    rebaseline_platforms,
-                                   rebaselining_tests,
-                                   executive=executive)
+                                   rebaselining_tests)
     html_generator.generate_html()
     if not options.quiet:
         html_generator.show_html()
index 7c55b94c9cc68e2322f6d57b36beee60d2a25725..6c4c68925ce09c1adb5157b0ac5a75b799b5511b 100644 (file)
 
 """Unit tests for rebaseline_chromium_webkit_tests.py."""
 
-import os
 import sys
 import unittest
 
 from webkitpy.tool import mocktool
-from webkitpy.layout_tests import port
-from webkitpy.layout_tests import rebaseline_chromium_webkit_tests
 from webkitpy.common.system.executive import Executive, ScriptError
 
+import port
+import rebaseline_chromium_webkit_tests
+
 
 class MockPort(object):
     def __init__(self, image_diff_exists):
@@ -113,44 +113,44 @@ class TestRebaseliner(unittest.TestCase):
     def test_diff_baselines_txt(self):
         rebaseliner = self.make_rebaseliner()
         output = rebaseliner._port.expected_text(
-            os.path.join(rebaseliner._port.layout_tests_dir(),
-                         'passes/text.html'))
+            rebaseliner._port._filesystem.join(rebaseliner._port.layout_tests_dir(),
+                                               'passes/text.html'))
         self.assertFalse(rebaseliner._diff_baselines(output, output,
                                                      is_image=False))
 
     def test_diff_baselines_png(self):
         rebaseliner = self.make_rebaseliner()
         image = rebaseliner._port.expected_image(
-            os.path.join(rebaseliner._port.layout_tests_dir(),
-                         'passes/image.html'))
+            rebaseliner._port._filesystem.join(rebaseliner._port.layout_tests_dir(),
+                                               'passes/image.html'))
         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(
+        options = mocktool.MockOptions(configuration=None, html_directory='/tmp')
+        host_port = port.get('test', options)
+        generator = rebaseline_chromium_webkit_tests.HtmlGenerator(
+            host_port,
             target_port=None,
-            options=mocktool.MockOptions(configuration=None,
-                                         html_directory='/tmp'),
+            options=options,
             platforms=['mac'],
-            rebaselining_tests=tests,
-            executive=Executive())
+            rebaselining_tests=tests)
+        return generator, host_port
 
     def test_generate_baseline_links(self):
         orig_platform = sys.platform
-        orig_exists = os.path.exists
-
         try:
+            # FIXME: Use the mock filesystem instead of monkey-patching sys.platform
             sys.platform = 'darwin'
-            os.path.exists = lambda x: True
-            generator = self.make_generator(["foo.txt"])
+            generator, host_port = self.make_generator(["foo.txt"])
+            host_port._filesystem.exists = lambda x: True
             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__':
index 42321cfb45bae8f29e55b1e838f378e032ad7458..5bc880b8ef8ee433ca9780883b809ab135e6938b 100644 (file)
@@ -326,7 +326,7 @@ class CommitQueue(AbstractPatchQueue, StepSequenceErrorHandler, CommitQueueTaskD
         archive = self._tool.workspace.create_zip(zip_path, results_directory)
         # Remove the results directory to prevent http logs, etc. from getting huge between runs.
         # We could have create_zip remove the original, but this is more explicit.
-        self._tool.filesystem.remove_tree(results_directory, ignore_errors=True)
+        self._tool.filesystem.rmtree(results_directory)
         return archive
 
     def refetch_patch(self, patch):