2010-12-22 Dirk Pranke <dpranke@chromium.org>
authordpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Dec 2010 00:55:07 +0000 (00:55 +0000)
committerdpranke@chromium.org <dpranke@chromium.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 23 Dec 2010 00:55:07 +0000 (00:55 +0000)
        Reviewed by Ojan Vafai.

        nrwt multiprocessing - start over, prepare to fork the code

        This code cleans up the signatures and implementation of the
        TestRunner class so we can easily fork it to run either the
        stable implementation or the new, unstable message-passing
        implementation. The two variants will have different
        implementations of the run_tests() method. We will switch
        between the two based on the setting for the '--worker-model'
        switch. We rename the two currently valid values to 'old-inline'
        and 'old-threads'.

        https://bugs.webkit.org/show_bug.cgi?id=51081

        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:

git-svn-id: http://svn.webkit.org/repository/webkit/trunk@74523 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Tools/ChangeLog
Tools/Scripts/webkitpy/layout_tests/layout_package/message_broker.py
Tools/Scripts/webkitpy/layout_tests/run_webkit_tests.py
Tools/Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py

index adc7cd1c96948e4dbf482a0b3769ad2abbafdbe7..898a301d10a51d1229ca855634c70aae7ffd387d 100644 (file)
@@ -1,3 +1,23 @@
+2010-12-22  Dirk Pranke  <dpranke@chromium.org>
+
+        Reviewed by Ojan Vafai.
+
+        nrwt multiprocessing - start over, prepare to fork the code
+        This code cleans up the signatures and implementation of the
+        TestRunner class so we can easily fork it to run either the
+        stable implementation or the new, unstable message-passing
+        implementation. The two variants will have different
+        implementations of the run_tests() method. We will switch
+        between the two based on the setting for the '--worker-model'
+        switch. We rename the two currently valid values to 'old-inline'
+        and 'old-threads'.
+
+        https://bugs.webkit.org/show_bug.cgi?id=51081
+
+        * Scripts/webkitpy/layout_tests/run_webkit_tests.py:
+        * Scripts/webkitpy/layout_tests/run_webkit_tests_unittest.py:
+
 2010-12-22  Victor Wang  <victorw@chromium.org>
 
         Reviewed by Darin Fisher.
index e520a9caa6b7682ebfdd2a98bb217eb85d7251bc..e0ca8db1d8aa5504a06e18465e4572bf62fee1d5 100644 (file)
@@ -53,9 +53,9 @@ _log = logging.getLogger(__name__)
 def get(port, options):
     """Return an instance of a WorkerMessageBroker."""
     worker_model = options.worker_model
-    if worker_model == 'inline':
+    if worker_model == 'old-inline':
         return InlineBroker(port, options)
-    if worker_model == 'threads':
+    if worker_model == 'old-threads':
         return MultiThreadedBroker(port, options)
     raise ValueError('unsupported value for --worker-model: %s' % worker_model)
 
index ebe64954030a41081bace19d6a07426213042c23..426d39d9f3936465bc149f719a16055f181fa424 100755 (executable)
@@ -143,7 +143,7 @@ class ResultSummary(object):
         """Add a TestResult into the appropriate bin.
 
         Args:
-          result: TestResult from dump_render_tree_thread.
+          result: TestResult
           expected: whether the result was what we expected it to be.
         """
 
@@ -252,19 +252,18 @@ class TestRunner:
     # in DumpRenderTree.
     DEFAULT_TEST_TIMEOUT_MS = 6 * 1000
 
-    def __init__(self, port, options, printer, message_broker):
+    def __init__(self, port, options, printer):
         """Initialize test runner data structures.
 
         Args:
           port: an object implementing port-specific
           options: a dictionary of command line options
           printer: a Printer object to record updates to.
-          message_broker: object used to communicate with workers.
         """
         self._port = port
         self._options = options
         self._printer = printer
