Title: [120488] trunk/Tools
Revision
120488
Author
[email protected]
Date
2012-06-15 13:25:37 -0700 (Fri, 15 Jun 2012)

Log Message

webkitpy: remove DummyOptions and clean up the code in Port.get_option() and Port.set_option_default()
https://bugs.webkit.org/show_bug.cgi?id=89135

Re-land change in r120370 with fix in
PortFactory.get_from_builder_name() that changes BuilderOptions
to an actual optparse.Values object.

* Scripts/webkitpy/layout_tests/port/base.py:
(Port.__init__):
(Port.get_option):
(Port.set_option_default):
* Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py:
(ChromiumWinTest.test_setup_environ_for_server_register_cygwin):
* Scripts/webkitpy/layout_tests/port/factory.py:
(_builder_options):
(PortFactory.get_from_builder_name):
* Scripts/webkitpy/layout_tests/port/factory_unittest.py:
(FactoryTest.test_get_from_builder_name):
* Scripts/webkitpy/style/checkers/test_expectations.py:
(TestExpectationsChecker._determine_port_from_expectations_path):
* Scripts/webkitpy/tool/mocktool.py:
(MockOptions.ensure_value):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (120487 => 120488)


--- trunk/Tools/ChangeLog	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/ChangeLog	2012-06-15 20:25:37 UTC (rev 120488)
@@ -1,3 +1,28 @@
+2012-06-15  Dirk Pranke  <[email protected]>
+
+        webkitpy: remove DummyOptions and clean up the code in Port.get_option() and Port.set_option_default()
+        https://bugs.webkit.org/show_bug.cgi?id=89135
+
+        Re-land change in r120370 with fix in
+        PortFactory.get_from_builder_name() that changes BuilderOptions
+        to an actual optparse.Values object.
+
+        * Scripts/webkitpy/layout_tests/port/base.py:
+        (Port.__init__):
+        (Port.get_option):
+        (Port.set_option_default):
+        * Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py:
+        (ChromiumWinTest.test_setup_environ_for_server_register_cygwin):
+        * Scripts/webkitpy/layout_tests/port/factory.py:
+        (_builder_options):
+        (PortFactory.get_from_builder_name):
+        * Scripts/webkitpy/layout_tests/port/factory_unittest.py:
+        (FactoryTest.test_get_from_builder_name):
+        * Scripts/webkitpy/style/checkers/test_expectations.py:
+        (TestExpectationsChecker._determine_port_from_expectations_path):
+        * Scripts/webkitpy/tool/mocktool.py:
+        (MockOptions.ensure_value):
+
 2012-06-15  Bill Budge  <[email protected]>
 
         Add [email protected] to committers.py

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (120487 => 120488)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py	2012-06-15 20:25:37 UTC (rev 120488)
@@ -34,6 +34,7 @@
 import difflib
 import errno
 import os
+import optparse
 import re
 
 try:
@@ -61,19 +62,6 @@
 _log = logutils.get_logger(__file__)
 
 
