Diff
Modified: trunk/Tools/ChangeLog (98876 => 98877)
--- trunk/Tools/ChangeLog 2011-10-31 20:31:26 UTC (rev 98876)
+++ trunk/Tools/ChangeLog 2011-10-31 20:50:51 UTC (rev 98877)
@@ -1,3 +1,25 @@
+2011-10-31 Eric Seidel <[email protected]>
+
+ webkitpy tests depend too much on the user's environment
+ https://bugs.webkit.org/show_bug.cgi?id=71234
+
+ Reviewed by Dirk Pranke.
+
+ This change just makes a bunch of our older tests use
+ more modern mocking to avoid trying to launch processes
+ or read from the user's filesystem during unittesting.
+
+ I found many of these by adding an assert in Executive.run_command
+ that we were not unittesting. I can't add that assert always
+ as there are some valid uses of Executive during unittesting.
+ Once I fix more of these, I may find a way to add such an assert conditionally.
+
+ * Scripts/webkitpy/common/checkout/baselineoptimizer.py:
+ * Scripts/webkitpy/common/net/credentials_unittest.py:
+ * Scripts/webkitpy/layout_tests/controllers/manager_unittest.py:
+ * Scripts/webkitpy/layout_tests/port/base.py:
+ * Scripts/webkitpy/layout_tests/port/base_unittest.py:
+
2011-10-31 Kenneth Rohde Christiansen <[email protected]>
[Qt] MiniBrowser doesn't resize as the size is always overridden
Modified: trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py (98876 => 98877)
--- trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py 2011-10-31 20:31:26 UTC (rev 98876)
+++ trunk/Tools/Scripts/webkitpy/common/checkout/baselineoptimizer.py 2011-10-31 20:50:51 UTC (rev 98877)
@@ -43,7 +43,9 @@
fallback_path = ['LayoutTests']
for port_name in port_factory.all_port_names():
- port = port_factory.get(port_name)
+ # FIXME: This should pass User and Executive as well to allow for easy mocking.
+ # Alternatively, we should get a pre-mocked PortFactory from tool.
+ port = port_factory.get(port_name, filesystem=fs)
webkit_base = port.webkit_base()
search_path = port.baseline_search_path()
if search_path:
Modified: trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py (98876 => 98877)
--- trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py 2011-10-31 20:31:26 UTC (rev 98876)
+++ trunk/Tools/Scripts/webkitpy/common/net/credentials_unittest.py 2011-10-31 20:50:51 UTC (rev 98877)
@@ -35,6 +35,7 @@
from webkitpy.common.system.executive import Executive
from webkitpy.common.system.outputcapture import OutputCapture
from webkitpy.thirdparty.mock import Mock
+from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive
# FIXME: Other unit tests probably want this class.
@@ -51,6 +52,14 @@
os.rmdir(self._directory_path)
+# Note: All tests should use this class instead of Credentials directly to avoid using a real Executive.
+class MockedCredentials(Credentials):
+ def __init__(self, *args, **kwargs):
+ if 'executive' not in kwargs:
+ kwargs['executive'] = MockExecutive()
+ Credentials.__init__(self, *args, **kwargs)
+
+
class CredentialsTest(unittest.TestCase):
example_security_output = """keychain: "/Users/test/Library/Keychains/login.keychain"
class: "inet"
@@ -79,7 +88,7 @@
"""
def test_keychain_lookup_on_non_mac(self):
- class FakeCredentials(Credentials):
+ class FakeCredentials(MockedCredentials):
def _is_mac_os_x(self):
return False
credentials = FakeCredentials("bugs.webkit.org")
@@ -87,10 +96,11 @@
self.assertEqual(credentials._credentials_from_keychain("foo"), ["foo", None])
def test_security_output_parse(self):
- credentials = Credentials("bugs.webkit.org")
+ credentials = MockedCredentials("bugs.webkit.org")
self.assertEqual(credentials._parse_security_tool_output(self.example_security_output), ["[email protected]", "SECRETSAUCE"])
def test_security_output_parse_entry_not_found(self):
+ # FIXME: This test won't work if the user has a credential for foo.example.com!
credentials = Credentials("foo.example.com")
if not credentials._is_mac_os_x():
return # This test does not run on a non-Mac.
@@ -104,7 +114,7 @@
def _assert_security_call(self, username=None):
executive_mock = Mock()
- credentials = Credentials("example.com", executive=executive_mock)
+ credentials = MockedCredentials("example.com", executive=executive_mock)
expected_stderr = "Reading Keychain for example.com account and password. Click \"Allow\" to continue...\n"
OutputCapture().assert_outputs(self, credentials._run_security_tool, [username], expected_stderr=expected_stderr)
@@ -119,8 +129,7 @@
self._assert_security_call(username="foo")
def test_credentials_from_environment(self):
- executive_mock = Mock()
- credentials = Credentials("example.com", executive=executive_mock)
+ credentials = MockedCredentials("example.com")
saved_environ = os.environ.copy()
os.environ['WEBKIT_BUGZILLA_USERNAME'] = "foo"
@@ -132,7 +141,7 @@
def test_read_credentials_without_git_repo(self):
# FIXME: This should share more code with test_keyring_without_git_repo
- class FakeCredentials(Credentials):
+ class FakeCredentials(MockedCredentials):
def _is_mac_os_x(self):
return True
@@ -155,7 +164,7 @@
def get_password(self, host, username):
return "NOMNOMNOM"
- class FakeCredentials(Credentials):
+ class FakeCredentials(MockedCredentials):
def _is_mac_os_x(self):
return True
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py (98876 => 98877)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2011-10-31 20:31:26 UTC (rev 98876)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_unittest.py 2011-10-31 20:50:51 UTC (rev 98877)
@@ -34,7 +34,7 @@
import sys
import unittest
-from webkitpy.common.system import filesystem_mock
+from webkitpy.common.system.filesystem_mock import MockFileSystem
from webkitpy.common.system import outputcapture
from webkitpy.thirdparty.mock import Mock
from webkitpy import layout_tests
@@ -46,7 +46,7 @@
from webkitpy.layout_tests.controllers.manager import Manager, natural_sort_key, test_key, TestRunInterruptedException, TestShard
from webkitpy.layout_tests.models.result_summary import ResultSummary
from webkitpy.layout_tests.views import printing
-from webkitpy.tool.mocktool import MockOptions
+from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive
class ManagerWrapper(Manager):
@@ -70,7 +70,8 @@
def get_shards(self, num_workers, fully_parallel, test_list=None):
test_list = test_list or self.test_list
port = layout_tests.port.get(port_name='test')
- port._filesystem = filesystem_mock.MockFileSystem()
+ port._filesystem = MockFileSystem()
+ # FIXME: This should use MockOptions() instead of Mock()
self.manager = ManagerWrapper(port=port, options=Mock(), printer=Mock())
return self.manager._shard_tests(test_list, num_workers, fully_parallel)
@@ -183,10 +184,10 @@
self._finished_list_called = True
options, args = run_webkit_tests.parse_args(['--platform=test', '--print=nothing', 'http/tests/passes', 'passes'])
- port = layout_tests.port.get(port_name=options.platform, options=options)
+ # Note we do not pass a filesystem as the "test port" magically suplies its own mock filesystem.
+ port = layout_tests.port.get(port_name=options.platform, options=options, user=MockUser(), executive=MockExecutive())
run_webkit_tests._set_up_derived_options(port, options)
- printer = printing.Printer(port, options, StringIO.StringIO(), StringIO.StringIO(),
- configure_logging=True)
+ printer = printing.Printer(port, options, StringIO.StringIO(), StringIO.StringIO(), configure_logging=True)
manager = LockCheckingManager(port, options, printer)
manager.collect_tests(args)
manager.parse_expectations()
@@ -197,9 +198,9 @@
tester.assertEquals(num_unexpected_results, 0)
def test_interrupt_if_at_failure_limits(self):
- port = Mock()
+ port = Mock() # FIXME: This should be a tighter mock.
port.TEST_PATH_SEPARATOR = '/'
- port._filesystem = filesystem_mock.MockFileSystem()
+ port._filesystem = MockFileSystem()
manager = Manager(port=port, options=MockOptions(), printer=Mock())
manager._options = MockOptions(exit_after_n_failures=None, exit_after_n_crashes_or_timeouts=None)
@@ -225,7 +226,7 @@
def test_needs_servers(self):
def get_manager_with_tests(test_names):
- port = Mock()
+ port = Mock() # FIXME: Use a tighter mock.
port.TEST_PATH_SEPARATOR = '/'
manager = Manager(port, options=MockOptions(http=True), printer=Mock())
manager._test_files = set(test_names)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (98876 => 98877)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-10-31 20:31:26 UTC (rev 98876)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-10-31 20:50:51 UTC (rev 98877)
@@ -553,10 +553,8 @@
for test_or_category in self.skipped_layout_tests():
if test_or_category == test_name:
return True
- category = self._filesystem.join(self.layout_tests_dir(),
- test_or_category)
- if (self._filesystem.isdir(category) and
- test_name.startswith(test_or_category)):
+ category = self._filesystem.join(self.layout_tests_dir(), test_or_category)
+ if self._filesystem.isdir(category) and test_name.startswith(test_or_category):
return True
return False
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py (98876 => 98877)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py 2011-10-31 20:31:26 UTC (rev 98876)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base_unittest.py 2011-10-31 20:50:51 UTC (rev 98877)
@@ -37,7 +37,7 @@
from webkitpy.common.system import outputcapture
from webkitpy.common.system.path import abspath_to_uri
from webkitpy.thirdparty.mock import Mock
-from webkitpy.tool import mocktool
+from webkitpy.tool.mocktool import MockOptions, MockUser, MockExecutive
from webkitpy.layout_tests.port import Port, Driver, DriverOutput
@@ -46,14 +46,20 @@
class PortTest(unittest.TestCase):
+ def make_port(self, *args, **kwargs):
+ kwargs.setdefault('filesystem', MockFileSystem())
+ kwargs.setdefault('user', MockUser())
+ kwargs.setdefault('executive', MockExecutive())
+ return Port(*args, **kwargs)
+
def test_format_wdiff_output_as_html(self):
output = "OUTPUT %s %s %s" % (Port._WDIFF_DEL, Port._WDIFF_ADD, Port._WDIFF_END)
- html = Port()._format_wdiff_output_as_html(output)
+ html = self.make_port()._format_wdiff_output_as_html(output)
expected_html = "<head><style>.del { background: #faa; } .add { background: #afa; }</style></head><pre>OUTPUT <span class=del> <span class=add> </span></pre>"
self.assertEqual(html, expected_html)
def test_wdiff_command(self):
- port = Port()
+ port = self.make_port()
port._path_to_wdiff = lambda: "/path/to/wdiff"
command = port._wdiff_command("/actual/path", "/expected/path")
expected_command = [
@@ -74,7 +80,7 @@
return new_file
def test_pretty_patch_os_error(self):
- port = Port(executive=executive_mock.MockExecutive2(exception=OSError))
+ port = self.make_port(executive=executive_mock.MockExecutive2(exception=OSError))
oc = outputcapture.OutputCapture()
oc.capture_output()
self.assertEqual(port.pretty_patch_text("patch.txt"),
@@ -87,7 +93,7 @@
def test_pretty_patch_script_error(self):
# FIXME: This is some ugly white-box test hacking ...
- port = Port(executive=executive_mock.MockExecutive2(exception=ScriptError))
+ port = self.make_port(executive=executive_mock.MockExecutive2(exception=ScriptError))
port._pretty_patch_available = True
self.assertEqual(port.pretty_patch_text("patch.txt"),
port._pretty_patch_error_html)
@@ -96,7 +102,7 @@
self.assertEqual(port.pretty_patch_text("patch.txt"),
port._pretty_patch_error_html)
- def test_run_wdiff(self):
+ def integration_test_run_wdiff(self):
executive = Executive()
# This may fail on some systems. We could ask the port
# object for the wdiff path, but since we don't know what
@@ -106,7 +112,7 @@
except Exception, e:
wdiff_path = None
- port = Port()
+ port = self.make_port(executive=executive)
port._path_to_wdiff = lambda: wdiff_path
if wdiff_path:
@@ -141,7 +147,7 @@
self.assertFalse(port._wdiff_available)
def test_diff_text(self):
- port = Port()
+ port = self.make_port()
# Make sure that we don't run into decoding exceptions when the
# filenames are unicode, with regular or malformed input (expected or
# actual input is always raw bytes, not unicode).
@@ -169,29 +175,37 @@
def test_default_configuration_notfound(self):
# Test that we delegate to the config object properly.
- port = Port(config=config_mock.MockConfig(default_configuration='default'))
+ port = self.make_port(config=config_mock.MockConfig(default_configuration='default'))
self.assertEqual(port.default_configuration(), 'default')
def test_layout_tests_skipping(self):
- port = Port()
+ filesystem = MockFileSystem({
+ '/mock-checkout/LayoutTests/media/video-zoom.html': '',
+ '/mock-checkout/LayoutTests/foo/bar.html': '',
+ })
+ port = self.make_port(filesystem=filesystem)
port.skipped_layout_tests = lambda: ['foo/bar.html', 'media']
self.assertTrue(port.skips_layout_test('foo/bar.html'))
self.assertTrue(port.skips_layout_test('media/video-zoom.html'))
self.assertFalse(port.skips_layout_test('foo/foo.html'))
def test_setup_test_run(self):
- port = Port()
+ port = self.make_port()
# This routine is a no-op. We just test it for coverage.
port.setup_test_run()
def test_test_dirs(self):
- port = Port()
+ filesystem = MockFileSystem({
+ '/mock-checkout/LayoutTests/canvas/test': '',
+ '/mock-checkout/LayoutTests/css2.1/test': '',
+ })
+ port = self.make_port(filesystem=filesystem)
dirs = port.test_dirs()
self.assertTrue('canvas' in dirs)
self.assertTrue('css2.1' in dirs)
def test_test_to_uri(self):
- port = Port()
+ port = self.make_port()
layout_test_dir = port.layout_tests_dir()
test = 'foo/bar.html'
path = port._filesystem.join(layout_test_dir, test)
@@ -203,28 +217,28 @@
def test_get_option__set(self):
options, args = optparse.OptionParser().parse_args([])
options.foo = 'bar'
- port = Port(options=options)
+ port = self.make_port(options=options)
self.assertEqual(port.get_option('foo'), 'bar')
def test_get_option__unset(self):
- port = Port()
+ port = self.make_port()
self.assertEqual(port.get_option('foo'), None)
def test_get_option__default(self):
- port = Port()
+ port = self.make_port()
self.assertEqual(port.get_option('foo', 'bar'), 'bar')
def test_name__unset(self):
- port = Port()
+ port = self.make_port()
self.assertEqual(port.name(), None)
def test_name__set(self):
- port = Port(port_name='foo')
+ port = self.make_port(port_name='foo')
self.assertEqual(port.name(), 'foo')
def test_additional_platform_directory(self):
filesystem = MockFileSystem()
- port = Port(port_name='foo', filesystem=filesystem)
+ port = self.make_port(port_name='foo', filesystem=filesystem)
port.baseline_search_path = lambda: ['LayoutTests/platform/foo']
layout_test_dir = port.layout_tests_dir()
test_file = 'fast/test.html'
@@ -254,7 +268,7 @@
def test_uses_test_expectations_file(self):
filesystem = MockFileSystem()
- port = Port(port_name='foo', filesystem=filesystem)
+ port = self.make_port(port_name='foo', filesystem=filesystem)
port.path_to_test_expectations_file = lambda: '/mock-results/test_expectations.txt'
self.assertFalse(port.uses_test_expectations_file())
port._filesystem = MockFileSystem({'/mock-results/test_expectations.txt': ''})
