[SCM] WebKit Debian packaging branch, debian/experimental, updated. upstream/1.3.3-9427-gc2be6fc

dpranke at chromium.org dpranke at chromium.org
Wed Dec 22 14:44:28 UTC 2010


The following commit has been merged in the debian/experimental branch:
commit 117ab79dd7b02e96e8b0a278756251c01631eb33
Author: dpranke at chromium.org <dpranke at chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Date:   Tue Oct 19 01:02:35 2010 +0000

    2010-10-18  Dirk Pranke  <dpranke at 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.
    
    git-svn-id: http://svn.webkit.org/repository/webkit/trunk@70013 268f45cc-cd09-0410-ab3c-d52691b4dbfc

diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog
index 2a9a3f7..9170b90 100644
--- a/WebKitTools/ChangeLog
+++ b/WebKitTools/ChangeLog
@@ -1,5 +1,53 @@
 2010-10-18  Dirk Pranke  <dpranke at 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 at chromium.org>
+
         Reviewed by Eric Seidel.
 
         Re-submit a revised version of r69638 - enabling new-run-webkit-tests
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
index cb96913..96360e7 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
@@ -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):
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
index baecad9..93f8808 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base_unittest.py
@@ -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."""
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
index 8356bd9..4d17b51 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium.py
@@ -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()
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py
index 1dd3eae..3f13b93 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_gpu_unittest.py
@@ -26,13 +26,9 @@
 
 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)
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py
index 176991b..b26a6b5 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_linux.py
@@ -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)
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py
index 64016ab..d1c383c 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_mac.py
@@ -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'
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
index 2e9461a..cb45430 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_unittest.py
@@ -26,24 +26,22 @@
 # (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)
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py
index 67d2ab2..69b529a 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/chromium_win.py
@@ -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',
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py
index 648ccad..8a6af56 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/dryrun.py
@@ -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
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/factory_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/factory_unittest.py
index 81c3732..978a557 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/factory_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/factory_unittest.py
@@ -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
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
index 327b19e..d383a4c 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/mac_unittest.py
@@ -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)
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/port_testcase.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/port_testcase.py
index 47597d6..04ada50 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/port_testcase.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/port_testcase.py
@@ -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()
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
index 3b81167..3691c5a 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/test.py
@@ -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)
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
index ed19c09..c940f1e 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/webkit.py
@@ -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()
 
diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py
index fce6e82..ef33a47 100644
--- a/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py
+++ b/WebKitTools/Scripts/webkitpy/layout_tests/rebaseline_chromium_webkit_tests_unittest.py
@@ -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
index 0000000..cceaa2e
--- /dev/null
+++ b/WebKitTools/Scripts/webkitpy/tool/mocktool_unittest.py
@@ -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()

-- 
WebKit Debian packaging



More information about the Pkg-webkit-commits mailing list