-class DummyOptions(object):
-    """Fake implementation of optparse.Values. Cloned from webkitpy.tool.mocktool.MockOptions."""
-
-    def __init__(self, *args, **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 WebKitPort now that Chromium behaves mostly like other webkit ports.
 class Port(object):
     """Abstract class for Port-specific hooks for the layout_test package."""
@@ -111,7 +99,7 @@
         # 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 = options or DummyOptions()
+        self._options = options or optparse.Values()
 
         self.host = host
         self._executive = host.executive
@@ -678,18 +666,10 @@
         return self._architecture
 
     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
+        return getattr(self._options, name, default_value)
 
     def set_option_default(self, name, default_value):
-        # FIXME: Callers could also use optparse_parser.Values.ensure_value,
-        # since this should always be a optparse_parser.Values object.
-        if not hasattr(self._options, name) or getattr(self._options, name) is None:
-            return setattr(self._options, name, default_value)
+        return self._options.ensure_value(name, default_value)
 
     def path_from_webkit_base(self, *comps):
         """Returns the full path to path made by joining the top of the

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py (120487 => 120488)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/chromium_win_unittest.py	2012-06-15 20:25:37 UTC (rev 120488)
@@ -38,11 +38,6 @@
 
 
 class ChromiumWinTest(port_testcase.PortTestCase):
-    class RegisterCygwinOption(object):
-        def __init__(self):
-            self.register_cygwin = True
-            self.results_directory = '/'
-
     port_name = 'chromium-win'
     port_maker = chromium_win.ChromiumWinPort
     os_name = 'win'
@@ -67,7 +62,7 @@
         self.assertEquals(env['CYGWIN_PATH'], '/mock-checkout/Source/WebKit/chromium/third_party/cygwin/bin')
 
     def test_setup_environ_for_server_register_cygwin(self):
-        port = self.make_port(options=ChromiumWinTest.RegisterCygwinOption())
+        port = self.make_port(options=MockOptions(register_cygwin=True, results_directory='/'))
         port._executive = MockExecutive(should_log=True)
         expected_stderr = "MOCK run_command: ['/mock-checkout/Source/WebKit/chromium/third_party/cygwin/setup_mount.bat'], cwd=None\n"
         output = outputcapture.OutputCapture()

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py (120487 => 120488)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/factory.py	2012-06-15 20:25:37 UTC (rev 120488)
@@ -58,10 +58,10 @@
             help='use 32-bit binaries by default (x86 instead of x86_64)')]
 
 
-class BuilderOptions(object):
-    def __init__(self, builder_name):
-        self.configuration = "Debug" if re.search(r"[d|D](ebu|b)g", builder_name) else "Release"
-        self.builder_name = builder_name
+def _builder_options(builder_name):
+    configuration = "Debug" if re.search(r"[d|D](ebu|b)g", builder_name) else "Release"
+    builder_name = builder_name
+    return optparse.Values({'builder_name': builder_name, 'configuration': configuration})
 
 
 class PortFactory(object):
@@ -129,6 +129,6 @@
     def get_from_builder_name(self, builder_name):
         port_name = builders.port_name_for_builder_name(builder_name)
         assert(port_name)  # Need to update port_name_for_builder_name
-        port = self.get(port_name, BuilderOptions(builder_name))
+        port = self.get(port_name, _builder_options(builder_name))
         assert(port)  # Need to update port_name_for_builder_name
         return port

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py (120487 => 120488)


--- trunk/Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/factory_unittest.py	2012-06-15 20:25:37 UTC (rev 120488)
@@ -120,6 +120,10 @@
     def test_unknown_default(self):
         self.assertRaises(NotImplementedError, factory.PortFactory(MockSystemHost(os_name='vms')).get)
 
+    def test_get_from_builder_name(self):
+        self.assertEquals(factory.PortFactory(MockSystemHost()).get_from_builder_name('Webkit Mac10.7').name(),
+                          'chromium-mac-lion')
 
+
 if __name__ == '__main__':
     unittest.main()

Modified: trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py (120487 => 120488)


--- trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/Scripts/webkitpy/style/checkers/test_expectations.py	2012-06-15 20:25:37 UTC (rev 120488)
@@ -29,6 +29,7 @@
 """Checks WebKit style for test_expectations files."""
 
 import logging
+import optparse
 import os
 import re
 import sys
@@ -36,7 +37,6 @@
 from common import TabChecker
 from webkitpy.common.host import Host
 from webkitpy.layout_tests.models.test_expectations import TestExpectationParser
-from webkitpy.layout_tests.port.base import DummyOptions
 
 
 _log = logging.getLogger(__name__)
@@ -49,7 +49,7 @@
 
     def _determine_port_from_expectations_path(self, host, expectations_path):
         # Pass a configuration to avoid calling default_configuration() when initializing the port (takes 0.5 seconds on a Mac Pro!).
-        options = DummyOptions(configuration='Release')
+        options = optparse.Values({'configuration': 'Release'})
         for port_name in host.port_factory.all_port_names():
             port = host.port_factory.get(port_name, options=options)
             if port.path_to_test_expectations_file().replace(port.path_from_webkit_base() + host.filesystem.sep, '') == expectations_path:

Modified: trunk/Tools/Scripts/webkitpy/tool/mocktool.py (120487 => 120488)


--- trunk/Tools/Scripts/webkitpy/tool/mocktool.py	2012-06-15 20:00:56 UTC (rev 120487)
+++ trunk/Tools/Scripts/webkitpy/tool/mocktool.py	2012-06-15 20:25:37 UTC (rev 120488)
@@ -36,8 +36,7 @@
 from webkitpy.common.config.ports_mock import MockPort
 
 
-# FIXME: This should be moved somewhere in common and renamed
-# something without Mock in the name.
+# FIXME: We should just replace this with optparse.Values(default=kwargs)
 class MockOptions(object):
     """Mock implementation of optparse.Values."""
 
@@ -53,7 +52,12 @@
         self.__dict__.update(**kwargs)
         return self
 
+    def ensure_value(self, key, value):
+        if getattr(self, key, None) == None:
+            self.__dict__[key] = value
+        return self.__dict__[key]
 
+
 # FIXME: This should be renamed MockWebKitPatch.
 class MockTool(MockHost):
     def __init__(self, *args, **kwargs):
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to