2010-11-06 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Nov 2010 03:38:40 +0000 (03:38 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sun, 7 Nov 2010 03:38:40 +0000 (03:38 +0000)
        Reviewed by Eric Siedel.

        new-run-webkit-tests: update port/base and port/webkit to use the
        new FileSystem and Config abstractions, pulling more logic out of
        the base Port classes into separate, mockable objects.

        Also create a MockFileSystem object, a MockConfig object, move
        MockExecutive into common/system to be next to executive, and
        update the config object to use a FileSystem.

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

        * Scripts/webkitpy/common/newstringio.py: Added.
        * Scripts/webkitpy/common/newstringio_unittest.py: Added.
        * Scripts/webkitpy/common/system/executive_mock.py: Added.
        * Scripts/webkitpy/common/system/filesystem_mock.py: Added.
        * Scripts/webkitpy/layout_tests/port/base.py:
        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
        * Scripts/webkitpy/layout_tests/port/config.py:
        * Scripts/webkitpy/layout_tests/port/config_mock.py:
        * Scripts/webkitpy/layout_tests/port/config_unittest.py:
        * Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py:
        * Scripts/webkitpy/layout_tests/port/webkit.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
        * Scripts/webkitpy/tool/mocktool.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@71474 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/config.py
WebKitTools/Scripts/webkitpy/layout_tests/port/config_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py
WebKitTools/Scripts/webkitpy/tool/mocktool.py

index c04460f53e0865844ec685d2430e7382e7cbf6c6..5812c846dce9ea709403acc4d9b10f633fb340f5 100644 (file)
@@ -1,3 +1,31 @@
+2010-11-06  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Siedel.
+
+        new-run-webkit-tests: update port/base and port/webkit to use the
+        new FileSystem and Config abstractions, pulling more logic out of
+        the base Port classes into separate, mockable objects.
+
+        Also create a MockFileSystem object, a MockConfig object, move
+        MockExecutive into common/system to be next to executive, and
+        update the config object to use a FileSystem.
+
+        https://bugs.webkit.org/show_bug.cgi?id=48280
+
+        * Scripts/webkitpy/common/newstringio.py: Added.
+        * Scripts/webkitpy/common/newstringio_unittest.py: Added.
+        * Scripts/webkitpy/common/system/executive_mock.py: Added.
+        * Scripts/webkitpy/common/system/filesystem_mock.py: Added.
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/config.py:
+        * Scripts/webkitpy/layout_tests/port/config_mock.py:
+        * Scripts/webkitpy/layout_tests/port/config_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/google_chrome_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
+        * Scripts/webkitpy/tool/mocktool.py:
+
 2010-11-06  Dirk Pranke  <dpranke@chromium.org>
 
         Unreviewed, build breakage.
index a98b85811f7f9e1db8bb1aa39b9cfda0bac4406c..4a2970e3203c600f5e289f8e83dd8aa42900a1b4 100644 (file)
 """Abstract base class of Port-specific entrypoints for the layout tests
 test infrastructure (the Port and Driver classes)."""
 
-from __future__ import with_statement
-
 import cgi
-import codecs
 import difflib
 import errno
 import os
@@ -42,15 +39,17 @@ import sys
 import time
 
 import apache_http_server
+import config as port_config
 import http_lock
 import http_server
 import test_files
 import websocket_server
 
-from webkitpy.common.memoized import memoized
+from webkitpy.common import system
+from webkitpy.common.system import filesystem
 from webkitpy.common.system import logutils
+from webkitpy.common.system import path
 from webkitpy.common.system.executive import Executive, ScriptError
-from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.common.system.user import User
 
 
@@ -75,27 +74,26 @@ class DummyOptions(object):
 
 # FIXME: This class should merge with webkitpy.webkit_port at some point.
 class Port(object):
-    """Abstract class for Port-specific hooks for the layout_test package.
-    """
-
-    @staticmethod
-    def flag_from_configuration(configuration):
-        flags_by_configuration = {
-            "Debug": "--debug",
-            "Release": "--release",
-        }
-        return flags_by_configuration[configuration]
-
-    def __init__(self, **kwargs):
-        self._name = kwargs.get('port_name', None)
-        self._options = kwargs.get('options')
+    """Abstract class for Port-specific hooks for the layout_test package."""
+
+    def __init__(self, port_name=None, options=None,
+                 executive=None,
+                 user=None,
+                 filesystem=None,
+                 config=None,
+                 **kwargs):
+        self._name = port_name
+        self._options = options
         if self._options is None:
             # FIXME: Ideally we'd have a package-wide way to get a
             # well-formed options object that had all of the necessary
             # options defined on it.
             self._options = DummyOptions()
-        self._executive = kwargs.get('executive', Executive())
-        self._user = kwargs.get('user', User())
+        self._executive = executive or Executive()
+        self._user = user or User()
+        self._filesystem = filesystem or system.filesystem.FileSystem()
+        self._config = config or port_config.Config(self._executive,
+                                                    self._filesystem)
         self._helper = None
         self._http_server = None
         self._webkit_base_dir = None
@@ -118,7 +116,7 @@ class Port(object):
         self._wdiff_available = True
 
         self._pretty_patch_path = self.path_from_webkit_base("BugsSite",
-              "PrettyPatch", "prettify.rb")
+            "PrettyPatch", "prettify.rb")
         self._pretty_patch_available = True
         self.set_option_default('configuration', None)
         if self._options.configuration is None:
