Diff
Modified: trunk/Tools/ChangeLog (89867 => 89868)
--- trunk/Tools/ChangeLog 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/ChangeLog 2011-06-27 22:17:45 UTC (rev 89868)
@@ -1,3 +1,44 @@
+2011-06-27 Eric Seidel <e...@webkit.org>
+
+ Reviewed by Adam Barth.
+
+ new-run-webkit-tests needs a --webkit-test-runner option
+ https://bugs.webkit.org/show_bug.cgi?id=63439
+
+ NRWT doesn't actually know how to run with the WebKitTestRunner yet
+ but it does have a --webkit-test-runner option and will build WebKitTestRunner correctly.
+
+ There is a bunch of other little cleanup in this patch which I added as I
+ took a tour through all of our hard-coded DumpRenderTree strings.
+
+ * Scripts/webkitpy/common/net/layouttestresults.py: Added FIXME.
+ * Scripts/webkitpy/common/system/outputcapture.py: Fixed spacing to pass PEP8.
+ * Scripts/webkitpy/layout_tests/layout_package/manager.py: Made the FIXME slightly stronger (bad bug).
+ * Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py: test_shell is dead.
+ * Scripts/webkitpy/layout_tests/layout_package/test_failures.py: Unwrapped silly wrapping.
+ * Scripts/webkitpy/layout_tests/port/base.py: Removed two dead methods.
+ * Scripts/webkitpy/layout_tests/port/config.py:
+ - build_dumprendertree had no business in this class, removed it.
+ - Exposed _FLAGS_FROM_CONFIGURATIONS through flag_for_configuration()
+ - Exposed _script_path as script_path() (this probably belongs elsewhere).
+ * Scripts/webkitpy/layout_tests/port/config_unittest.py:
+ - Moved these tests to webkit_unittests.
+ * Scripts/webkitpy/layout_tests/port/webkit.py:
+ - Now respects the --webkit-test-runner option.
+ - setup_test_run is empty in base.py too, no need to override it.
+ * Scripts/webkitpy/layout_tests/port/webkit_unittest.py:
+ - Test the new hotness.
+ - Bad, bad, bad! The old code was using a real Executive during unit-testing!
+ I think this code is still hitting disk during the unit tests. :(
+ * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+ - Add the option.
+ * Scripts/webkitpy/tool/bot/queueengine_unittest.py:
+ - Remove optional args option.
+ * Scripts/webkitpy/tool/commands/commandtest.py:
+ - Remove optional args option.
+ * Scripts/webkitpy/tool/commands/queues_unittest.py:
+ - Remove optional args option.
+
2011-06-27 Adam Barth <aba...@webkit.org>
Reviewed by Dirk Pranke.
Modified: trunk/Tools/Scripts/webkitpy/common/net/layouttestresults.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/common/net/layouttestresults.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/common/net/layouttestresults.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -48,6 +48,7 @@
stderr_key = u'Tests that had stderr output:'
fail_key = u'Tests where results did not match expected results:'
timeout_key = u'Tests that timed out:'
+ # FIXME: This may need to be made aware of WebKitTestRunner results for WebKit2.
crash_key = u'Tests that caused the DumpRenderTree tool to crash:'
missing_key = u'Tests that had no expected results (probably new):'
webprocess_crash_key = u'Tests that caused the Web process to crash:'
Modified: trunk/Tools/Scripts/webkitpy/common/system/executive_mock.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/common/system/executive_mock.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/common/system/executive_mock.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -51,16 +51,22 @@
def kill_process(self, pid):
pass
- def run_command(self, arg_list, error_handler=None, return_exit_code=False,
- decode_output=False, return_stderr=False):
+ def run_command(self,
+ args,
+ cwd=None,
+ input=None,
+ error_handler=None,
+ return_exit_code=False,
+ return_stderr=True,
+ decode_output=False):
if self._exception:
raise self._exception
if return_exit_code:
return self._exit_code
if self._run_command_fn:
- return self._run_command_fn(arg_list)
+ return self._run_command_fn(args)
if self._exit_code and error_handler:
- script_error = executive.ScriptError(script_args=arg_list,
+ script_error = executive.ScriptError(script_args=args,
exit_code=self._exit_code,
output=self._output)
error_handler(script_error)
Modified: trunk/Tools/Scripts/webkitpy/common/system/outputcapture.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/common/system/outputcapture.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/common/system/outputcapture.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -32,6 +32,7 @@
import unittest
from StringIO import StringIO
+
class OutputCapture(object):
def __init__(self):
self.saved_outputs = dict()
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/manager.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -652,7 +652,7 @@
# FIXME: If we start workers up too quickly, DumpRenderTree appears
# to thrash on something and time out its first few tests. Until
# we can figure out what's going on, sleep a bit in between
- # workers.
+ # workers. This needs a bug filed.
time.sleep(0.1)
self._printer.print_update("Starting testing ...")
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/single_test_runner.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -143,7 +143,7 @@
return TestResult(self._filename, failures, driver_output.test_time, driver_output.has_stderr())
def _save_baselines(self, driver_output):
- # Although all test_shell/DumpRenderTree output should be utf-8,
+ # Although all DumpRenderTree output should be utf-8,
# we do not ever decode it inside run-webkit-tests. For some tests
# DumpRenderTree may not output utf-8 text (e.g. webarchives).
self._save_baseline_data(driver_output.text, ".txt",
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/layout_package/test_failures.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -104,14 +104,12 @@
return cPickle.dumps(self)
def should_kill_dump_render_tree(self):
- """Returns True if we should kill DumpRenderTree before the next
- test."""
+ """Returns True if we should kill DumpRenderTree before the next test."""
return False
class FailureTimeout(TestFailure):
- """Test timed out. We also want to restart DumpRenderTree if this
- happens."""
+ """Test timed out. We also want to restart DumpRenderTree if this happens."""
def __init__(self, is_reftest=False):
self.is_reftest = is_reftest
@@ -130,6 +128,7 @@
@staticmethod
def message():
+ # FIXME: This is wrong for WebKit2 (which uses WebKitTestRunner).
return "DumpRenderTree crashed"
def should_kill_dump_render_tree(self):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/base.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -589,12 +589,6 @@
WebKit source tree and the list of path components in |*comps|."""
return self._config.path_from_webkit_base(*comps)
- def script_path(self, script_name):
- return self._config.script_path(script_name)
-
- def script_shell_command(self, script_name):
- return self._config.script_shell_command(script_name)
-
def path_to_test_expectations_file(self):
"""Update the test expectations to the passed-in string.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/config.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/config.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/config.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -71,32 +71,21 @@
def build_directory(self, configuration):
"""Returns the path to the build directory for the configuration."""
if configuration:
- flags = ["--configuration",
- self._FLAGS_FROM_CONFIGURATIONS[configuration]]
+ flags = ["--configuration", self.flag_for_configuration(configuration)]
else:
configuration = ""
flags = ["--top-level"]
if not self._build_directories.get(configuration):
- args = ["perl", self._script_path("webkit-build-directory")] + flags
+ args = ["perl", self.script_path("webkit-build-directory")] + flags
self._build_directories[configuration] = (
- self._executive.run_command(args).rstrip())
+ self._executive.run_command(args, cwd=self.webkit_base_dir()).rstrip())
return self._build_directories[configuration]
- def build_dumprendertree(self, configuration):
- """Builds DRT in the given configuration.
+ def flag_for_configuration(self, configuration):
+ return self._FLAGS_FROM_CONFIGURATIONS[configuration]
- Returns True if the build was successful and up-to-date."""
- flag = self._FLAGS_FROM_CONFIGURATIONS[configuration]
- exit_code = self._executive.run_command([
- self._script_path("build-dumprendertree"), flag],
- return_exit_code=True)
- if exit_code != 0:
- _log.error("Failed to build DumpRenderTree")
- return False
- return True
-
def default_configuration(self):
"""Returns the default configuration for the user.
@@ -107,15 +96,15 @@
if not self._default_configuration:
self._default_configuration = 'Release'
if self._default_configuration not in self._FLAGS_FROM_CONFIGURATIONS:
- _log.warn("Configuration \"%s\" is not a recognized value.\n" %
- self._default_configuration)
- _log.warn("Scripts may fail. "
- "See 'set-webkit-configuration --help'.")
+ _log.warn("Configuration \"%s\" is not a recognized value.\n" % self._default_configuration)
+ _log.warn("Scripts may fail. See 'set-webkit-configuration --help'.")
return self._default_configuration
def path_from_webkit_base(self, *comps):
return self._filesystem.join(self.webkit_base_dir(), *comps)
+ # FIXME: We should only have one implementation of this logic,
+ # if scm.find_checkout_root() is broken for Chromium, we should fix (or at least wrap) it!
def webkit_base_dir(self):
"""Returns the absolute path to the top of the WebKit tree.
@@ -133,9 +122,9 @@
self._webkit_base_dir = abspath[0:abspath.find('Tools') - 1]
return self._webkit_base_dir
- def _script_path(self, script_name):
- return self._filesystem.join(self.webkit_base_dir(), "Tools",
- "Scripts", script_name)
+ def script_path(self, script_name):
+ # This is intentionally relative. Callers should pass the checkout_root/webkit_base_dir to run_command as the cwd.
+ return self._filesystem.join("Tools", "Scripts", script_name)
def _determine_configuration(self):
# This mirrors the logic in webkitdirs.pm:determineConfiguration().
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/config_unittest.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/config_unittest.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/config_unittest.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -91,29 +91,6 @@
self.assertTrue(c.build_directory('Debug').endswith('/Debug'))
self.assertRaises(KeyError, c.build_directory, 'Unknown')
- def test_build_dumprendertree__success(self):
- c = self.make_config(exit_code=0)
- self.assertTrue(c.build_dumprendertree("Debug"))
- self.assertTrue(c.build_dumprendertree("Release"))
- self.assertRaises(KeyError, c.build_dumprendertree, "Unknown")
-
- def test_build_dumprendertree__failure(self):
- c = self.make_config(exit_code=-1)
-
- # FIXME: Build failures should log errors. However, the message we
- # get depends on how we're being called; as a standalone test,
- # we'll get the "no handlers found" message. As part of
- # test-webkitpy, we get the actual message. Really, we need
- # outputcapture to install its own handler.
- oc = outputcapture.OutputCapture()
- oc.capture_output()
- self.assertFalse(c.build_dumprendertree('Debug'))
- oc.restore_output()
-
- oc.capture_output()
- self.assertFalse(c.build_dumprendertree('Release'))
- oc.restore_output()
-
def test_default_configuration__release(self):
self.assert_configuration('Release', 'Release')
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -41,8 +41,10 @@
import time
import webbrowser
+
from webkitpy.common.net.buildbot import BuildBot
from webkitpy.common.system import ospath
+from webkitpy.common.system.executive import ScriptError
from webkitpy.layout_tests.port import base, builders, server_process
_log = logging.getLogger("webkitpy.layout_tests.port.webkit")
@@ -58,6 +60,11 @@
# FIXME: disable pixel tests until they are run by default on the build machines.
self.set_option_default("pixel_tests", False)
+ def driver_name(self):
+ if self.get_option('webkit_test_runner'):
+ return "WebKitTestRunner"
+ return "DumpRenderTree"
+
def baseline_search_path(self):
return [self._webkit_baseline_path(self._name)]
@@ -75,14 +82,29 @@
result_set.set_platform(platform)
return result_set
+ def _driver_build_script_name(self):
+ if self.get_option('webkit_test_runner'):
+ return "build-webkittestrunner"
+ return "build-dumprendertree"
+
def _build_driver(self):
configuration = self.get_option('configuration')
- return self._config.build_dumprendertree(configuration)
+ try:
+ # FIXME: We should probably have a run_script helper which automatically adds the configuration flags.
+ self._executive.run_command([
+ self._config.script_path(self._driver_build_script_name()),
+ self._config.flag_for_configuration(configuration)],
+ # FIXME: It's unclear if this cwd= is necessary. Again a helper should do this for us.
+ cwd=self._config.webkit_base_dir())
+ except ScriptError:
+ _log.error("Failed to build %s" % self.driver_name())
+ return False
+ return True
def _check_driver(self):
driver_path = self._path_to_driver()
if not self._filesystem.exists(driver_path):
- _log.error("DumpRenderTree was not found at %s" % driver_path)
+ _log.error("%s was not found at %s" % (self.driver_name(), driver_path))
return False
return True
@@ -177,10 +199,6 @@
# to have multiple copies of webkit checked out and built.
return self._build_path('layout-test-results')
- def setup_test_run(self):
- # This port doesn't require any specific configuration.
- pass
-
def create_driver(self, worker_number):
return WebKitDriver(self, worker_number)
@@ -427,8 +445,8 @@
if error_lines and error_lines[-1] == "#EOF":
error_lines.pop() # Remove the expected "#EOF"
error = "\n".join(error_lines)
- # FIXME: This seems like the wrong section of code to be doing
- # this reset in.
+
+ # FIXME: This seems like the wrong section of code to be doing this reset in.
self._server_process.error = ""
return base.DriverOutput(text, image, actual_image_hash, audio,
crash=self._server_process.crashed, test_time=time.time() - start_time,
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/port/webkit_unittest.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -28,19 +28,25 @@
import unittest
from webkitpy.common.system import filesystem_mock
+from webkitpy.common.system.outputcapture import OutputCapture
from webkitpy.layout_tests.port.webkit import WebKitPort
from webkitpy.layout_tests.port import port_testcase
+from webkitpy.tool.mocktool import MockExecutive
+from webkitpy.tool.mocktool import MockOptions
+
class TestWebKitPort(WebKitPort):
def __init__(self, symbol_list=None, feature_list=None,
- expectations_file=None, skips_file=None, **kwargs):
+ expectations_file=None, skips_file=None,
+ executive=None, **kwargs):
self.symbol_list = symbol_list
self.feature_list = feature_list
self.expectations_file = expectations_file
self.skips_file = skips_file
- WebKitPort.__init__(self, **kwargs)
+ executive = executive or MockExecutive(should_log=False)
+ WebKitPort.__init__(self, executive=executive, **kwargs)
def _runtime_feature_list(self):
return self.feature_list
@@ -91,8 +97,7 @@
self.assertEqual(result_directories, expected_directories)
def test_skipped_layout_tests(self):
- self.assertEqual(TestWebKitPort(None, None).skipped_layout_tests(),
- set(["media"]))
+ self.assertEqual(TestWebKitPort(None, None).skipped_layout_tests(), set(["media"]))
def test_test_expectations(self):
# Check that we read both the expectations file and anything in a
@@ -109,6 +114,21 @@
BUG_SKIPPED SKIP : fast/html/keygen.html = FAIL
BUG_SKIPPED SKIP : media = FAIL""")
+ def test_build_driver(self):
+ output = OutputCapture()
+ port = TestWebKitPort()
+ # Delay setting _executive to avoid logging during construction
+ port._executive = MockExecutive(should_log=True)
+ port._options = MockOptions(configuration="Release") # This should not be necessary, but I think TestWebKitPort is actually reading from disk (and thus detects the current configuration).
+ expected_stderr = "MOCK run_command: ['Tools/Scripts/build-dumprendertree', '--release']\n"
+ self.assertTrue(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr))
-if __name__ == '__main__':
- unittest.main()
+ # Make sure when passed --webkit-test-runner web build the right tool.
+ port._options = MockOptions(webkit_test_runner=True, configuration="Release")
+ expected_stderr = "MOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release']\n"
+ self.assertTrue(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr))
+
+ # Make sure that failure to build returns False.
+ port._executive = MockExecutive(should_log=True, should_throw=True)
+ expected_stderr = "MOCK run_command: ['Tools/Scripts/build-webkittestrunner', '--release']\n"
+ self.assertFalse(output.assert_outputs(self, port._build_driver, expected_stderr=expected_stderr))
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -268,7 +268,9 @@
optparse.make_option("--complex-text", action="" default=False,
help="Use the complex text code path for all text (Mac OS X and Windows only)"),
optparse.make_option("--threaded", action="" default=False,
- help="Run a concurrent _javascript_ thead with each test")
+ help="Run a concurrent _javascript_ thead with each test"),
+ optparse.make_option("--webkit-test-runner", "-2", action=""
+ help="Use WebKitTestRunner rather than DumpRenderTree."),
]
# Missing Mac-specific old-run-webkit-tests options:
Modified: trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/tool/bot/queueengine_unittest.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -160,7 +160,7 @@
if not termination_message:
termination_message = "Delegate terminated queue."
expected_stderr = "\n%s\n" % termination_message
- OutputCapture().assert_outputs(self, engine.run, [], expected_stderr=expected_stderr)
+ OutputCapture().assert_outputs(self, engine.run, expected_stderr=expected_stderr)
def _test_terminating_queue(self, exception, termination_message):
work_item_index = LoggingDelegate.expected_callbacks.index('process_work_item')
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/commandtest.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/tool/commands/commandtest.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/commandtest.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -32,7 +32,7 @@
from webkitpy.tool.mocktool import MockOptions, MockTool
class CommandsTest(unittest.TestCase):
- def assert_execute_outputs(self, command, args, expected_stdout="", expected_stderr="", expected_exception=None, options=MockOptions(), tool=MockTool()):
+ def assert_execute_outputs(self, command, args=[], expected_stdout="", expected_stderr="", expected_exception=None, options=MockOptions(), tool=MockTool()):
options.blocks = None
options.cc = 'MOCK cc'
options.component = 'MOCK component'
Modified: trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py (89867 => 89868)
--- trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2011-06-27 22:15:25 UTC (rev 89867)
+++ trunk/Tools/Scripts/webkitpy/tool/commands/queues_unittest.py 2011-06-27 22:17:45 UTC (rev 89868)
@@ -166,7 +166,7 @@
tool.status_server = MockStatusServer(work_items=[2, 197])
expected_stdout = "MOCK: fetch_attachment: 2 is not a known attachment id\n" # A mock-only message to prevent us from making mistakes.
expected_stderr = "MOCK: release_work_item: None 2\n"
- patch_id = OutputCapture().assert_outputs(self, queue._next_patch, [], expected_stdout=expected_stdout, expected_stderr=expected_stderr)
+ patch_id = OutputCapture().assert_outputs(self, queue._next_patch, expected_stdout=expected_stdout, expected_stderr=expected_stderr)
self.assertEquals(patch_id, None) # 2 is an invalid patch id
self.assertEquals(queue._next_patch().id(), 197)
