2010-10-18 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Oct 2010 01:02:35 +0000 (01:02 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Tue, 19 Oct 2010 01:02:35 +0000 (01:02 +0000)
        Reviewed by Eric Siedel.

        new-run-webkit-tests: clean up the options-parsing code in the port
        classes.

        This change modifies the Port interface to have a get_option() and
        set_option_default() method for accessing the options argument
        passed to the constructor. If the constructor is not passed an
        options argument, we default to a MockOptions() argument from
        mocktool, which has the same semantics we want.

        Note that there is a disadvantage to port.get_option('foo') over
        port._options.foo, which is that you lose some of the checking
        for whether 'foo' is set (typos result in the default value, not
        an exception being raised. This is desired in this case, since the
        Port class is not allowed to assume that options does have any
        particular values set, and so this change ensures that all of
        the subclasses are following the same, intended, logic.

        Arguably this is the wrong semantics to have, and the Port
        classes should be able to assume a default set of
        attributes/arguments, but that change will need to wait for a
        different CL where we can modify new-run-webkit-tests to pull a
        list of arguments from the port factory routines.

        Also, add unit tests for webkitpy.tool.mocktool.MockOptions .

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

        * 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_gpu_unittest.py:
        * Scripts/webkitpy/layout_tests/port/chromium_linux.py:
        * Scripts/webkitpy/layout_tests/port/chromium_mac.py:
        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
        * Scripts/webkitpy/layout_tests/port/dryrun.py:
        * Scripts/webkitpy/layout_tests/port/factory_unittest.py:
        * Scripts/webkitpy/layout_tests/port/mac_unittest.py:
        * Scripts/webkitpy/layout_tests/port/port_testcase.py:
        * Scripts/webkitpy/layout_tests/port/test.py:
        * Scripts/webkitpy/layout_tests/port/webkit.py:
        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py:
        * Scripts/webkitpy/tool/mocktool_unittest.py: Added.

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

17 files changed:
WebKitTools/ChangeLog
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py
WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py
WebKitTools/Scripts/webkitpy/layout_tests/port/factory_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/port_testcase.py
WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py
WebKitTools/Scripts/webkitpy/tool/mocktool_unittest.py [new file with mode: 0644]

index 2a9a3f76bc0581ff85245df1e2dc413b2cd42d48..9170b90bb74485dde56cdd26a71a51dc57363907 100644 (file)
@@ -1,3 +1,51 @@
+2010-10-18  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Eric Siedel.
+
+        new-run-webkit-tests: clean up the options-parsing code in the port
+        classes.
+        
+        This change modifies the Port interface to have a get_option() and
+        set_option_default() method for accessing the options argument
+        passed to the constructor. If the constructor is not passed an
+        options argument, we default to a MockOptions() argument from
+        mocktool, which has the same semantics we want.
+
+        Note that there is a disadvantage to port.get_option('foo') over
+        port._options.foo, which is that you lose some of the checking
+        for whether 'foo' is set (typos result in the default value, not
+        an exception being raised. This is desired in this case, since the
+        Port class is not allowed to assume that options does have any
+        particular values set, and so this change ensures that all of
+        the subclasses are following the same, intended, logic.
+
+        Arguably this is the wrong semantics to have, and the Port
+        classes should be able to assume a default set of
+        attributes/arguments, but that change will need to wait for a
+        different CL where we can modify new-run-webkit-tests to pull a
+        list of arguments from the port factory routines.
+
+        Also, add unit tests for webkitpy.tool.mocktool.MockOptions .
+
+        https://bugs.webkit.org/show_bug.cgi?id=47510
+
+        * 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_gpu_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_linux.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_mac.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/chromium_win.py:
+        * Scripts/webkitpy/layout_tests/port/dryrun.py:
+        * Scripts/webkitpy/layout_tests/port/factory_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/mac_unittest.py:
+        * Scripts/webkitpy/layout_tests/port/port_testcase.py:
+        * Scripts/webkitpy/layout_tests/port/test.py:
+        * Scripts/webkitpy/layout_tests/port/webkit.py:
+        * Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py:
+        * Scripts/webkitpy/tool/mocktool_unittest.py: Added.
+
 2010-10-18  Dirk Pranke  <dpranke@chromium.org>
 
         Reviewed by Eric Seidel.
index cb96913582d6f177cf2abbdda875ee7ce6b6174e..96360e7d066833445b16edb5c6c9a19f6688e4cf 100644 (file)
@@ -56,6 +56,22 @@ from webkitpy.common.system.user import User
 _log = logutils.get_logger(__file__)
 
 
+class DummyOptions(object):
+    """Fake implementation of optparse.Values. Cloned from
+    webkitpy.tool.mocktool.MockOptions.
+
+    """
+
+    def __init__(self, **kwargs):
+        # The caller can set option values using keyword arguments. We don't
+        # set any values by default because we don't know how this
+        # object will be used. Generally speaking unit tests should
+        # subclass this or provider wrapper functions that set a common
+        # set of options.
+        for key, value in kwargs.items():
+            self.__dict__[key] = value
+
+
 # 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.
@@ -71,7 +87,12 @@ class Port(object):
 
     def __init__(self, **kwargs):
         self._name = kwargs.get('port_name', None)
-        self._options = kwargs.get('options', None)
+        self._options = kwargs.get('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._helper = None
@@ -98,6 +119,9 @@ class Port(object):
         self._pretty_patch_path = self.path_from_webkit_base("BugsSite",
               "PrettyPatch", "prettify.rb")
         self._pretty_patch_available = True
+        self.set_option_default('configuration', None)
+        if self._options.configuration is None:
+            self._options.configuration = self.default_configuration()
 
     def default_child_processes(self):
         """Return the number of DumpRenderTree instances to use for this
@@ -433,6 +457,18 @@ class Port(object):
         may be different (e.g., 'win-xp' instead of 'chromium-win-xp'."""
         return self._name
 
+    def get_option(self, name, default_value=None):
+        # FIXME: Eventually we should not have to do a test for
+        # hasattr(), and we should be able to just do
+        # self.options.value. See additional FIXME in the constructor.
+        if hasattr(self._options, name):
+            return getattr(self._options, name)
+        return default_value
+
+    def set_option_default(self, name, default_value):
+        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
@@ -497,12 +533,12 @@ class Port(object):
         """Start a web server if it is available. Do nothing if
         it isn't. This routine is allowed to (and may) fail if a server
         is already running."""
-        if self._options.use_apache:
+        if self.get_option('use_apache'):
             self._http_server = apache_http_server.LayoutTestApacheHttpd(self,
-                self._options.results_directory)
+                self.get_option('results_directory'))
         else:
             self._http_server = http_server.Lighttpd(self,
-                self._options.results_directory)
+                self.get_option('results_directory'))
         self._http_server.start()
 
     def start_websocket_server(self):
@@ -510,7 +546,7 @@ class Port(object):
         it isn't. This routine is allowed to (and may) fail if a server
         is already running."""
         self._websocket_server = websocket_server.PyWebSocket(self,
-            self._options.results_directory)
+            self.get_option('results_directory'))
         self._websocket_server.start()
 
     def acquire_http_lock(self):
index baecad9d4ab2571e876f21e3636d54c6d5b1012c..93f88082ab38a6fe92dcd3f9a3b3c243a4e905fa 100644 (file)
@@ -26,7 +26,7 @@
 # (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 base
+import optparse
 import os
 import StringIO
 import sys
@@ -36,7 +36,9 @@ import unittest
 from webkitpy.common.system.path import abspath_to_uri
 from webkitpy.common.system.executive import Executive, ScriptError
 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.
@@ -247,6 +249,41 @@ class PortTest(unittest.TestCase):
         self.assertEqual(port.filename_to_uri(test_file),
                          abspath_to_uri(test_file))
 
+    def test_get_option__set(self):
+        options, args = optparse.OptionParser().parse_args()
+        options.foo = 'bar'
+        port = base.Port(options=options)
+        self.assertEqual(port.get_option('foo'), 'bar')
+
+    def test_get_option__unset(self):
+        port = base.Port()
+        self.assertEqual(port.get_option('foo'), None)
+
+    def test_get_option__default(self):
+        port = base.Port()
+        self.assertEqual(port.get_option('foo', 'bar'), 'bar')
+
+    def test_set_option_default__unset(self):
+        port = base.Port()
+        port.set_option_default('foo', 'bar')
+        self.assertEqual(port.get_option('foo'), 'bar')
+
+    def test_set_option_default__set(self):
+        options, args = optparse.OptionParser().parse_args()
+        options.foo = 'bar'
+        port = base.Port(options=options)
+        # This call should have no effect.
+        port.set_option_default('foo', 'new_bar')
+        self.assertEqual(port.get_option('foo'), 'bar')
+
+    def test_name__unset(self):
+        port = base.Port()
+        self.assertEqual(port.name(), None)
+
+    def test_name__set(self):
+        port = base.Port(port_name='foo')
+        self.assertEqual(port.name(), 'foo')
+
 
 class VirtualTest(unittest.TestCase):
     """Tests that various methods expected to be virtual are."""
index 8356bd90dfb066583d63713cfbc30771b76761b4..4d17b51c828a72e33bd9ac23e1946466e5032beb 100644 (file)
@@ -88,11 +88,6 @@ class ChromiumPort(base.Port):
 
     def __init__(self, **kwargs):
         base.Port.__init__(self, **kwargs)
-        if 'options' in kwargs:
-            options = kwargs['options']
-            if (options and (not hasattr(options, 'configuration') or
-                             options.configuration is None)):
-                options.configuration = self.default_configuration()
         self._chromium_base_dir = None
 
     def baseline_path(self):
@@ -104,9 +99,9 @@ class ChromiumPort(base.Port):
         dump_render_tree_binary_path = self._path_to_driver()
         result = check_file_exists(dump_render_tree_binary_path,
                                     'test driver') and result
-        if result and self._options.build:
+        if result and self.get_option('build'):
             result = self._check_driver_build_up_to_date(
-                self._options.configuration)
+                self.get_option('configuration'))
         else:
             _log.error('')
 
@@ -115,7 +110,7 @@ class ChromiumPort(base.Port):
             result = check_file_exists(helper_path,
                                        'layout test helper') and result
 
-        if self._options.pixel_tests:
+        if self.get_option('pixel_tests'):
             result = self.check_image_diff(
                 'To override, invoke with --no-pixel-tests') and result
 
@@ -205,10 +200,11 @@ class ChromiumPort(base.Port):
     def results_directory(self):
         try:
             return self.path_from_chromium_base('webkit',
-                self._options.configuration, self._options.results_directory)
+                self.get_option('configuration'),
+                self.get_option('results_directory'))
         except AssertionError:
-            return self._build_path(self._options.configuration,
-                                    self._options.results_directory)
+            return self._build_path(self.get_option('configuration'),
+                                    self.get_option('results_directory'))
 
     def setup_test_run(self):
         # Delete the disk cache if any to ensure a clean test run.
@@ -262,7 +258,7 @@ class ChromiumPort(base.Port):
         # FIXME: This drt_overrides handling should be removed when we switch
         # from tes_shell to DRT.
         drt_overrides = ''
-        if self._options and self._options.use_drt:
+        if self.get_option('use_drt'):
             drt_overrides_path = self.path_from_webkit_base('LayoutTests',
                 'platform', 'chromium', 'drt_expectations.txt')
             if os.path.exists(drt_overrides_path):
@@ -357,9 +353,9 @@ class ChromiumPort(base.Port):
 
     def _path_to_image_diff(self):
         binary_name = 'image_diff'
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             binary_name = 'ImageDiff'
-        return self._build_path(self._options.configuration, binary_name)
+        return self._build_path(self.get_option('configuration'), binary_name)
 
 
 class ChromiumDriver(base.Driver):
@@ -378,27 +374,27 @@ class ChromiumDriver(base.Driver):
             driver_args.append("--pixel-tests=" +
                                self._port._convert_path(self._image_path))
 
-        if self._options.use_drt:
+        if self._port.get_option('use_drt'):
             driver_args.append('--test-shell')
         else:
             driver_args.append('--layout-tests')
 
-        if self._options.startup_dialog:
+        if self._port.get_option('startup_dialog'):
             driver_args.append('--testshell-startup-dialog')
 
-        if self._options.gp_fault_error_box:
+        if self._port.get_option('gp_fault_error_box'):
             driver_args.append('--gp-fault-error-box')
 
-        if self._options.accelerated_compositing:
+        if self._port.get_option('accelerated_compositing'):
             driver_args.append('--enable-accelerated-compositing')
 
-        if self._options.accelerated_2d_canvas:
+        if self._port.get_option('accelerated_2d_canvas'):
             driver_args.append('--enable-accelerated-2d-canvas')
         return driver_args
 
     def start(self):
         # FIXME: Should be an error to call this method twice.
-        cmd = self._command_wrapper(self._options.wrapper)
+        cmd = self._command_wrapper(self._port.get_option('wrapper'))
         cmd.append(self._port._path_to_driver())
         cmd += self._driver_args()
 
index 1dd3eaed8ca4209d6720081437410e572f66f696..3f13b93f805ab082e7bdd7efe68579a85455e83c 100644 (file)
 
 import os
 import unittest
-import chromium_gpu
-
 
-class MockOptions(object):
-    def __init__(self):
-        self.accelerated_compositing = None
-        self.accelerated_2d_canvas = None
+from webkitpy.tool import mocktool
+import chromium_gpu
 
 
 class ChromiumGpuTest(unittest.TestCase):
@@ -47,7 +43,9 @@ class ChromiumGpuTest(unittest.TestCase):
 
     def assertOverridesWorked(self, port_name):
         # test that we got the right port
-        port = chromium_gpu.get(port_name=port_name, options=MockOptions())
+        mock_options = mocktool.MockOptions(accelerated_compositing=None,
+                                            accelerated_2d_canvas=None)
+        port = chromium_gpu.get(port_name=port_name, options=mock_options)
         self.assertTrue(port._options.accelerated_compositing)
         self.assertTrue(port._options.accelerated_2d_canvas)
 
index 176991b7f6a68df779f71d511044132869e1e189..b26a6b5741573f2b7e43692b8d1b3a691d8ded07 100644 (file)
@@ -52,7 +52,7 @@ class ChromiumLinuxPort(chromium.ChromiumPort):
     def check_build(self, needs_http):
         result = chromium.ChromiumPort.check_build(self, needs_http)
         if needs_http:
-            if self._options.use_apache:
+            if self.get_option('use_apache'):
                 result = self._check_apache_install() and result
             else:
                 result = self._check_lighttpd_install() and result
@@ -81,7 +81,7 @@ class ChromiumLinuxPort(chromium.ChromiumPort):
         base = self.path_from_chromium_base()
         if os.path.exists(os.path.join(base, 'sconsbuild')):
             return os.path.join(base, 'sconsbuild', *comps)
-        if os.path.exists(os.path.join(base, 'out', *comps)) or not self._options.use_drt:
+        if os.path.exists(os.path.join(base, 'out', *comps)) or not self.get_option('use_drt'):
             return os.path.join(base, 'out', *comps)
         base = self.path_from_webkit_base()
         if os.path.exists(os.path.join(base, 'sconsbuild')):
@@ -147,9 +147,9 @@ class ChromiumLinuxPort(chromium.ChromiumPort):
 
     def _path_to_driver(self, configuration=None):
         if not configuration:
-            configuration = self._options.configuration
+            configuration = self.get_option('configuration')
         binary_name = 'test_shell'
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             binary_name = 'DumpRenderTree'
         return self._build_path(configuration, binary_name)
 
index 64016ab042cae8a8e27bdcaab461cf5cae3a2f81..d1c383c13ec2f13af5fb1f1540579c4c889fdd63 100644 (file)
@@ -73,7 +73,7 @@ class ChromiumMacPort(chromium.ChromiumPort):
 
     def driver_name(self):
         """name for this port's equivalent of DumpRenderTree."""
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             return "DumpRenderTree"
         return "TestShell"
 
@@ -100,7 +100,7 @@ class ChromiumMacPort(chromium.ChromiumPort):
 
     def _build_path(self, *comps):
         path = self.path_from_chromium_base('xcodebuild', *comps)
-        if os.path.exists(path) or not self._options.use_drt:
+        if os.path.exists(path) or not self.get_option('use_drt'):
             return path
         return self.path_from_webkit_base('WebKit', 'chromium', 'xcodebuild',
                                           *comps)
@@ -138,15 +138,15 @@ class ChromiumMacPort(chromium.ChromiumPort):
         # FIXME: make |configuration| happy with case-sensitive file
         # systems.
         if not configuration:
-            configuration = self._options.configuration
+            configuration = self.get_option('configuration')
         return self._build_path(configuration, self.driver_name() + '.app',
             'Contents', 'MacOS', self.driver_name())
 
     def _path_to_helper(self):
         binary_name = 'layout_test_helper'
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             binary_name = 'LayoutTestHelper'
-        return self._build_path(self._options.configuration, binary_name)
+        return self._build_path(self.get_option('configuration'), binary_name)
 
     def _path_to_wdiff(self):
         return 'wdiff'
index 2e9461a3f470ab1a28df8ee8a523972fc3f8f529..cb4543043e426b31b9398de3335903d8efcfd513 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 chromium
-import chromium_linux
-import chromium_mac
-import chromium_win
+import os
 import unittest
 import StringIO
-import os
 
+from webkitpy.tool import mocktool
 from webkitpy.thirdparty.mock import Mock
 
+import chromium
+import chromium_linux
+import chromium_mac
+import chromium_win
 
 class ChromiumDriverTest(unittest.TestCase):
 
     def setUp(self):
         mock_port = Mock()
-        # FIXME: The Driver should not be grabbing at port._options!
-        mock_port._options = Mock()
-        mock_port._options.wrapper = ""
         self.driver = chromium.ChromiumDriver(mock_port, image_path=None, options=None)
 
     def test_test_shell_command(self):
@@ -106,25 +104,19 @@ class ChromiumPortTest(unittest.TestCase):
             return 'default'
 
     def test_path_to_image_diff(self):
-        class MockOptions:
-            def __init__(self):
-                self.use_drt = True
-
-        port = ChromiumPortTest.TestLinuxPort(options=MockOptions())
+        mock_options = mocktool.MockOptions(use_drt=True)
+        port = ChromiumPortTest.TestLinuxPort(options=mock_options)
         self.assertTrue(port._path_to_image_diff().endswith(
             '/out/default/ImageDiff'), msg=port._path_to_image_diff())
-        port = ChromiumPortTest.TestMacPort(options=MockOptions())
+        port = ChromiumPortTest.TestMacPort(options=mock_options)
         self.assertTrue(port._path_to_image_diff().endswith(
             '/xcodebuild/default/ImageDiff'))
         # FIXME: Figure out how this is going to work on Windows.
         #port = chromium_win.ChromiumWinPort('test-port', options=MockOptions())
 
     def test_skipped_layout_tests(self):
-        class MockOptions:
-            def __init__(self):
-                self.use_drt = True
-
-        port = ChromiumPortTest.TestLinuxPort(options=MockOptions())
+        mock_options = mocktool.MockOptions(use_drt=True)
+        port = ChromiumPortTest.TestLinuxPort(options=mock_options)
 
         fake_test = os.path.join(port.layout_tests_dir(), "fast/js/not-good.js")
 
@@ -138,22 +130,14 @@ DEFER LINUX WIN : fast/js/very-good.js = TIMEOUT PASS"""
         self.assertTrue("fast/js/not-good.js" in skipped_tests)
 
     def test_default_configuration(self):
-        class EmptyOptions:
-            def __init__(self):
-                pass
-
-        options = EmptyOptions()
-        port = ChromiumPortTest.TestLinuxPort(options)
-        self.assertEquals(options.configuration, 'default')
+        mock_options = mocktool.MockOptions()
+        port = ChromiumPortTest.TestLinuxPort(options=mock_options)
+        self.assertEquals(mock_options.configuration, 'default')
         self.assertTrue(port.default_configuration_called)
 
-        class OptionsWithUnsetConfiguration:
-            def __init__(self):
-                self.configuration = None
-
-        options = OptionsWithUnsetConfiguration()
-        port = ChromiumPortTest.TestLinuxPort(options)
-        self.assertEquals(options.configuration, 'default')
+        mock_options = mocktool.MockOptions(configuration=None)
+        port = ChromiumPortTest.TestLinuxPort(mock_options)
+        self.assertEquals(mock_options.configuration, 'default')
         self.assertTrue(port.default_configuration_called)
 
     def test_diff_image(self):
@@ -161,9 +145,6 @@ DEFER LINUX WIN : fast/js/very-good.js = TIMEOUT PASS"""
             def _path_to_image_diff(self):
                 return "/path/to/image_diff"
 
-        class EmptyOptions:
-            use_drt = False
-
         class MockExecute:
             def __init__(self, result):
                 self._result = result
@@ -180,8 +161,8 @@ DEFER LINUX WIN : fast/js/very-good.js = TIMEOUT PASS"""
                     return self._result
                 return ''
 
-        options = EmptyOptions()
-        port = ChromiumPortTest.TestLinuxPort(options)
+        mock_options = mocktool.MockOptions(use_drt=False)
+        port = ChromiumPortTest.TestLinuxPort(mock_options)
 
         # Images are different.
         port._executive = MockExecute(0)
index 67d2ab2f1dc9074fdaa627ec2a4d09f33b8cb560..69b529ae1b43ab4ed663e0f6eb41657ea6fb7458 100644 (file)
@@ -55,9 +55,7 @@ class ChromiumWinPort(chromium.ChromiumPort):
         # python executable to run cgi program.
         env["CYGWIN_PATH"] = self.path_from_chromium_base(
             "third_party", "cygwin", "bin")
-        if (sys.platform == "win32" and self._options and
-            hasattr(self._options, "register_cygwin") and
-            self._options.register_cygwin):
+        if (sys.platform == "win32" and self.get_option('register_cygwin')):
             setup_mount = self.path_from_chromium_base("third_party",
                                                        "cygwin",
                                                        "setup_mount.bat")
@@ -113,7 +111,7 @@ class ChromiumWinPort(chromium.ChromiumPort):
         if os.path.exists(p):
             return p
         p = self.path_from_chromium_base('chrome', *comps)
-        if os.path.exists(p) or not self._options.use_drt:
+        if os.path.exists(p) or not self.get_option('use_drt'):
             return p
         return os.path.join(self.path_from_webkit_base(), 'WebKit', 'chromium',
                             *comps)
@@ -141,23 +139,23 @@ class ChromiumWinPort(chromium.ChromiumPort):
 
     def _path_to_driver(self, configuration=None):
         if not configuration:
-            configuration = self._options.configuration
+            configuration = self.get_option('configuration')
         binary_name = 'test_shell.exe'
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             binary_name = 'DumpRenderTree.exe'
         return self._build_path(configuration, binary_name)
 
     def _path_to_helper(self):
         binary_name = 'layout_test_helper.exe'
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             binary_name = 'LayoutTestHelper.exe'
-        return self._build_path(self._options.configuration, binary_name)
+        return self._build_path(self.get_option('configuration'), binary_name)
 
     def _path_to_image_diff(self):
         binary_name = 'image_diff.exe'
-        if self._options.use_drt:
+        if self.get_option('use_drt'):
             binary_name = 'ImageDiff.exe'
-        return self._build_path(self._options.configuration, binary_name)
+        return self._build_path(self.get_option('configuration'), binary_name)
 
     def _path_to_wdiff(self):
         return self.path_from_chromium_base('third_party', 'cygwin', 'bin',
index 648ccadaae6f45f7d06b2b5eb6cb1edd2ff58b10..8a6af56c8420e450c41897afe543e66706f9ae76 100644 (file)
@@ -101,7 +101,6 @@ class DryrunDriver(base.Driver):
 
     def __init__(self, port, image_path, options, executive):
         self._port = port
-        self._options = options
         self._image_path = image_path
         self._executive = executive
         self._layout_tests_dir = None
index 81c37327a20992ee0c4c3cd9e84d311d6ea4598b..978a557a57b20a3b8b40e99998e35a85eee2966e 100644 (file)
@@ -29,6 +29,8 @@
 import sys
 import unittest
 
+from webkitpy.tool import mocktool
+
 import chromium_gpu
 import chromium_linux
 import chromium_mac
@@ -52,21 +54,11 @@ class FactoryTest(unittest.TestCase):
     # FIXME: The ports themselves should expose what options they require,
     # instead of passing generic "options".
 
-    class WebKitOptions(object):
-        """Represents the minimum options for WebKit port."""
-        def __init__(self):
-            self.pixel_tests = False
-
-    class ChromiumOptions(WebKitOptions):
-        """Represents minimum options for Chromium port."""
-        def __init__(self):
-            FactoryTest.WebKitOptions.__init__(self)
-            self.chromium = True
-
     def setUp(self):
         self.real_sys_platform = sys.platform
-        self.webkit_options = FactoryTest.WebKitOptions()
-        self.chromium_options = FactoryTest.ChromiumOptions()
+        self.webkit_options = mocktool.MockOptions(pixel_tests=False)
+        self.chromium_options = mocktool.MockOptions(pixel_tests=False,
+                                                    chromium=True)
 
     def tearDown(self):
         sys.platform = self.real_sys_platform
index 327b19ee5fa54503b4afc29b71e00e79bda56f1b..d383a4cd8837e59ca54cdae97961a4a55b1f4004 100644 (file)
@@ -35,7 +35,7 @@ import port_testcase
 
 
 class MacTest(port_testcase.PortTestCase):
-    def make_port(self, options=port_testcase.MockOptions()):
+    def make_port(self, options=port_testcase.mock_options):
         if sys.platform != 'darwin':
             return None
         port_obj = mac.MacPort(options=options)
index 47597d67cc25a638a9f645a7b5e0be29d656155f..04ada5000e3b35382c3c960922e28c553c3f404e 100644 (file)
@@ -32,20 +32,15 @@ import os
 import tempfile
 import unittest
 
-
-class MockOptions(object):
-    def __init__(self,
-                 results_directory='layout-test-results',
-                 use_apache=True,
-                 configuration='Release'):
-        self.results_directory = results_directory
-        self.use_apache = use_apache
-        self.configuration = configuration
+from webkitpy.tool import mocktool
+mock_options = mocktool.MockOptions(results_directory='layout-test-results',
+                                    use_apache=True,
+                                    configuration='Release')
 
 
 class PortTestCase(unittest.TestCase):
     """Tests the WebKit port implementation."""
-    def make_port(self, options=MockOptions()):
+    def make_port(self, options=mock_options):
         """Override in subclass."""
         raise NotImplementedError()
 
index 3b8116799af3cd4eea6eb1dfdb8da904cd5f9800..3691c5aae9fc321c95178636cdcd03b69bddd73f 100644 (file)
@@ -215,14 +215,11 @@ class TestPort(base.Port):
     def name(self):
         return self._name
 
-    def options(self):
-        return self._options
-
     def _path_to_wdiff(self):
         return None
 
     def results_directory(self):
-        return '/tmp/' + self._options.results_directory
+        return '/tmp/' + self.get_option('results_directory')
 
     def setup_test_run(self):
         pass
@@ -285,7 +282,6 @@ class TestDriver(base.Driver):
     def __init__(self, port, image_path, options, executive):
         self._port = port
         self._image_path = image_path
-        self._options = options
         self._executive = executive
         self._image_written = False
 
@@ -302,7 +298,7 @@ class TestDriver(base.Driver):
         if test.hang:
             time.sleep((float(timeoutms) * 4) / 1000.0)
 
-        if self._port.options().pixel_tests and test.actual_image:
+        if self._port.get_option('pixel_tests') and test.actual_image:
             with open(self._image_path, 'w') as file:
                 file.write(test.actual_image)
 
index ed19c09c62dacbee7a3a698d76aaa3d82caebb7c..c940f1e7a070a60740e49be9da6efbbaa9cd6a90 100644 (file)
@@ -65,9 +65,7 @@ class WebKitPort(base.Port):
 
         # FIXME: disable pixel tests until they are run by default on the
         # build machines.
-        if self._options and (not hasattr(self._options, "pixel_tests") or
-           self._options.pixel_tests is None):
-            self._options.pixel_tests = False
+        self.set_option_default('pixel_tests', False)
 
     def baseline_path(self):
         return self._webkit_baseline_path(self._name)
@@ -86,7 +84,7 @@ class WebKitPort(base.Port):
     def _build_driver(self):
         exit_code = self._executive.run_command([
             self.script_path("build-dumprendertree"),
-            self.flag_from_configuration(self._options.configuration),
+            self.flag_from_configuration(self.get_option('configuration')),
         ], return_exit_code=True)
         if exit_code != 0:
             _log.error("Failed to build DumpRenderTree")
@@ -101,11 +99,11 @@ class WebKitPort(base.Port):
         return True
 
     def check_build(self, needs_http):
-        if self._options.build and not self._build_driver():
+        if self.get_option('build') and not self._build_driver():
             return False
         if not self._check_driver():
             return False
-        if self._options.pixel_tests:
+        if self.get_option('pixel_tests'):
             if not self.check_image_diff():
                 return False
         if not self._check_port_build():
@@ -184,7 +182,7 @@ class WebKitPort(base.Port):
     def results_directory(self):
         # Results are store relative to the built products to make it easy
         # to have multiple copies of webkit checked out and built.
-        return self._build_path(self._options.results_directory)
+        return self._build_path(self.get_option('results_directory'))
 
     def setup_test_run(self):
         # This port doesn't require any specific configuration.
@@ -360,7 +358,7 @@ class WebKitPort(base.Port):
         if not self._cached_build_root:
             self._cached_build_root = self._webkit_build_directory([
                 "--configuration",
-                self.flag_from_configuration(self._options.configuration),
+                self.flag_from_configuration(self.get_option('configuration')),
             ])
         return os.path.join(self._cached_build_root, *comps)
 
@@ -401,7 +399,6 @@ class WebKitDriver(base.Driver):
     def __init__(self, port, image_path, options, executive=Executive()):
         self._port = port
         self._image_path = image_path
-        self._options = options
         self._executive = executive
         self._driver_tempdir = tempfile.mkdtemp(prefix='DumpRenderTree-')
 
@@ -414,17 +411,17 @@ class WebKitDriver(base.Driver):
         if self._image_path:
             driver_args.append('--pixel-tests')
 
-        if self._options.use_drt:
-            if self._options.accelerated_compositing:
+        if self._port.get_option('use_drt'):
+            if self._port.get_option('accelerated_compositing'):
                 driver_args.append('--enable-accelerated-compositing')
 
-            if self._options.accelerated_2d_canvas:
+            if self._port.get_option('accelerated_2d_canvas'):
                 driver_args.append('--enable-accelerated-2d-canvas')
 
         return driver_args
 
     def start(self):
-        command = self._command_wrapper(self._options.wrapper)
+        command = self._command_wrapper(self._port.get_option('wrapper'))
         command += [self._port._path_to_driver(), '-']
         command += self._driver_args()
 
index fce6e82caef835a73398084b69148008db26ae68..ef33a47481fdefd472246e6ef2093f3595245392 100644 (file)
@@ -33,6 +33,7 @@ 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
@@ -46,12 +47,6 @@ class MockPort(object):
         return self.image_diff_exists
 
 
-class MockOptions(object):
-    def __init__(self, configuration=None, html_directory=None):
-        self.configuration = configuration
-        self.html_directory = html_directory
-
-
 def get_mock_get(config_expectations):
     def mock_get(port_name, options):
         return MockPort(config_expectations[options.configuration])
@@ -64,7 +59,8 @@ class TestGetHostPortObject(unittest.TestCase):
         # that Image diff is (or isn't) present in the two configs.
         port.get = get_mock_get({'Release': release_present,
                                  'Debug': debug_present})
-        options = MockOptions()
+        options = mocktool.MockOptions(configuration=None,
+                                       html_directory=None)
         port_obj = rebaseline_chromium_webkit_tests.get_host_port_object(
             options)
         if valid_port_obj:
@@ -90,7 +86,8 @@ class TestGetHostPortObject(unittest.TestCase):
 
 class TestRebaseliner(unittest.TestCase):
     def make_rebaseliner(self):
-        options = MockOptions()
+        options = mocktool.MockOptions(configuration=None,
+                                       html_directory=None)
         host_port_obj = port.get('test', options)
         target_options = options
         target_port_obj = port.get('test', target_options)
@@ -126,7 +123,8 @@ class TestHtmlGenerator(unittest.TestCase):
     def make_generator(self, tests):
         return rebaseline_chromium_webkit_tests.HtmlGenerator(
             target_port=None,
-            options=MockOptions(html_directory="/tmp"),
+            options=mocktool.MockOptions(configuration=None,
+                                         html_directory='/tmp'),
             platforms=['mac'],
             rebaselining_tests=tests,
             executive=Executive())
diff --git a/WebKitTools/Scripts/webkitpy/tool/mocktool_unittest.py b/WebKitTools/Scripts/webkitpy/tool/mocktool_unittest.py
new file mode 100644 (file)
index 0000000..cceaa2e
--- /dev/null
@@ -0,0 +1,59 @@
+# Copyright (C) 2010 Google Inc. All rights reserved.
+#
+# Redistribution and use in source and binary forms, with or without
+# modification, are permitted provided that the following conditions are
+# met:
+#
+#    * Redistributions of source code must retain the above copyright
+# notice, this list of conditions and the following disclaimer.
+#    * Redistributions in binary form must reproduce the above
+# copyright notice, this list of conditions and the following disclaimer
+# in the documentation and/or other materials provided with the
+# distribution.
+#    * Neither the name of Google Inc. nor the names of its
+# contributors may be used to endorse or promote products derived from
+# this software without specific prior written permission.
+#
+# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+# "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+# LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+# A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+# OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+# SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+# LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+# DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+# THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+# OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+import unittest
+
+from mocktool import MockOptions
+
+
+class MockOptionsTest(unittest.TestCase):
+    # MockOptions() should implement the same semantics that
+    # optparse.Values does.
+
+    def test_get__set(self):
+        # Test that we can still set options after we construct the
+        # object.
+        options = MockOptions()
+        options.foo = 'bar'
+        self.assertEqual(options.foo, 'bar')
+
+    def test_get__unset(self):
+        # Test that unset options raise an exception (regular Mock
+        # objects return an object and hence are different from
+        # optparse.Values()).
+        options = MockOptions()
+        self.assertRaises(AttributeError, lambda: options.foo)
+
+    def test_kwarg__set(self):
+        # Test that keyword arguments work in the constructor.
+        options = MockOptions(foo='bar')
+        self.assertEqual(options.foo, 'bar')
+
+
+if __name__ == '__main__':
+    unittest.main()