@@ -265,7 +263,8 @@ class Port(object):
 
         baselines = []
         for platform_dir in baseline_search_path:
-            if os.path.exists(os.path.join(platform_dir, baseline_filename)):
+            if self.path_exists(self._filesystem.join(platform_dir,
+                                                      baseline_filename)):
                 baselines.append((platform_dir, baseline_filename))
 
             if not all_baselines and baselines:
@@ -274,7 +273,8 @@ class Port(object):
         # If it wasn't found in a platform directory, return the expected
         # result in the test directory, even if no such file actually exists.
         platform_dir = self.layout_tests_dir()
-        if os.path.exists(os.path.join(platform_dir, baseline_filename)):
+        if self.path_exists(self._filesystem.join(platform_dir,
+                                                  baseline_filename)):
             baselines.append((platform_dir, baseline_filename))
 
         if baselines:
@@ -304,35 +304,32 @@ class Port(object):
         platform_dir, baseline_filename = self.expected_baselines(
             filename, suffix)[0]
         if platform_dir:
-            return os.path.join(platform_dir, baseline_filename)
-        return os.path.join(self.layout_tests_dir(), baseline_filename)
-
-    def _expected_file_contents(self, test, extension, encoding):
-        path = self.expected_filename(test, extension)
-        if not os.path.exists(path):
-            return None
-        open_mode = 'r'
-        if encoding is None:
-            open_mode = 'r+b'
-        with codecs.open(path, open_mode, encoding) as file:
-            return file.read()
+            return self._filesystem.join(platform_dir, baseline_filename)
+        return self._filesystem.join(self.layout_tests_dir(), baseline_filename)
 
     def expected_checksum(self, test):
         """Returns the checksum of the image we expect the test to produce, or None if it is a text-only test."""
-        return self._expected_file_contents(test, '.checksum', 'ascii')
+        path = self.expected_filename(test, '.checksum')
+        if not self.path_exists(path):
+            return None
+        return self._filesystem.read_text_file(path)
 
     def expected_image(self, test):
         """Returns the image we expect the test to produce."""
-        return self._expected_file_contents(test, '.png', None)
+        path = self.expected_filename(test, '.png')
+        if not self.path_exists(path):
+            return None
+        return self._filesystem.read_binary_file(path)
 
     def expected_text(self, test):
         """Returns the text output we expect the test to produce."""
-        # NOTE: -expected.txt files are ALWAYS utf-8.  However,
-        # we do not decode the output from DRT, so we should not
-        # decode the -expected.txt values either to allow comparisons.
-        text = self._expected_file_contents(test, '.txt', None)
-        if not text:
+        # FIXME: DRT output is actually utf-8, but since we don't decode the
+        # output from DRT (instead treating it as a binary string), we read the
+        # baselines as a binary string, too.
+        path = self.expected_filename(test, '.txt')
+        if not self.path_exists(path):
             return ''
+        text = self._filesystem.read_binary_file(path)
         return text.strip("\r\n").replace("\r\n", "\n") + "\n"
 
     def filename_to_uri(self, filename):
