Diff
Modified: trunk/Tools/ChangeLog (107866 => 107867)
--- trunk/Tools/ChangeLog 2012-02-16 02:25:31 UTC (rev 107866)
+++ trunk/Tools/ChangeLog 2012-02-16 02:41:08 UTC (rev 107867)
@@ -1,3 +1,62 @@
+2012-02-15 Dirk Pranke <[email protected]>
+
+ webkitpy: add a worker_args concept to start_worker()
+ https://bugs.webkit.org/show_bug.cgi?id=78572
+
+ Reviewed by Tony Chang.
+
+ This change replaces the three NRWT-specific arguments passed
+ through the broker to the worker with a generic WorkerArguments
+ wrapper class and a separate set_inline_arguments() call that can
+ be used to pass additional data to the worker when it is running
+ in the same process as the manager (this is needed for testing).
+ With the addition of set_inline_arguments() we also no longer
+ need to pass an optional argument to the worker.run() call.
+
+ Note that this method is *only* implemented on inline workers,
+ so calling this on a regular (child process) worker will result
+ in a runtime error.
+
+ * Scripts/webkitpy/layout_tests/controllers/manager.py:
+ (Manager._run_tests):
+ * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
+ (AbstractWorker.__init__):
+ (AbstractWorker.run):
+ (_ManagerConnection.start_worker):
+ (_InlineManager.start_worker):
+ (_InlineManager.set_inline_arguments):
+ (_InlineManager.run_message_loop):
+ (_MultiProcessManager.start_worker): Reworked signature.
+ (_WorkerConnection.__init__):
+ (_InlineWorkerConnection.__init__):
+ (_InlineWorkerConnection.set_inline_arguments): New method.
+ (_InlineWorkerConnection):
+ (_InlineWorkerConnection.run):
+ (_Process.run):
+ (_MultiProcessWorkerConnection.__init__):
+ * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py:
+ (_TestWorker.__init__):
+ (_TestWorker.set_inline_arguments):
+ (_TestWorker.handle_test):
+ (_TestWorker.run):
+ (_TestsMixin.test_cancel):
+ (_TestsMixin.test_done):
+ (_TestsMixin.test_unknown_message):
+ (InlineBrokerTests): New class for more testing.
+ (InlineBrokerTests.setUp):
+ (InlineBrokerTests.test_inline_arguments): New test.
+ (InterfaceTest.test_managerconnection_is_abstract):
+ (InterfaceTest.test_workerconnection_is_abstract):
+ * Scripts/webkitpy/layout_tests/controllers/worker.py:
+ (WorkerArguments):
+ (WorkerArguments.__init__):
+ (Worker.__init__):
+ (Worker.set_inline_arguments):
+ (Worker):
+ (Worker.run):
+ * Scripts/webkitpy/layout_tests/controllers/worker_unittest.py:
+ (WorkerTest.test_default_platform_in_worker):
+
2012-02-15 Adam Klein <[email protected]>
Unreviewed, rolling out r107704.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (107866 => 107867)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-02-16 02:25:31 UTC (rev 107866)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py 2012-02-16 02:41:08 UTC (rev 107867)
@@ -759,7 +759,18 @@
self._printer.print_update('Starting %s ...' % grammar.pluralize('worker', num_workers))
for worker_number in xrange(num_workers):
- worker_connection = manager_connection.start_worker(worker_number, self._port, self.results_directory(), self._options)
+ worker_arguments = worker.WorkerArguments(worker_number, self.results_directory(), self._options)
+ worker_connection = manager_connection.start_worker(worker_arguments)
+ if self._options.worker_model == 'inline':
+ # FIXME: We need to be able to share a port with the work so
+ # that some of the tests can query state on the port; ideally
+ # we'd rewrite the tests so that this wasn't necessary.
+ #
+ # Note that this only works because in the inline case
+ # the worker hasn't really started yet and won't start
+ # running until we call run_message_loop(), below.
+ worker_connection.set_inline_arguments(self._port)
+
worker_state = _WorkerState(worker_number, worker_connection)
self._worker_states[worker_connection.name] = worker_state
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py (107866 => 107867)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py 2012-02-16 02:25:31 UTC (rev 107866)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py 2012-02-16 02:41:08 UTC (rev 107867)
@@ -86,7 +86,7 @@
class AbstractWorker(message_broker.BrokerClient):
- def __init__(self, worker_connection, worker_number, results_directory, options):
+ def __init__(self, worker_connection, worker_arguments=None):
"""The constructor should be used to do any simple initialization
necessary, but should not do anything that creates data structures
that cannot be Pickled or sent across processes (like opening
@@ -96,16 +96,11 @@
Args:
worker_connection - handle to the BrokerConnection object creating
the worker and that can be used for messaging.
- worker_number - identifier for this particular worker
- options - command-line argument object from optparse"""
+ worker_arguments - (optional, Picklable) object passed to the worker from the manager"""
message_broker.BrokerClient.__init__(self)
self._worker_connection = worker_connection
- self._options = options
- self._worker_number = worker_number
- self._name = 'worker/%d' % worker_number
- self._results_directory = results_directory
- def run(self, port):
+ def run(self):
"""Callback for the worker to start executing. Typically does any
remaining initialization and then calls broker_connection.run_message_loop()."""
raise NotImplementedError
@@ -130,7 +125,12 @@
MANAGER_TOPIC, ANY_WORKER_TOPIC)
self._worker_class = worker_class
- def start_worker(self, worker_number, port, results_directory, options):
+ def start_worker(self, worker_arguments=None):
+ """Starts a new worker.
+
+ Args:
+ worker_arguments - an optional Picklable object that is passed to the worker constructor
+ """
raise NotImplementedError
@@ -139,31 +139,35 @@
_ManagerConnection.__init__(self, broker, client, worker_class)
self._inline_worker = None
- def start_worker(self, worker_number, port, results_directory, options):
+ def start_worker(self, worker_arguments=None):
self._inline_worker = _InlineWorkerConnection(self._broker,
- self._client, self._worker_class, worker_number, results_directory, options)
- self._port = port
+ self._client, self._worker_class, worker_arguments)
return self._inline_worker
+ def set_inline_arguments(self, arguments=None):
+ # Note that this method only exists here, and not on all
+ # ManagerConnections; calling this method on a MultiProcessManager
+ # will deliberately result in a runtime error.
+ self._inline_worker.set_inline_arguments(arguments)
+
def run_message_loop(self, delay_secs=None):
# Note that delay_secs is ignored in this case since we can't easily
# implement it.
- self._inline_worker.run(self._port)
+ self._inline_worker.run()
self._broker.run_all_pending(MANAGER_TOPIC, self._client)
class _MultiProcessManager(_ManagerConnection):
- def start_worker(self, worker_number, port, results_directory, options):
- # Note that we ignore the port since we can't pass it to the child (it isn't Picklable).
+ def start_worker(self, worker_arguments=None):
worker_connection = _MultiProcessWorkerConnection(self._broker,
- self._worker_class, worker_number, results_directory, options)
+ self._worker_class, worker_arguments)
worker_connection.start()
return worker_connection
class _WorkerConnection(message_broker.BrokerConnection):
- def __init__(self, broker, worker_class, worker_number, results_directory, options):
- self._client = worker_class(self, worker_number, results_directory, options)
+ def __init__(self, broker, worker_class, worker_arguments=None):
+ self._client = worker_class(self, worker_arguments)
self.name = self._client.name()
message_broker.BrokerConnection.__init__(self, broker, self._client,
ANY_WORKER_TOPIC, MANAGER_TOPIC)
@@ -182,8 +186,8 @@
class _InlineWorkerConnection(_WorkerConnection):
- def __init__(self, broker, manager_client, worker_class, worker_number, results_directory, options):
- _WorkerConnection.__init__(self, broker, worker_class, worker_number, results_directory, options)
+ def __init__(self, broker, manager_client, worker_class, worker_arguments):
+ _WorkerConnection.__init__(self, broker, worker_class, worker_arguments)
self._alive = False
self._manager_client = manager_client
@@ -196,9 +200,12 @@
def join(self, timeout):
assert not self._alive
- def run(self, port):
+ def set_inline_arguments(self, arguments):
+ self._client.set_inline_arguments(arguments)
+
+ def run(self):
self._alive = True
- self._client.run(port)
+ self._client.run()
self._alive = False
def yield_to_broker(self):
@@ -218,11 +225,12 @@
self._client = client
def run(self):
- self._client.run(port=None)
+ self._client.run()
+
class _MultiProcessWorkerConnection(_WorkerConnection):
- def __init__(self, broker, worker_class, worker_number, results_directory, options):
- _WorkerConnection.__init__(self, broker, worker_class, worker_number, results_directory, options)
+ def __init__(self, broker, worker_class, worker_arguments):
+ _WorkerConnection.__init__(self, broker, worker_class, worker_arguments)
self._proc = _Process(self, self._client)
def cancel(self):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py (107866 => 107867)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py 2012-02-16 02:25:31 UTC (rev 107866)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py 2012-02-16 02:41:08 UTC (rev 107867)
@@ -54,23 +54,25 @@
class _TestWorker(manager_worker_broker.AbstractWorker):
- def __init__(self, broker_connection, worker_number, results_directory, options):
+ def __init__(self, broker_connection, worker_obj=None):
self._broker_connection = broker_connection
- self._options = options
- self._worker_number = worker_number
- self._name = 'TestWorker/%d' % worker_number
+ self._name = 'TestWorker'
self._stopped = False
self._canceled = False
+ self._thing_to_greet = 'everybody'
self._starting_queue = starting_queue
self._stopping_queue = stopping_queue
+ def set_inline_arguments(self, thing_to_greet):
+ self._thing_to_greet = thing_to_greet
+
def handle_stop(self, src):
self._stopped = True
def handle_test(self, src, an_int, a_str):
assert an_int == 1
assert a_str == "hello, world"
- self._broker_connection.post_message('test', 2, 'hi, everybody')
+ self._broker_connection.post_message('test', 2, 'hi, ' + self._thing_to_greet)
def is_done(self):
return self._stopped or self._canceled
@@ -81,7 +83,7 @@
def cancel(self):
self._canceled = True
- def run(self, port):
+ def run(self):
if self._starting_queue:
self._starting_queue.put('')
@@ -144,7 +146,7 @@
def test_cancel(self):
self.make_broker()
- worker = self._broker.start_worker(0, None, None, None)
+ worker = self._broker.start_worker()
worker.cancel()
self._broker.post_message('test', 1, 'hello, world')
worker.join(0.5)
@@ -152,7 +154,7 @@
def test_done(self):
self.make_broker()
- worker = self._broker.start_worker(0, None, None, None)
+ worker = self._broker.start_worker()
self._broker.post_message('test', 1, 'hello, world')
self._broker.post_message('stop')
self._broker.run_message_loop()
@@ -164,7 +166,7 @@
def test_unknown_message(self):
self.make_broker()
- worker = self._broker.start_worker(0, None, None, None)
+ worker = self._broker.start_worker()
self._broker.post_message('unknown')
self._broker.run_message_loop()
worker.join(0.5)
@@ -173,9 +175,24 @@
self.assertFalse(worker.is_alive())
self.assertEquals(self._exception[0], ValueError)
self.assertEquals(self._exception[1],
- "TestWorker/0: received message 'unknown' it couldn't handle")
+ "TestWorker: received message 'unknown' it couldn't handle")
+class InlineBrokerTests(_TestsMixin, unittest.TestCase):
+ def setUp(self):
+ _TestsMixin.setUp(self)
+ self._worker_model = 'inline'
+
+ def test_inline_arguments(self):
+ self.make_broker()
+ worker = self._broker.start_worker()
+ worker.set_inline_arguments('me')
+ self._broker.post_message('test', 1, 'hello, world')
+ self._broker.post_message('stop')
+ self._broker.run_message_loop()
+ self.assertEquals(self._a_str, 'hi, me')
+
+
# FIXME: https://bugs.webkit.org/show_bug.cgi?id=54520.
if sys.platform not in ('cygwin', 'win32'):
@@ -195,13 +212,13 @@
# signature we expect.
broker = make_broker(self, 'inline')
obj = manager_worker_broker._ManagerConnection(broker._broker, self, None)
- self.assertRaises(NotImplementedError, obj.start_worker, 0, None, None, None)
+ self.assertRaises(NotImplementedError, obj.start_worker)
def test_workerconnection_is_abstract(self):
# Test that all the base class methods are abstract and have the
# signature we expect.
broker = make_broker(self, 'inline')
- obj = manager_worker_broker._WorkerConnection(broker._broker, _TestWorker, 0, None, None)
+ obj = manager_worker_broker._WorkerConnection(broker._broker, _TestWorker, None)
self.assertRaises(NotImplementedError, obj.cancel)
self.assertRaises(NotImplementedError, obj.is_alive)
self.assertRaises(NotImplementedError, obj.join, None)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py (107866 => 107867)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-02-16 02:25:31 UTC (rev 107866)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-02-16 02:41:08 UTC (rev 107867)
@@ -44,9 +44,20 @@
_log = logging.getLogger(__name__)
+class WorkerArguments(object):
+ def __init__(self, worker_number, results_directory, options):
+ self.worker_number = worker_number
+ self.results_directory = results_directory
+ self.options = options
+
+
class Worker(manager_worker_broker.AbstractWorker):
- def __init__(self, worker_connection, worker_number, results_directory, options):
- manager_worker_broker.AbstractWorker.__init__(self, worker_connection, worker_number, results_directory, options)
+ def __init__(self, worker_connection, worker_arguments):
+ super(Worker, self).__init__(worker_connection, worker_arguments)
+ self._worker_number = worker_arguments.worker_number
+ self._name = 'worker/%d' % self._worker_number
+ self._results_directory = worker_arguments.results_directory
+ self._options = worker_arguments.options
self._done = False
self._canceled = False
self._port = None
@@ -83,10 +94,11 @@
def name(self):
return self._name
- def run(self, port):
- if port:
- self._port = port
- else:
+ def set_inline_arguments(self, port):
+ self._port = port
+
+ def run(self):
+ if not self._port:
# We are running in a child process and need to create a new Host.
if self._options.platform and 'test' in self._options.platform:
# It is lame to import mocks into real code, but this allows us to use the test port in multi-process tests as well.
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker_unittest.py (107866 => 107867)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker_unittest.py 2012-02-16 02:25:31 UTC (rev 107866)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker_unittest.py 2012-02-16 02:41:08 UTC (rev 107867)
@@ -28,7 +28,7 @@
import unittest
-from webkitpy.layout_tests.controllers.worker import Worker
+from webkitpy.layout_tests.controllers.worker import Worker, WorkerArguments
from webkitpy.tool.mocktool import MockOptions
@@ -45,9 +45,9 @@
# This checks that we got a port and didn't raise an exception
# if we didn't specify a port with the --platform flag.
worker_connection = FakeConnection()
- worker = Worker(worker_connection, 1, "/tmp", MockOptions(platform=None, print_options=None, verbose=False, batch_size=0))
+ worker = Worker(worker_connection, WorkerArguments(1, '/tmp', MockOptions(platform=None, print_options=None, verbose=False, batch_size=0)))
worker._done = True
- worker.run(None)
+ worker.run()
self.assertNotEquals(worker._port, None)
