From f0702477cd32c971cf4da973ffa19fec81760b31 Mon Sep 17 00:00:00 2001 From: "dpranke@chromium.org" Date: Fri, 3 Dec 2010 01:13:59 +0000 Subject: [PATCH] 2010-12-02 Sam Weinig Fix Qt build. * WebKitTestRunner/qt/PlatformWebViewQt.cpp: (WTR::PlatformWebView::PlatformWebView): git-svn-id: http://svn.webkit.org/repository/webkit/trunk@73222 268f45cc-cd09-0410-ab3c-d52691b4dbfc --- WebKitTools/ChangeLog | 32 +++++++++ .../layout_package/dump_render_tree_thread.py | 10 +-- .../layout_package/message_broker.py | 72 +++++++------------ .../layout_package/message_broker_unittest.py | 18 ++--- .../webkitpy/layout_tests/port/base.py | 4 ++ .../webkitpy/layout_tests/run_webkit_tests.py | 10 +-- .../layout_tests/run_webkit_tests_unittest.py | 15 ++++ 7 files changed, 98 insertions(+), 63 deletions(-) diff --git a/WebKitTools/ChangeLog b/WebKitTools/ChangeLog index e714ed6b8ff9..6b50cae046c4 100644 --- a/WebKitTools/ChangeLog +++ b/WebKitTools/ChangeLog @@ -25,6 +25,38 @@ * WebKitTestRunner/win/PlatformWebViewWin.cpp: (WTR::PlatformWebView::PlatformWebView): +2010-12-02 Dirk Pranke + + Reviewed by Tony Chang. + + new-run-webkit-tests: minor cleanup for multiprocessing work + + This change: + * moves worker naming into TestShellThread, eliminating a + parameter to the constructor and putting the responsibility in + the correct place. + * eliminates the _WorkerState() class in message_broker, because + it turns out that state really needs to be in run_webkit_tests + * renames the Broker classes to be module-private. + * fixes a bunch of minor commenting and whitespace issues to + make subsequent patches a bit clearer. + * Adds a Port hook for default_worker_model() so that we can + accomodate the potential for different worker models on + different ports. + * merge in the fix from 50420 for the brokenness introduced in + bug 50367. + + This patch depends on bug 50367. + + https://bugs.webkit.org/show_bug.cgi?id=50372 + + * Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py: + * Scripts/webkitpy/layout_tests/layout_package/message_broker.py: + * Scripts/webkitpy/layout_tests/layout_package/message_broker_unittest.py: + * Scripts/webkitpy/layout_tests/port/base.py: + * Scripts/webkitpy/layout_tests/run_webkit_tests.py: + * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py: + 2010-12-02 Dirk Pranke Reviewed by Tony Chang. diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py index 65d0b0912e95..880cc60a882c 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py @@ -98,7 +98,7 @@ class WatchableThread(threading.Thread): class TestShellThread(WatchableThread): - def __init__(self, port, options, worker_number, worker_name, + def __init__(self, port, options, worker_number, filename_list_queue, result_queue): """Initialize all the local state for this DumpRenderTree thread. @@ -106,7 +106,6 @@ class TestShellThread(WatchableThread): port: interface to port-specific hooks options: command line options argument from optparse worker_number: identifier for a particular worker thread. - worker_name: for logging. filename_list_queue: A thread safe Queue class that contains lists of tuples of (filename, uri) pairs. result_queue: A thread safe Queue class that will contain @@ -116,7 +115,7 @@ class TestShellThread(WatchableThread): self._port = port self._options = options self._worker_number = worker_number - self._name = worker_name + self._name = 'worker/%d' % worker_number self._filename_list_queue = filename_list_queue self._result_queue = result_queue @@ -184,6 +183,9 @@ class TestShellThread(WatchableThread): def get_num_tests(self): return self._num_tests + def name(self): + return self._name + def next_timeout(self): """Return the time the test is supposed to finish by.""" if self._next_timeout: @@ -333,7 +335,7 @@ class TestShellThread(WatchableThread): class SingleTestThread(threading.Thread): def run(self): - result = worker._run_single_test(driver, test_input) + result = worker._run_single_test(test_input, driver) thread = SingleTestThread() thread.start() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker.py index e520a9caa6b7..285b2e7b1fd4 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker.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. -"""Module for handling messages, threads, processes, and concurrency for run-webkit-tests. +"""Module for handling messages and concurrency for run-webkit-tests. Testing is accomplished by having a manager (TestRunner) gather all of the tests to be run, and sending messages to a pool of workers (TestShellThreads) @@ -54,29 +54,20 @@ def get(port, options): """Return an instance of a WorkerMessageBroker.""" worker_model = options.worker_model if worker_model == 'inline': - return InlineBroker(port, options) + return _InlineBroker(port, options) if worker_model == 'threads': - return MultiThreadedBroker(port, options) + return _MultiThreadedBroker(port, options) raise ValueError('unsupported value for --worker-model: %s' % worker_model) -class _WorkerState(object): - def __init__(self, name): - self.name = name - self.thread = None - - -class WorkerMessageBroker(object): +class _WorkerMessageBroker(object): def __init__(self, port, options): self._port = port self._options = options self._num_workers = int(self._options.child_processes) - # This maps worker names to their _WorkerState values. - self._workers = {} - - def _threads(self): - return tuple([w.thread for w in self._workers.values()]) + # This maps worker names to their TestShellThread objects. + self._threads = {} def start_workers(self, test_runner): """Starts up the pool of workers for running the tests. @@ -86,13 +77,19 @@ class WorkerMessageBroker(object): """ self._test_runner = test_runner for worker_number in xrange(self._num_workers): - worker = _WorkerState('worker-%d' % worker_number) - worker.thread = self._start_worker(worker_number, worker.name) - self._workers[worker.name] = worker - return self._threads() + thread = self.start_worker(worker_number) + self._threads[thread.name()] = thread + return self._threads.values() - def _start_worker(self, worker_number, worker_name): - raise NotImplementedError + def start_worker(self, worker_number): + # FIXME: Replace with something that isn't a thread. + # Note: Don't start() the thread! If we did, it would actually + # create another thread and start executing it, and we'd no longer + # be single-threaded. + return dump_render_tree_thread.TestShellThread(self._port, + self._options, worker_number, + self._test_runner._current_filename_queue, + self._test_runner._result_queue) def run_message_loop(self): """Loop processing messages until done.""" @@ -107,43 +104,27 @@ class WorkerMessageBroker(object): pass -class InlineBroker(WorkerMessageBroker): - def _start_worker(self, worker_number, worker_name): - # FIXME: Replace with something that isn't a thread. - thread = dump_render_tree_thread.TestShellThread(self._port, - self._options, worker_number, worker_name, - self._test_runner._current_filename_queue, - self._test_runner._result_queue) - # Note: Don't start() the thread! If we did, it would actually - # create another thread and start executing it, and we'd no longer - # be single-threaded. - return thread - +class _InlineBroker(_WorkerMessageBroker): def run_message_loop(self): - thread = self._threads()[0] + thread = self._threads.values()[0] thread.run_in_main_thread(self._test_runner, self._test_runner._current_result_summary) self._test_runner.update() -class MultiThreadedBroker(WorkerMessageBroker): - def _start_worker(self, worker_number, worker_name): - thread = dump_render_tree_thread.TestShellThread(self._port, - self._options, worker_number, worker_name, - self._test_runner._current_filename_queue, - self._test_runner._result_queue) +class _MultiThreadedBroker(_WorkerMessageBroker): + def start_worker(self, worker_number): + thread = _WorkerMessageBroker.start_worker(self, worker_number) thread.start() return thread def run_message_loop(self): - threads = self._threads() - # Loop through all the threads waiting for them to finish. some_thread_is_alive = True while some_thread_is_alive: some_thread_is_alive = False t = time.time() - for thread in threads: + for thread in self._threads.values(): exception_info = thread.exception_info() if exception_info is not None: # Re-raise the thread's exception here to make it @@ -156,7 +137,7 @@ class MultiThreadedBroker(WorkerMessageBroker): some_thread_is_alive = True next_timeout = thread.next_timeout() if next_timeout and t > next_timeout: - log_wedged_worker(thread.getName(), thread.id()) + log_wedged_worker(thread.name(), thread.id()) thread.clear_next_timeout() self._test_runner.update() @@ -165,8 +146,7 @@ class MultiThreadedBroker(WorkerMessageBroker): time.sleep(0.01) def cancel_workers(self): - threads = self._threads() - for thread in threads: + for thread in self._threads.values(): thread.cancel() diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker_unittest.py index 6f04fd35518a..c006471e4a9f 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker_unittest.py @@ -46,7 +46,7 @@ import message_broker class TestThread(threading.Thread): def __init__(self, started_queue, stopping_queue): threading.Thread.__init__(self) - self._thread_id = None + self._id = None self._started_queue = started_queue self._stopping_queue = stopping_queue self._timeout = False @@ -54,10 +54,10 @@ class TestThread(threading.Thread): self._exception_info = None def id(self): - return self._thread_id + return self._id - def getName(self): - return "worker-0" + def name(self): + return 'worker/0' def run(self): self._covered_run() @@ -65,7 +65,7 @@ class TestThread(threading.Thread): def _covered_run(self): # FIXME: this is a separate routine to work around a bug # in coverage: see http://bitbucket.org/ned/coveragepy/issue/85. - self._thread_id = thread.get_ident() + self._id = thread.get_ident() try: self._started_queue.put('') msg = self._stopping_queue.get() @@ -117,11 +117,11 @@ class MultiThreadedBrokerTest(unittest.TestCase): options = mocktool.MockOptions(child_processes='1') starting_queue = Queue.Queue() stopping_queue = Queue.Queue() - broker = message_broker.MultiThreadedBroker(port, options) + broker = message_broker._MultiThreadedBroker(port, options) broker._test_runner = runner child_thread = TestThread(starting_queue, stopping_queue) - broker._workers['worker-0'] = message_broker._WorkerState('worker-0') - broker._workers['worker-0'].thread = child_thread + name = child_thread.name() + broker._threads[name] = child_thread child_thread.start() started_msg = starting_queue.get() stopping_queue.put(msg) @@ -169,7 +169,7 @@ class Test(unittest.TestCase): child_thread.start() msg = starting_queue.get() - message_broker.log_wedged_worker(child_thread.getName(), + message_broker.log_wedged_worker(child_thread.name(), child_thread.id()) stopping_queue.put('') child_thread.join(timeout=1.0) diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py index bc5a9aa3e4d2..94524fc83e19 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/port/base.py @@ -127,6 +127,10 @@ class Port(object): port.""" return self._executive.cpu_count() + def default_worker_model(self): + """Return the concurrency model the port should use.""" + return 'threads' + def baseline_path(self): """Return the absolute path to the directory to store new baselines in for this port.""" diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py index f4e92a68698d..0b11d9b88b23 100755 --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py @@ -603,7 +603,7 @@ class TestRunner: if not self._options.dry_run: threads = message_broker.start_workers(self) else: - threads = {} + threads = [] self._printer.print_update("Starting testing ...") keyboard_interrupted = False @@ -1338,7 +1338,8 @@ def run(port, options, args, regular_output=sys.stderr, def _set_up_derived_options(port_obj, options): """Sets the options values that depend on other options values.""" - + if options.worker_model is None: + options.worker_model = port_obj.default_worker_model() if options.worker_model == 'inline': if options.child_processes and int(options.child_processes) > 1: _log.warning("--worker-model=inline overrides --child-processes") @@ -1569,9 +1570,10 @@ def parse_args(args=None): optparse.make_option("--child-processes", help="Number of DumpRenderTrees to run in parallel."), # FIXME: Display default number of child processes that will run. + # FIXME: Display default threading model (will be port-specific). optparse.make_option("--worker-model", action="store", - default="threads", help=("controls worker model. Valid values are " - "'inline' and 'threads' (default).")), + help=("controls worker model. Valid values are " + "'inline' and 'threads'.")), optparse.make_option("--experimental-fully-parallel", action="store_true", default=False, help="run all tests in parallel"), diff --git a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py index 6bb741a57c91..0ae9a09c7880 100644 --- a/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py +++ b/WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py @@ -47,6 +47,7 @@ from webkitpy.common import array_stream from webkitpy.common.system import outputcapture from webkitpy.common.system import user from webkitpy.layout_tests import port +from webkitpy.layout_tests.port import test from webkitpy.layout_tests import run_webkit_tests from webkitpy.layout_tests.layout_package import dump_render_tree_thread from webkitpy.layout_tests.port.test import TestPort, TestDriver @@ -186,6 +187,7 @@ class MainTest(unittest.TestCase): def test_batch_size(self): batch_tests_run = get_tests_run(['--batch-size', '2']) + self.assertEquals(len(batch_tests_run), 9) for batch in batch_tests_run: self.assertTrue(len(batch) <= 2, '%s had too many tests' % ', '.join(batch)) @@ -297,6 +299,7 @@ class MainTest(unittest.TestCase): def test_run_singly(self): batch_tests_run = get_tests_run(['--run-singly']) + self.assertEqual(len(batch_tests_run), 14) for batch in batch_tests_run: self.assertEquals(len(batch), 1, '%s had too many tests' % ', '.join(batch)) @@ -392,6 +395,18 @@ class MainTest(unittest.TestCase): self.assertRaises(ValueError, logging_run, ['--worker-model', 'unknown']) + def test_worker_model__port_override(self): + class OverridePort(test.TestPort): + def default_worker_model(self): + return 'INVALID' + + options, parsed_args = run_webkit_tests.parse_args( + ['--print', 'nothing', '--platform', 'test', '--noshow-results']) + port_obj = OverridePort(options=options) + self.assertRaises(ValueError, run_webkit_tests.run, port_obj, + options, parsed_args) + + MainTest = skip_if(MainTest, sys.platform == 'cygwin' and compare_version(sys, '2.6')[0] < 0, 'new-run-webkit-tests tests hang on Cygwin Python 2.5.2') -- 2.36.0