@@ -362,7 +359,7 @@ class Port(object):
                 protocol = "http"
             return "%s://127.0.0.1:%u/%s" % (protocol, port, relative_path)
 
-        return abspath_to_uri(os.path.abspath(filename))
+        return path.abspath_to_uri(os.path.abspath(filename))
 
     def tests(self, paths):
         """Return the list of tests found (relative to layout_tests_dir()."""
@@ -373,20 +370,19 @@ class Port(object):
 
         Used by --clobber-old-results."""
         layout_tests_dir = self.layout_tests_dir()
-        return filter(lambda x: os.path.isdir(os.path.join(layout_tests_dir, x)),
-                      os.listdir(layout_tests_dir))
+        return filter(lambda x: self._filesystem.isdir(self._filesystem.join(layout_tests_dir, x)),
+                      self._filesystem.listdir(layout_tests_dir))
 
     def path_isdir(self, path):
-        """Returns whether the path refers to a directory of tests.
-
-        Used by test_expectations.py to apply rules to whole directories."""
-        return os.path.isdir(path)
+        """Return True if the path refers to a directory of tests."""
+        # Used by test_expectations.py to apply rules to whole directories.
+        return self._filesystem.isdir(path)
 
     def path_exists(self, path):
-        """Returns whether the path refers to an existing test or baseline."""
+        """Return True if the path refers to an existing test or baseline."""
         # Used by test_expectations.py to determine if an entry refers to a
-        # valid test and by printing.py to determine if baselines exist."""
-        return os.path.exists(path)
+        # valid test and by printing.py to determine if baselines exist.
+        return self._filesystem.exists(path)
 
     def update_baseline(self, path, data, encoding):
         """Updates the baseline for a test.
@@ -398,11 +394,12 @@ class Port(object):
             data: contents of the baseline.
             encoding: file encoding to use for the baseline.
         """
-        write_mode = "w"
+        # FIXME: remove the encoding parameter in favor of text/binary
+        # functions.
         if encoding is None:
-            write_mode = "wb"
-        with codecs.open(path, write_mode, encoding=encoding) as file:
-            file.write(data)
+            self._filesystem.write_binary_file(path, data)
+        else:
+            self._filesystem.write_text_file(path, data)
 
     def uri_to_test_name(self, uri):
         """Return the base layout test name for a given URI.
@@ -414,7 +411,7 @@ class Port(object):
         """
         test = uri
         if uri.startswith("file:///"):
-            prefix = abspath_to_uri(self.layout_tests_dir()) + "/"
+            prefix = path.abspath_to_uri(self.layout_tests_dir()) + "/"
             return test[len(prefix):]
 
         if uri.startswith("http://127.0.0.1:8880/"):
@@ -441,18 +438,16 @@ class Port(object):
         for test_or_category in self.skipped_layout_tests():
             if test_or_category == test_name:
                 return True
-            category = os.path.join(self.layout_tests_dir(), test_or_category)
-            if os.path.isdir(category) and test_name.startswith(test_or_category):
+            category = self._filesystem.join(self.layout_tests_dir(),
+                                             test_or_category)
+            if (self._filesystem.isdir(category) and
+                test_name.startswith(test_or_category)):
                 return True
         return False
 
     def maybe_make_directory(self, *path):
         """Creates the specified directory if it doesn't already exist."""
-        try:
-            os.makedirs(os.path.join(*path))
-        except OSError, e:
-            if e.errno != errno.EEXIST:
-                raise
+        self._filesystem.maybe_make_directory(*path)
 
     def name(self):
         """Return the name of the port (e.g., 'mac', 'chromium-win-xp').
@@ -473,19 +468,13 @@ class Port(object):
         if not hasattr(self._options, name):
             return setattr(self._options, name, default_value)
 
-    # FIXME: This could be replaced by functions in webkitpy.common.checkout.scm.
     def path_from_webkit_base(self, *comps):
         """Returns the full path to path made by joining the top of the
         WebKit source tree and the list of path components in |*comps|."""
-        if not self._webkit_base_dir:
-            abspath = os.path.abspath(__file__)
-            self._webkit_base_dir = abspath[0:abspath.find('WebKitTools')]
-
-        return os.path.join(self._webkit_base_dir, *comps)
+        return self._config.path_from_webkit_base(*comps)
 
-    # FIXME: Callers should eventually move to scm.script_path.
     def script_path(self, script_name):
-        return self.path_from_webkit_base("WebKitTools", "Scripts", script_name)
+        return self._config.script_path(script_name)
 
     def path_to_test_expectations_file(self):
         """Update the test expectations to the passed-in string.