-        self._message_broker = message_broker
+        self._message_broker = None
 
         # disable wss server. need to install pyOpenSSL on buildbots.
         # self._websocket_secure_server = websocket_server.PyWebSocket(
@@ -525,7 +524,7 @@ class TestRunner:
         """Groups tests into batches.
         This helps ensure that tests that depend on each other (aka bad tests!)
         continue to run together as most cross-tests dependencies tend to
-        occur within the same directory. If use_real_shards is false, we
+        occur within the same directory. If use_real_shards is False, we
         put each (non-HTTP/websocket) test into its own shard for maximum
         concurrency instead of trying to do any sort of real sharding.
 
@@ -617,12 +616,13 @@ class TestRunner:
 
         self._printer.print_update('Starting %s ...' %
                                    grammar.pluralize('worker', num_workers))
-        message_broker = self._message_broker
+        self._message_broker = message_broker.get(self._port, self._options)
+        broker = self._message_broker
         self._current_filename_queue = filename_queue
         self._current_result_summary = result_summary
 
         if not self._options.dry_run:
-            threads = message_broker.start_workers(self)
+            threads = broker.start_workers(self)
         else:
             threads = {}
 
@@ -631,15 +631,15 @@ class TestRunner:
         interrupted = False
         if not self._options.dry_run:
             try:
-                message_broker.run_message_loop()
+                broker.run_message_loop()
             except KeyboardInterrupt:
                 _log.info("Interrupted, exiting")
-                message_broker.cancel_workers()
+                broker.cancel_workers()
                 keyboard_interrupted = True
                 interrupted = True
             except TestRunInterruptedException, e:
                 _log.info(e.reason)
-                message_broker.cancel_workers()
+                broker.cancel_workers()
                 interrupted = True
             except:
                 # Unexpected exception; don't try to clean up workers.
@@ -649,6 +649,8 @@ class TestRunner:
         thread_timings, test_timings, individual_test_timings = \
             self._collect_timing_info(threads)
 
+        broker.cleanup()
+        self._message_broker = None
         return (interrupted, keyboard_interrupted, thread_timings, test_timings,
                 individual_test_timings)
 
@@ -1011,8 +1013,7 @@ class TestRunner:
     def _print_aggregate_test_statistics(self, individual_test_timings):
         """Prints aggregate statistics (e.g. median, mean, etc.) for all tests.
         Args:
-          individual_test_timings: List of dump_render_tree_thread.TestStats
-              for all tests.
+          individual_test_timings: List of TestResults for all tests.
         """
         test_types = []  # Unit tests don't actually produce any timings.
         if individual_test_timings:
@@ -1047,8 +1048,7 @@ class TestRunner:
                                   result_summary):
         """Prints the run times for slow, timeout and crash tests.
         Args:
-          individual_test_timings: List of dump_render_tree_thread.TestStats
-              for all tests.
+          individual_test_timings: List of TestStats for all tests.
           result_summary: summary object for test run
         """
         # Reverse-sort by the time spent in DumpRenderTree.
@@ -1321,10 +1321,13 @@ def run(port, options, args, regular_output=sys.stderr,
           error.
 
     """
-    _set_up_derived_options(port, options)
+    warnings = _set_up_derived_options(port, options)
 
     printer = printing.Printer(port, options, regular_output, buildbot_output,
         int(options.child_processes), options.experimental_fully_parallel)
+    for w in warnings:
+        _log.warning(w)
+
     if options.help_printing:
         printer.help_printing()
         printer.cleanup()
