2011-01-19 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jan 2011 01:49:19 +0000 (01:49 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 20 Jan 2011 01:49:19 +0000 (01:49 +0000)
        Reviewed by Mihai Parparita.

        add a .sep property, abspath(), isabs(), mtime(), and
        open_text_file_for_writing() to filesystem modules. Some of
        these properties are not needed in this patch but will be needed
        in subsequent patches (I'm doing this to avoid having to track
        multiple versions of a single file).

        Also, change most of the port/* modules to use the filesystem
        objects instead of referencing the filesystem directly.

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

        * Scripts/webkitpy/common/system/filesystem.py:
        * Scripts/webkitpy/common/system/filesystem_mock.py:
        * Scripts/webkitpy/layout_tests/port/__init__.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/config.py:
        * Scripts/webkitpy/layout_tests/port/test.py:
        * Scripts/webkitpy/layout_tests/port/test_files_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@76184 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/port/__init__.py
Tools/Scripts/webkitpy/layout_tests/port/base.py
Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/chromium.py
Tools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
Tools/Scripts/webkitpy/layout_tests/port/config.py
Tools/Scripts/webkitpy/layout_tests/port/test.py
Tools/Scripts/webkitpy/layout_tests/port/test_files_unittest.py

index e5013341666b3ba7059823836b030ad4d619ae01..b20dfcb1c1a3efe2ae6b232a574cbdff21c7edab 100644 (file)
@@ -1,3 +1,29 @@
+2011-01-19  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Mihai Parparita.
+
+        add a .sep property, abspath(), isabs(), mtime(), and
+        open_text_file_for_writing() to filesystem modules. Some of
+        these properties are not needed in this patch but will be needed
+        in subsequent patches (I'm doing this to avoid having to track
+        multiple versions of a single file).
+
+        Also, change most of the port/* modules to use the filesystem
+        objects instead of referencing the filesystem directly.
+
+        https://bugs.webkit.org/show_bug.cgi?id=52748
+
+        * Scripts/webkitpy/common/system/filesystem.py:
+        * Scripts/webkitpy/common/system/filesystem_mock.py:
+        * Scripts/webkitpy/layout_tests/port/__init__.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/config.py:
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        * Scripts/webkitpy/layout_tests/port/test_files_unittest.py:
+
 2011-01-19  Maciej Stachowiak  <mjs@apple.com>
 
         Reviewed by Anders Carlsson.
index 6dcf2072016cc27b21b8c3b0d4355619c181c48e..d75276eb3b35fb8ab9eeb794e3592363fef6d459 100644 (file)
@@ -44,6 +44,11 @@ class FileSystem(object):
 
     Unless otherwise noted, all paths are allowed to be either absolute
     or relative."""
+    def __init__(self):
+        self.sep = os.sep
+
+    def abspath(self, path):
+        return os.path.abspath(path)
 
     def basename(self, path):
         """Wraps os.path.basename()."""
@@ -100,6 +105,10 @@ class FileSystem(object):
         """Wraps glob.glob()."""
         return glob.glob(path)
 
+    def isabs(self, path):
+        """Return whether the path is an absolute path."""
+        return os.path.isabs(path)
+
     def isfile(self, path):
         """Return whether the path refers to a file."""
         return os.path.isfile(path)
@@ -159,6 +168,9 @@ class FileSystem(object):
     def move(self, src, dest):
         shutil.move(src, dest)
 
+    def mtime(self, path):
+        return os.stat(path).st_mtime
+
     def normpath(self, path):
         """Wraps os.path.normpath()."""
         return os.path.normpath(path)
@@ -169,6 +181,13 @@ class FileSystem(object):
         f = os.fdopen(temp_fd, 'wb')
         return f, temp_name
 
+    def open_text_file_for_writing(self, path, append=False):
+        """Returns a file handle suitable for writing to."""
+        mode = 'w'
+        if append:
+            mode = 'wa'
+        return codecs.open(path, mode, 'utf8')
+
     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:
index 88c18e8ca58643c14dec090afcbe8a7adf6936ca..0004944994f4b73547743e0ad178075ab9c2ed60 100644 (file)
@@ -43,7 +43,9 @@ class MockFileSystem(object):
                 not exist.
         """
         self.files = files or {}
-        self._current_tmpno = 0
+        self.written_files = {}
+        self.sep = '/'
+        self.current_tmpno = 0
 
     def _raise_not_found(self, path):
         raise IOError(errno.ENOENT, path, os.strerror(errno.ENOENT))
@@ -52,6 +54,9 @@ class MockFileSystem(object):
         idx = path.rfind('/')
         return (path[0:idx], path[idx + 1:])
 
+    def abspath(self, path):
+        return path
+
     def basename(self, path):
         return self._split(path)[1]
 
@@ -111,6 +116,9 @@ class MockFileSystem(object):
         else:
             return [f for f in self.files if f == path]
 
+    def isabs(self, path):
+        return path.startswith('/')
+
     def isfile(self, path):
         return path in self.files and self.files[path] is not None
 
@@ -119,7 +127,12 @@ class MockFileSystem(object):
             return False
         if not path.endswith('/'):
             path += '/'
-        return any(f.startswith(path) for f in self.files)
+
+        # We need to use a copy of the keys here in order to avoid switching
+        # to a different thread and potentially modifying the dict in
+        # mid-iteration.
+        files = self.files.keys()[:]
+        return any(f.startswith(path) for f in files)
 
     def join(self, *comps):
         return re.sub(re.escape(os.path.sep), '/', os.path.join(*comps))
@@ -144,6 +157,11 @@ class MockFileSystem(object):
                     files.append(remaining)
         return dirs + files
 
+    def mtime(self, path):
+        if self.exists(path):
+            return 0
+        self._raise_not_found(path)
+
     def _mktemp(self, suffix='', prefix='tmp', dir=None, **kwargs):
         if dir is None:
             dir = '/__im_tmp'
@@ -170,7 +188,7 @@ class MockFileSystem(object):
 
                 # FIXME: Should we delete non-empty directories?
                 if self._filesystem.exists(self._directory_path):
-                    self._filesystem.rmdir(self._directory_path)
+                    self._filesystem.rmtree(self._directory_path)
 
         return TemporaryDirectory(fs=self, **kwargs)
 
@@ -191,14 +209,17 @@ class MockFileSystem(object):
         path = self._mktemp(suffix)
         return WritableFileObject(self, path), path
 
+    def open_text_file_for_writing(self, path, append=False):
+        return WritableFileObject(self, path, append)
+
     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:
-                self._raise_not_found(path)
-            return self.files[path]
+        # Intentionally raises KeyError if we don't recognize the path.
+        if self.files[path] is None:
+            self._raise_not_found(path)
+        return self.files[path]
 
     def remove(self, path):
         if self.files[path] is None:
@@ -224,12 +245,13 @@ class MockFileSystem(object):
 
     def write_binary_file(self, path, contents):
         self.files[path] = contents
+        self.written_files[path] = contents
 
 
 class WritableFileObject(object):
     def __init__(self, fs, path, append=False, encoding=None):
         self.fs = fs
-        self.name = name
+        self.path = path
         self.closed = False
         if path not in self.fs.files or not append:
             self.fs.files[path] = ""
@@ -244,5 +266,5 @@ class WritableFileObject(object):
         self.closed = True
 
     def write(self, str):
-        self.fs.files[self.name] += str
-        self.fs.written_files[self.name] = self.fs.files[self.name]
+        self.fs.files[self.path] += str
+        self.fs.written_files[self.path] = self.fs.files[self.path]
index e3ad6f44c1d2f280dc971558abc1fcd642a47c36..59ab2ef11109bbb300f3ddd5db91b222c836c7f5 100644 (file)
@@ -30,3 +30,5 @@
 """Port-specific entrypoints for the layout tests test infrastructure."""
 
 from factory import get
+
+from test import unit_test_filesystem
index 97b54c97d81fff939991e6aeb2a3331608888152..74c10184c9d726ef713856cbe7a0fb51f3359a34 100644 (file)
@@ -244,7 +244,7 @@ class Port(object):
                 tree)
             results_filename - relative path from top of tree to the results
                 file
-            (os.path.join of the two gives you the full path to the file,
+            (port.join() of the two gives you the full path to the file,
                 unless None was returned.)
         Return values will be in the format appropriate for the current
         platform (e.g., "\\" for path separators on Windows). If the results
@@ -255,7 +255,7 @@ class Port(object):
         conjunction with the other baseline and filename routines that are
         platform specific.
         """
-        testname = os.path.splitext(self.relative_test_filename(filename))[0]
+        testname = self._filesystem.splitext(self.relative_test_filename(filename))[0]
 
         baseline_filename = testname + '-expected' + suffix
 
@@ -360,7 +360,7 @@ class Port(object):
                 protocol = "http"
             return "%s://127.0.0.1:%u/%s" % (protocol, port, relative_path)
 
-        return path.abspath_to_uri(os.path.abspath(filename))
+        return path.abspath_to_uri(self._filesystem.abspath(filename))
 
     def tests(self, paths):
         """Return the list of tests found (relative to layout_tests_dir()."""
@@ -702,7 +702,7 @@ class Port(object):
     def pretty_patch_text(self, diff_path):
         if not self._pretty_patch_available:
             return self._pretty_patch_error_html
-        command = ("ruby", "-I", os.path.dirname(self._pretty_patch_path),
+        command = ("ruby", "-I", self._filesystem.dirname(self._pretty_patch_path),
                    self._pretty_patch_path, diff_path)
         try:
             # Diffs are treated as binary (we pass decode_output=False) as they
index 8d586e354e42f0393c6c96fbbe10760bd37185ee..72f2d05a42ade9cacfc971088b19397058a7d538 100644 (file)
@@ -27,7 +27,6 @@
 # OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import optparse
-import os
 import sys
 import tempfile
 import unittest
@@ -194,7 +193,7 @@ class PortTest(unittest.TestCase):
     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")
+        test_file = port._filesystem.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
index 9aa0946f89bea6165b33ce5543f54235324bb2b4..eebf8530ce09ebb00a52a2fe6932775d62a76e95 100644 (file)
 
 """Chromium implementations of the Port interface."""
 
-from __future__ import with_statement
-
-import codecs
 import errno
 import logging
-import os
 import re
-import shutil
 import signal
 import subprocess
 import sys
-import tempfile
 import time
 import webbrowser
 
@@ -71,7 +65,7 @@ def check_file_exists(path_to_file, file_description, override_step=None,
             you're looking for (e.g., "HTTP Server"). Used for error logging.
         override_step: An optional string to be logged if the check fails.
         logging: Whether or not log the error messages."""
-    if not os.path.exists(path_to_file):
+    if not self._filesystem.exists(path_to_file):
         if logging:
             _log.error('Unable to find %s' % file_description)
             _log.error('    at %s' % path_to_file)
@@ -145,13 +139,11 @@ class ChromiumPort(base.Port):
                    diff_filename=None):
         executable = self._path_to_image_diff()
 
-        tempdir = tempfile.mkdtemp()
-        expected_filename = os.path.join(tempdir, "expected.png")
-        with open(expected_filename, 'w+b') as file:
-            file.write(expected_contents)
-        actual_filename = os.path.join(tempdir, "actual.png")
-        with open(actual_filename, 'w+b') as file:
-            file.write(actual_contents)
+        tempdir = self._filesystem.mkdtemp()
+        expected_filename = self._filesystem.join(str(tempdir), "expected.png")
+        self._filesystem.write_binary_file(expected_filename, expected_contents)
+        actual_filename = self._filesystem.join(str(tempdir), "actual.png")
+        self._filesystem.write_binary_file(actual_filename, actual_contents)
 
         if diff_filename:
             cmd = [executable, '--diff', expected_filename,
@@ -180,7 +172,7 @@ class ChromiumPort(base.Port):
             else:
                 raise e
         finally:
-            shutil.rmtree(tempdir, ignore_errors=True)
+            self._filesystem.rmtree(str(tempdir))
         return result
 
     def driver_name(self):
@@ -192,15 +184,15 @@ class ChromiumPort(base.Port):
         """Returns the full path to path made by joining the top of the
         Chromium source tree and the list of path components in |*comps|."""
         if not self._chromium_base_dir:
-            abspath = os.path.abspath(__file__)
+            abspath = self._filesystem.abspath(__file__)
             offset = abspath.find('third_party')
             if offset == -1:
-                self._chromium_base_dir = os.path.join(
+                self._chromium_base_dir = self._filesystem.join(
                     abspath[0:abspath.find('Tools')],
                     'WebKit', 'chromium')
             else:
                 self._chromium_base_dir = abspath[0:offset]
-        return os.path.join(self._chromium_base_dir, *comps)
+        return self._filesystem.join(self._chromium_base_dir, *comps)
 
     def path_to_test_expectations_file(self):
         return self.path_from_webkit_base('LayoutTests', 'platform',
@@ -218,10 +210,10 @@ class ChromiumPort(base.Port):
     def setup_test_run(self):
         # Delete the disk cache if any to ensure a clean test run.
         dump_render_tree_binary_path = self._path_to_driver()
-        cachedir = os.path.split(dump_render_tree_binary_path)[0]
-        cachedir = os.path.join(cachedir, "cache")
-        if os.path.exists(cachedir):
-            shutil.rmtree(cachedir)
+        cachedir = self._filesystem.dirname(dump_render_tree_binary_path)
+        cachedir = self._filesystem.join(cachedir, "cache")
+        if self._filesystem.exists(cachedir):
+            self._filesystem.rmtree(cachedir)
 
     def create_driver(self, worker_number):
         """Starts a new Driver and returns a handle to it."""
@@ -258,8 +250,7 @@ class ChromiumPort(base.Port):
         Basically this string should contain the equivalent of a
         test_expectations file. See test_expectations.py for more details."""
         expectations_path = self.path_to_test_expectations_file()
-        with codecs.open(expectations_path, "r", "utf-8") as file:
-            return file.read()
+        return self._filesystem.read_text_file(expectations_path)
 
     def test_expectations_overrides(self):
         try:
@@ -267,10 +258,9 @@ class ChromiumPort(base.Port):
                 'layout_tests', 'test_expectations.txt')
         except AssertionError:
             return None
-        if not os.path.exists(overrides_path):
+        if not self._filesystem.exists(overrides_path):
             return None
-        with codecs.open(overrides_path, "r", "utf-8") as file:
-            return file.read()
+        return self._filesystem.read_text_file(overrides_path)
 
     def skipped_layout_tests(self, extra_test_files=None):
         expectations_str = self.test_expectations()
@@ -319,8 +309,8 @@ class ChromiumPort(base.Port):
                 debug_path = self._path_to_driver('Debug')
                 release_path = self._path_to_driver('Release')
 
-                debug_mtime = os.stat(debug_path).st_mtime
-                release_mtime = os.stat(release_path).st_mtime
+                debug_mtime = self._filesystem.mtime(debug_path)
+                release_mtime = self._filesystem.mtime(release_path)
 
                 if (debug_mtime > release_mtime and configuration == 'Release' or
                     release_mtime > debug_mtime and configuration == 'Debug'):
@@ -363,7 +353,7 @@ class ChromiumDriver(base.Driver):
         self._worker_number = worker_number
         self._image_path = None
         if self._port.get_option('pixel_tests'):
-            self._image_path = os.path.join(
+            self._image_path = self._filesystem.join(
                 self._port.get_option('results_directory'),
                 'png_result%s.png' % self._worker_number)
 
@@ -455,9 +445,8 @@ class ChromiumDriver(base.Driver):
     def _output_image(self):
         """Returns the image output which driver generated."""
         png_path = self._image_path
-        if png_path and os.path.isfile(png_path):
-            with open(png_path, 'rb') as image_file:
-                return image_file.read()
+        if png_path and self._filesystem.isfile(png_path):
+            return self._filesystem.read_binary_file(png_path)
         else:
             return None
 
index 43bfbd48e94ceaa2d027c955c4bc152c1dd7b130..5a02cae4cab2c5d32e61a7f881be32ebe3abd522 100644 (file)
@@ -32,9 +32,11 @@ import StringIO
 
 from webkitpy.common.system import logtesting
 from webkitpy.common.system import executive_mock
+from webkitpy.common.system import filesystem_mock
 from webkitpy.tool import mocktool
 from webkitpy.thirdparty.mock import Mock
 
+
 import chromium
 import chromium_linux
 import chromium_mac
@@ -100,7 +102,8 @@ class ChromiumPortTest(unittest.TestCase):
         def __init__(self, options):
             chromium_linux.ChromiumLinuxPort.__init__(self,
                                                       port_name='test-port',
-                                                      options=options)
+                                                      options=options,
+                                                      filesystem=filesystem_mock.MockFileSystem())
 
         def default_configuration(self):
             self.default_configuration_called = True
index e08ed9d9c3376816f67ad5fcb3052e23d5117e5d..60166396aab0a6eacf9a8d46d564143e42a97886 100644 (file)
@@ -32,8 +32,6 @@
 # FIXME: This file needs to be unified with common/checkout/scm.py and
 # common/config/ports.py .
 
-import os
-
 from webkitpy.common.system import logutils
 from webkitpy.common.system import executive
 
@@ -131,7 +129,7 @@ class Config(object):
         #
         # This code will also work if there is no SCM system at all.
         if not self._webkit_base_dir:
-            abspath = os.path.abspath(__file__)
+            abspath = self._filesystem.abspath(__file__)
             self._webkit_base_dir = abspath[0:abspath.find('Tools') - 1]
         return self._webkit_base_dir
 
index 2adcec0709934ae1c01c0df187eea1e068df2908..5df5c2d9d6219bf9469496f2574e63c18112ade4 100644 (file)
@@ -146,10 +146,10 @@ LAYOUT_TEST_DIR = '/test.checkout/LayoutTests'
 # in order to fully control the test output and to demonstrate that
 # we don't need a real filesystem to run the tests.
 
-def unit_test_filesystem(test_list=None):
+def unit_test_filesystem(files=None):
     """Return the FileSystem object used by the unit tests."""
-    test_list = test_list or unit_test_list()
-    files = {}
+    test_list = unit_test_list()
+    files = files or {}
 
     def add_file(files, test, suffix, contents):
         dirname = test.name[0:test.name.rfind('/')]
@@ -184,16 +184,32 @@ WONTFIX SKIP : failures/expected/keyboard.html = CRASH
 WONTFIX SKIP : failures/expected/exception.html = CRASH
 """
 
-    return filesystem_mock.MockFileSystem(files)
+    fs = filesystem_mock.MockFileSystem(files)
+    fs._tests = test_list
+    return fs
 
 
 class TestPort(base.Port):
     """Test implementation of the Port interface."""
 
     def __init__(self, **kwargs):
-        self._tests = unit_test_list()
-        if 'filesystem' not in kwargs:
-            kwargs['filesystem'] = unit_test_filesystem(self._tests)
+        # FIXME: what happens if we're not passed in the test filesystem
+        # and the tests don't match what's in the filesystem?
+        #
+        # We'll leave as is for now to avoid unnecessary dependencies while
+        # converting all of the unit tests over to using
+        # unit_test_filesystem(). If things get out of sync the tests should
+        # fail in fairly obvious ways. Eventually we want to just do:
+        #
+        # assert kwargs['filesystem']._tests
+        # self._tests = kwargs['filesystem']._tests
+
+        if 'filesystem' not in kwargs or kwargs['filesystem'] is None:
+            kwargs['filesystem'] = unit_test_filesystem()
+            self._tests = kwargs['filesystem']._tests
+        else:
+            self._tests = unit_test_list()
+
         kwargs.setdefault('port_name', 'test')
         base.Port.__init__(self, **kwargs)
 
index eabdfd9c2b0be008de4b8bc76612f7d75ffe3505..a68950a91c35eb69d2bab51dba2056c1932abb10 100644 (file)
@@ -26,7 +26,6 @@
 # (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 os
 import unittest
 
 import base
@@ -37,8 +36,8 @@ class TestFilesTest(unittest.TestCase):
     def test_find_no_paths_specified(self):
         port = base.Port()
         layout_tests_dir = port.layout_tests_dir()
-        port.layout_tests_dir = lambda: os.path.join(layout_tests_dir,
-                                                     'fast', 'html')
+        port.layout_tests_dir = lambda: port._filesystem.join(layout_tests_dir,
+                                                              'fast', 'html')
         tests = test_files.find(port, [])
         self.assertNotEqual(tests, 0)