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):
git-svn-id: http://svn.webkit.org/repository/webkit/trunk@107867
268f45cc-cd09-0410-ab3c-
d52691b4dbfc
+2012-02-15 Dirk Pranke <dpranke@chromium.org>
+
+ 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 <adamk@chromium.org>
Unreviewed, rolling out r107704.
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
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
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
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
_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)
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
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):
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):
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
def cancel(self):
self._canceled = True
- def run(self, port):
+ def run(self):
if self._starting_queue:
self._starting_queue.put('')
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)
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()
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)
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.
# 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)
_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
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.
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
# 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)
projects / profile / ivi / webkit-efl.git / commitdiff