@@ -726,50 +715,8 @@ class Port(object):
                        e.message_with_output()))
             return self._pretty_patch_error_html
 
-    def _webkit_build_directory_command(self, args):
-        return ["perl", self.script_path("webkit-build-directory")] + args
-
-    @memoized
-    def _webkit_top_level_build_directory(self, top_level=True):
-        """This directory is above where products are built to and contains things like the Configuration file."""
-        args = self._webkit_build_directory_command(["--top-level"])
-        return self._executive.run_command(args).rstrip()
-
-    @memoized
-    def _webkit_configuration_build_directory(self, configuration=None):
-        """This is where products are normally built to."""
-        if not configuration:
-            configuration = self.flag_from_configuration(self.get_option('configuration'))
-        args = self._webkit_build_directory_command(["--configuration", configuration])
-        return self._executive.run_command(args).rstrip()
-
-    def _configuration_file_path(self):
-        return os.path.join(self._webkit_top_level_build_directory(), "Configuration")
-
-    # Easy override for unit tests
-    def _open_configuration_file(self):
-        configuration_path = self._configuration_file_path()
-        return codecs.open(configuration_path, "r", "utf-8")
-
-    def _read_configuration(self):
-        try:
-            with self._open_configuration_file() as file:
-                return file.readline().rstrip()
-        except:
-            return None
-
-    # FIXME: This list may be incomplete as Apple has some sekret configs.
-    _RECOGNIZED_CONFIGURATIONS = ("Debug", "Release")
-
     def default_configuration(self):
-        # FIXME: Unify this with webkitdir.pm configuration reading code.
-        configuration = self._read_configuration()
-        if not configuration:
-            configuration = "Release"
-        if configuration not in self._RECOGNIZED_CONFIGURATIONS:
-            _log.warn("Configuration \"%s\" found in %s is not a recognized value.\n" % (configuration, self._configuration_file_path()))
-            _log.warn("Scripts may fail.  See 'set-webkit-configuration --help'.")
-        return configuration
+        return self._config.default_configuration()
 
     #
     # PROTECTED ROUTINES
@@ -777,6 +724,8 @@ class Port(object):
     # The routines below should only be called by routines in this class
     # or any of its subclasses.
     #