@@ -1336,13 +1339,11 @@ def run(port, options, args, regular_output=sys.stderr,
         printer.cleanup()
         return 0
 
-    broker = message_broker.get(port, options)
-
     # We wrap any parts of the run that are slow or likely to raise exceptions
     # in a try/finally to ensure that we clean up the logging configuration.
     num_unexpected_results = -1
     try:
-        test_runner = TestRunner(port, options, printer, broker)
+        test_runner = TestRunner(port, options, printer)
         test_runner._print_config()
 
         printer.print_update("Collecting tests ...")
@@ -1371,7 +1372,6 @@ def run(port, options, args, regular_output=sys.stderr,
             _log.debug("Testing completed, Exit status: %d" %
                        num_unexpected_results)
     finally:
-        broker.cleanup()
         printer.cleanup()
 
     return num_unexpected_results
@@ -1379,10 +1379,12 @@ 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."""
+    # We return a list of warnings to print after the printer is initialized.
+    warnings = []
 
-    if options.worker_model == 'inline':
+    if options.worker_model == 'old-inline':
         if options.child_processes and int(options.child_processes) > 1:
-            _log.warning("--worker-model=inline overrides --child-processes")
+            warnings.append("--worker-model=old-inline overrides --child-processes")
         options.child_processes = "1"
     if not options.child_processes:
         options.child_processes = os.environ.get("WEBKIT_TEST_CHILD_PROCESSES",
@@ -1410,6 +1412,7 @@ def _set_up_derived_options(port_obj, options):
             options.time_out_ms = str(TestRunner.DEFAULT_TEST_TIMEOUT_MS)
 
     options.slow_time_out_ms = str(5 * int(options.time_out_ms))
+    return warnings
 
 
 def _gather_unexpected_results(options):
@@ -1608,8 +1611,8 @@ def parse_args(args=None):
             help="Number of DumpRenderTrees to run in parallel."),
         # FIXME: Display default number of child processes that will run.
         optparse.make_option("--worker-model", action="store",
-            default="threads", help=("controls worker model. Valid values are "
-            "'inline' and 'threads' (default).")),
+            default="old-threads", help=("controls worker model. Valid values "
+            "are 'old-inline', 'old-threads'.")),
         optparse.make_option("--experimental-fully-parallel",
             action="store_true", default=False,
             help="run all tests in parallel"),
index fe30403402d2190e91f2fd9033dabf7f101cdd80..d07e9287ba3923295b8b82844f25ecb04dfb0c71 100644 (file)
@@ -76,7 +76,7 @@ def parse_args(extra_args=None, record_results=False, tests_included=False,
     if not record_results:
         args.append('--no-record-results')
     if not '--child-processes' in extra_args:
-        args.extend(['--worker-model', 'inline'])
+        args.extend(['--worker-model', 'old-inline'])
     args.extend(extra_args)
     if not tests_included:
         # We use the glob to test that globbing works.
@@ -442,14 +442,10 @@ class MainTest(unittest.TestCase):
         self.assertEqual(None, test_port.tolerance_used_for_diff_image)
 
     def test_worker_model__inline(self):
-        self.assertTrue(passing_run(['--worker-model', 'inline']))
+        self.assertTrue(passing_run(['--worker-model', 'old-inline']))
 
     def test_worker_model__threads(self):
-        self.assertTrue(passing_run(['--worker-model', 'threads']))
-
-    def test_worker_model__processes(self):
-        self.assertRaises(ValueError, logging_run,
-                          ['--worker-model', 'processes'])
+        self.assertTrue(passing_run(['--worker-model', 'old-threads']))
 
     def test_worker_model__unknown(self):
         self.assertRaises(ValueError, logging_run,
@@ -537,7 +533,7 @@ class TestRunnerTest(unittest.TestCase):
         mock_port.filename_to_uri = lambda name: name
 
         runner = run_webkit_tests.TestRunner(port=mock_port, options=Mock(),
-            printer=Mock(), message_broker=Mock())
+            printer=Mock())
         expected_html = u"""<html>
   <head>
     <title>Layout Test Results (time)</title>
@@ -555,7 +551,7 @@ class TestRunnerTest(unittest.TestCase):
         # Test that _shard_tests in run_webkit_tests.TestRunner really
         # put the http tests first in the queue.
         runner = TestRunnerWrapper(port=Mock(), options=Mock(),
-            printer=Mock(), message_broker=Mock())
+            printer=Mock())
 
         test_list = [
           "LayoutTests/websocket/tests/unicode.htm",