2010-12-02 Sam Weinig <sam@webkit.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Dec 2010 01:13:59 +0000 (01:13 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 3 Dec 2010 01:13:59 +0000 (01:13 +0000)
        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
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/dump_render_tree_thread.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker.py
WebKitTools/Scripts/webkitpy/layout_tests/layout_package/message_broker_unittest.py
WebKitTools/Scripts/webkitpy/layout_tests/port/base.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
WebKitTools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py

index e714ed6b8ff9abb886cfff0b6c6be8f4f0cbbfdb..6b50cae046c4cd52cea57e45a1c473b5645ca49f 100644 (file)
         * WebKitTestRunner/win/PlatformWebViewWin.cpp:
         (WTR::PlatformWebView::PlatformWebView):
 
+2010-12-02  Dirk Pranke  <dpranke@chromium.org>
+
+        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  <dpranke@chromium.org>
 
         Reviewed by Tony Chang.
index 65d0b0912e95d88ca627eb2709d9a57560ddbfbf..880cc60a882ccbf6325c212ef4900c5d9ec14fd3 100644 (file)
@@ -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()
index e520a9caa6b7682ebfdd2a98bb217eb85d7251bc..285b2e7b1fd49029f238ae85d54a250875dedd1f 100644 (file)
@@ -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()
 
 
index 6f04fd35518a04ebd6919ff8a97613cf4c5be1f4..c006471e4a9fb25fe1c68094576f32ce5181f5d9 100644 (file)
@@ -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)
index bc5a9aa3e4d255c1f7492129d04cd2664c876208..94524fc83e19fa0bb91dcf01f21dcc96c03f57d4 100644 (file)
@@ -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."""
index f4e92a68698d4b069031d6bb1b37882dbaf1faf5..0b11d9b88b23ce19fe5af01a66984ac942436049 100755 (executable)
@@ -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"),
index 6bb741a57c91c0b0e264cfde7d6410cbf583966a..0ae9a09c7880e29620d02763438fc13a28413cd9 100644 (file)
@@ -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')