+    def _webkit_build_directory(self, args):
+        return self._config.build_directory(args[0])
 
     def _path_to_apache(self):
         """Returns the full path to the apache binary.
@@ -848,8 +797,8 @@ class Port(object):
     def _webkit_baseline_path(self, platform):
         """Return the  full path to the top of the baseline tree for a
         given platform."""
-        return os.path.join(self.layout_tests_dir(), 'platform',
-                            platform)
+        return self._filesystem.join(self.layout_tests_dir(), 'platform',
+                                     platform)
 
 
 class Driver:
index ee868e80f3aa0d685444b3f37dcec15558daef36..1e9c2b75e0249099b18f1482973401eb68f09991 100644 (file)
 
 import optparse
 import os
-import StringIO
 import sys
 import tempfile
 import unittest
 
-from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.common.system.executive import Executive, ScriptError
+from webkitpy.common.system import executive_mock
+from webkitpy.common.system import filesystem
+from webkitpy.common.system import outputcapture
+from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.thirdparty.mock import Mock
 from webkitpy.tool import mocktool
 
 import base
-
-# FIXME: This makes StringIO objects work with "with". Remove
-# when we upgrade to 2.6.
-class NewStringIO(StringIO.StringIO):
-    def __enter__(self):
-        return self
-
-    def __exit__(self, type, value, traceback):
-        pass
-
-
-class MockExecutive():
-    def __init__(self, exception):
-        self._exception = exception
-
-    def run_command(self, *args, **kwargs):
-        raise self._exception
-
-
-class UnitTestPort(base.Port):
-    """Subclass of base.Port used for unit testing."""
-    def __init__(self, configuration_contents=None, configuration_exception=IOError, executive_exception=None):
-        base.Port.__init__(self)
-        self._configuration_contents = configuration_contents
-        self._configuration_exception = configuration_exception
-        if executive_exception:
-            self._executive = MockExecutive(executive_exception)
-
-    def _open_configuration_file(self):
-        if self._configuration_contents:
-            return NewStringIO(self._configuration_contents)
-        raise self._configuration_exception
+import config
+import config_mock
 
 
 class PortTest(unittest.TestCase):
@@ -102,18 +74,21 @@ class PortTest(unittest.TestCase):
         return new_file
 
     def test_pretty_patch_os_error(self):
-        port = UnitTestPort(executive_exception=OSError)
+        port = base.Port(executive=executive_mock.MockExecutive2(exception=OSError))
+        oc = outputcapture.OutputCapture()
+        oc.capture_output()
         self.assertEqual(port.pretty_patch_text("patch.txt"),
                          port._pretty_patch_error_html)
 
         # This tests repeated calls to make sure we cache the result.
         self.assertEqual(port.pretty_patch_text("patch.txt"),
                          port._pretty_patch_error_html)
+        oc.restore_output()
 
     def test_pretty_patch_script_error(self):
         # FIXME: This is some ugly white-box test hacking ...
         base._pretty_patch_available = True
-        port = UnitTestPort(executive_exception=ScriptError)
+        port = base.Port(executive=executive_mock.MockExecutive2(exception=ScriptError))
         self.assertEqual(port.pretty_patch_text("patch.txt"),
                          port._pretty_patch_error_html)
 
@@ -194,13 +169,9 @@ class PortTest(unittest.TestCase):
         self.assertFalse('nosuchthing' in diff)
 
     def test_default_configuration_notfound(self):
-        # Regular IOError thrown while trying to get the configuration.
-        port = UnitTestPort()
-        self.assertEqual(port.default_configuration(), "Release")
-
-        # More exotic OSError thrown.
-        port = UnitTestPort(configuration_exception=OSError)
-        self.assertEqual(port.default_configuration(), "Release")
+        # Test that we delegate to the config object properly.
+        port = base.Port(config=config_mock.MockConfig(default_configuration='default'))
+        self.assertEqual(port.default_configuration(), 'default')
 
     def test_layout_tests_skipping(self):
         port = base.Port()
@@ -209,14 +180,6 @@ class PortTest(unittest.TestCase):
         self.assertTrue(port.skips_layout_test('media/video-zoom.html'))
         self.assertFalse(port.skips_layout_test('foo/foo.html'))
 
-    def test_default_configuration_found(self):
-        port = UnitTestPort(configuration_contents="Debug")
-        self.assertEqual(port.default_configuration(), "Debug")
-
-    def test_default_configuration_unknown(self):
-        port = UnitTestPort(configuration_contents="weird_value")
-        self.assertEqual(port.default_configuration(), "weird_value")
-
     def test_setup_test_run(self):
         port = base.Port()
         # This routine is a no-op. We just test it for coverage.
@@ -229,7 +192,6 @@ class PortTest(unittest.TestCase):
         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")
index 236409869b2608980b55aa09fb04e53cc4e9b623..01a3eb0efb329c4f12cd695f861b3b2181305c37 100644 (file)
 # FIXME: This file needs to be unified with common/checkout/scm.py and
 # common/config/ports.py .
 
-from __future__ import with_statement
-
-import codecs
-import os
-
 from webkitpy.common.checkout import scm
 from webkitpy.common.system import logutils
 
@@ -62,8 +57,9 @@ class Config(object):
         "Release": "--release",
     }
 
-    def __init__(self, executive):
+    def __init__(self, executive, filesystem):
         self._executive = executive
+        self._filesystem = filesystem
         self._webkit_base_dir = None
         self._default_configuration = None
         self._build_directories = {}
@@ -115,7 +111,7 @@ class Config(object):
         return self._default_configuration
 
     def path_from_webkit_base(self, *comps):
-        return os.path.join(self.webkit_base_dir(), *comps)
+        return self._filesystem.join(self.webkit_base_dir(), *comps)
 
     def webkit_base_dir(self):
         """Returns the absolute path to the top of the WebKit tree.
@@ -129,14 +125,16 @@ class Config(object):
         return self._webkit_base_dir
 
     def _script_path(self, script_name):
-        return os.path.join(self.webkit_base_dir(), "WebKitTools",
-                            "Scripts", script_name)
+        return self._filesystem.join(self.webkit_base_dir(), "WebKitTools",
+                                     "Scripts", script_name)
 
     def _determine_configuration(self):
         # This mirrors the logic in webkitdirs.pm:determineConfiguration().
         global _determined_configuration
         if _determined_configuration == -1:
             contents = self._read_configuration()
+            if not contents:
+                contents = "Release"
             if contents == "Deployment":
                 contents = "Release"
             if contents == "Development":
@@ -145,10 +143,9 @@ class Config(object):
         return _determined_configuration
 
     def _read_configuration(self):
-        configuration_path = os.path.join(self.build_directory(None),
-                                          "Configuration")
-        if not os.path.exists(configuration_path):
+        configuration_path = self._filesystem.join(self.build_directory(None),
+                                                   "Configuration")
+        if not self._filesystem.exists(configuration_path):
             return None
 
-        with codecs.open(configuration_path, "r", "utf-8") as fh:
-            return fh.read().rstrip()
+        return self._filesystem.read_text_file(configuration_path).rstrip()
index 4674cba360f2aebc3a39d1831ef7dd3b62bb2082..5685938eec614e844ac9f19817e41a809819dea3 100644 (file)
 # (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 codecs
-import os
-import StringIO
 import unittest
 
+from webkitpy.common.system import executive
+from webkitpy.common.system import executive_mock
+from webkitpy.common.system import filesystem
+from webkitpy.common.system import filesystem_mock
 from webkitpy.common.system import outputcapture
 
 import config
 
-
-# FIXME: This makes StringIO objects work with "with". Remove
-# when we upgrade to 2.6.
-class NewStringIO(StringIO.StringIO):
-    def __enter__(self):
-        return self
-
-    def __exit__(self, type, value, traceback):
-        pass
-
-
-class MockExecutive(object):
-    def __init__(self, output='', exit_code=0):
-        self._output = output
-        self._exit_code = exit_code
-
-    def run_command(self, arg_list, return_exit_code=False,
-                    decode_output=False):
-        if return_exit_code:
-            return self._exit_code
-        return self._output
-
-
 class ConfigTest(unittest.TestCase):
     def tearDown(self):
         config.clear_cached_configuration()
 
-    def assertConfiguration(self, contents, expected):
+    def make_config(self, output='', files={}, exit_code=0):
+        e = executive_mock.MockExecutive2(output=output, exit_code=exit_code)
+        fs = filesystem_mock.MockFileSystem(files)
+        return config.Config(e, fs)
+
+    def assert_configuration(self, contents, expected):
         # This tests that a configuration file containing
         # _contents_ endsd up being interpreted as _expected_.
-        #
-        # FIXME: rewrite this when we have a filesystem abstraction we
-        # can mock out more easily.
-        config.clear_cached_configuration()
-        orig_open = codecs.open
-
-        def wrap_open(contents):
-            def open_wrapper(path, mode, encoding):
-                return NewStringIO(contents)
-            return open_wrapper
-
-        try:
-            orig_exists = os.path.exists
-            os.path.exists = lambda p: True
-            codecs.open = wrap_open(contents)
-
-            e = MockExecutive(output='foo')
-            c = config.Config(e)
-            self.assertEqual(c.default_configuration(), expected)
-        finally:
-            os.path.exists = orig_exists
-            codecs.open = orig_open
+        c = self.make_config('foo', {'foo/Configuration': contents})
+        self.assertEqual(c.default_configuration(), expected)
 
     def test_build_directory_toplevel(self):
-        e = MockExecutive(output="toplevel")
-        c = config.Config(e)
+        c = self.make_config('toplevel')
         self.assertEqual(c.build_directory(None), 'toplevel')
 
         # Test again to check caching
         self.assertEqual(c.build_directory(None), 'toplevel')
 
     def test_build_directory__release(self):
-        e = MockExecutive(output="release")
-        c = config.Config(e)
+        c = self.make_config('release')
         self.assertEqual(c.build_directory('Release'), 'release')
 
     def test_build_directory__debug(self):
-        e = MockExecutive(output="debug")
-        c = config.Config(e)
+        c = self.make_config('debug')
         self.assertEqual(c.build_directory('Debug'), 'debug')
 
     def test_build_directory__unknown(self):
-        e = MockExecutive(output="unknown")
-        c = config.Config(e)
+        c = self.make_config("unknown")
         self.assertRaises(KeyError, c.build_directory, 'Unknown')
 
     def test_build_dumprendertree__success(self):
-        e = MockExecutive(exit_code=0)
-        c = config.Config(e)
+        c = self.make_config(exit_code=0)
         self.assertTrue(c.build_dumprendertree("Debug"))
         self.assertTrue(c.build_dumprendertree("Release"))
         self.assertRaises(KeyError, c.build_dumprendertree, "Unknown")
 
     def test_build_dumprendertree__failure(self):
-        e = MockExecutive(exit_code=-1)
-        c = config.Config(e)
+        c = self.make_config(exit_code=-1)
 
+        # FIXME: Build failures should log errors. However, the message we
+        # get depends on how we're being called; as a standalone test,
+        # we'll get the "no handlers found" message. As part of
+        # test-webkitpy, we get the actual message. Really, we need
+        # outputcapture to install its own handler.
         oc = outputcapture.OutputCapture()
         oc.capture_output()
         self.assertFalse(c.build_dumprendertree('Debug'))
-        (out, err) = oc.restore_output()
+        oc.restore_output()
 
         oc.capture_output()
         self.assertFalse(c.build_dumprendertree('Release'))
         oc.restore_output()
 
     def test_default_configuration__release(self):
-        self.assertConfiguration('Release', 'Release')
+        self.assert_configuration('Release', 'Release')
 
     def test_default_configuration__debug(self):
-        self.assertConfiguration('Debug', 'Debug')
+        self.assert_configuration('Debug', 'Debug')
 
     def test_default_configuration__deployment(self):
-        self.assertConfiguration('Deployment', 'Release')
+        self.assert_configuration('Deployment', 'Release')
 
     def test_default_configuration__development(self):
-        self.assertConfiguration('Development', 'Debug')
+        self.assert_configuration('Development', 'Debug')
 
     def test_default_configuration__notfound(self):
         # This tests what happens if the default configuration file
         # doesn't exist.
-        config.clear_cached_configuration()
-        try:
-            orig_exists = os.path.exists
-            os.path.exists = lambda p: False
-            e = MockExecutive(output="foo")
-            c = config.Config(e)
-            self.assertEqual(c.default_configuration(), "Release")
-        finally:
-            os.path.exists = orig_exists
+        c = self.make_config(output='foo', files={'foo/Configuration': None})
+        self.assertEqual(c.default_configuration(), "Release")
 
     def test_default_configuration__unknown(self):
         # Ignore the warning about an unknown configuration value.
         oc = outputcapture.OutputCapture()
         oc.capture_output()
-        self.assertConfiguration('Unknown', 'Unknown')
+        self.assert_configuration('Unknown', 'Unknown')
         oc.restore_output()
 
-    def test_path_from_webkit_base(self, *comps):
-        c = config.Config(None)
+    def test_path_from_webkit_base(self):
+        # FIXME: We use a real filesystem here. Should this move to a
+        # mocked one?
+        c = config.Config(executive.Executive(), filesystem.FileSystem())
         self.assertTrue(c.path_from_webkit_base('foo'))
 
     def test_webkit_base_dir(self):
-        c = config.Config(None)
+        # FIXME: We use a real filesystem here. Should this move to a
+        # mocked one?
+        c = config.Config(executive.Executive(), filesystem.FileSystem())
         self.assertTrue(c.webkit_base_dir())
 
 
index c4c885d6329ab3d0bbafaaeca17343a464ee7a35..e60c27416deb37935f40b45357cbd2971d2c6ea6 100644 (file)
@@ -28,7 +28,8 @@ import codecs
 import os
 import unittest
 
-import base_unittest
+from webkitpy.common import newstringio
+
 import factory
 import google_chrome
 
@@ -77,7 +78,7 @@ class GetGoogleChromePortTest(unittest.TestCase):
 
         def mock_open(path, mode, encoding):
             if 'test_expectations_chrome.txt' in path:
-                return base_unittest.NewStringIO(expected_string)
+                return newstringio.StringIO(expected_string)
             return orig_open(path, mode, encoding)
 
         try:
index 0b324f552a5508809bd88c87f282b610df3768c9..51f57c40e666167b18cdf52d64201875965247e8 100644 (file)
@@ -81,14 +81,8 @@ class WebKitPort(base.Port):
         return ''
 
     def _build_driver(self):
-        exit_code = self._executive.run_command([
-            self.script_path("build-dumprendertree"),
-            self.flag_from_configuration(self.get_option('configuration')),
-        ], return_exit_code=True)
-        if exit_code != 0:
-            _log.error("Failed to build DumpRenderTree")
-            return False
-        return True
+        configuration = self.get_option('configuration')
+        return self._config.build_dumprendertree(configuration)
 
     def _check_driver(self):
         driver_path = self._path_to_driver()
@@ -357,8 +351,8 @@ class WebKitPort(base.Port):
             'mac-tiger', 'mac-leopard', 'mac-snowleopard')
 
     def _build_path(self, *comps):
-        build_root = self._webkit_configuration_build_directory()
-        return os.path.join(build_root, *comps)
+        return self._filesystem.join(self._config.build_directory(
+            self.get_option('configuration')), *comps)
 
     def _path_to_driver(self):
         return self._build_path('DumpRenderTree')
index f21e7a5e74b28bcf0e13ad356500bbb89ba1b708..248a291d3930325fc72cbf06dfd62e3ad06a422b 100644 (file)
@@ -380,31 +380,30 @@ class RebaselineTest(unittest.TestCase):
             baseline = file + "-expected" + ext
             self.assertTrue(any(f.find(baseline) != -1 for f in file_list))
 
-    # FIXME: This test is failing on the bots. Also, this test touches the
-    #        file system.  Unit tests should not read or write the file system.
-    #        https://bugs.webkit.org/show_bug.cgi?id=47879
     def disabled_test_reset_results(self):
+        # FIXME: This test is disabled until we can rewrite it to use a
+        # mock filesystem.
+        #
+        # Test that we update expectations in place. If the expectation
+        # is missing, update the expected generic location.
         file_list = []
-        original_open = codecs.open
-        try:
-            # Test that we update expectations in place. If the expectation
-            # is missing, update the expected generic location.
-            file_list = []
-            codecs.open = _mocked_open(original_open, file_list)
-            passing_run(['--pixel-tests',
-                         '--reset-results',
-                         'passes/image.html',
-                         'failures/expected/missing_image.html'],
-                         tests_included=True)
-            self.assertEqual(len(file_list), 6)
-            self.assertBaselines(file_list,
-                "data/passes/image")
-            self.assertBaselines(file_list,
-                "data/failures/expected/missing_image")
-        finally:
-            codecs.open = original_open
-
-    def test_new_baseline(self):
+        passing_run(['--pixel-tests',
+                        '--reset-results',
+                        'passes/image.html',
+                        'failures/expected/missing_image.html'],
+                        tests_included=True)
+        self.assertEqual(len(file_list), 6)
+        self.assertBaselines(file_list,
+            "data/passes/image")
+        self.assertBaselines(file_list,
+            "data/failures/expected/missing_image")
+
+    def disabled_test_new_baseline(self):
+        # FIXME: This test is disabled until we can rewrite it to use a
+        # mock filesystem.
+        #
+        # Test that we update the platform expectations. If the expectation
+        # is mssing, then create a new expectation in the platform dir.
         file_list = []
         original_open = codecs.open
         try:
index b6ee95f571030e196a67252652d4f252c02c4dba..719f9b1bbc8235569d851e8f24216d8fc3442e9e 100644 (file)
@@ -571,6 +571,7 @@ class MockStatusServer(object):
 
 
 # FIXME: This should not inherit from Mock
+# FIXME: Unify with common.system.executive_mock.MockExecutive.
 class MockExecutive(Mock):
     def __init__(self, should_log):
         self._should_log = should_log