- Revision
- 121807
- Author
- dpra...@chromium.org
- Date
- 2012-07-03 16:11:18 -0700 (Tue, 03 Jul 2012)
Log Message
nrwt: moving child process logging code into manager_worker_broker
https://bugs.webkit.org/show_bug.cgi?id=90408
Reviewed by Ojan Vafai.
Users of manager_worker_broker should not have to be aware of
whether they're in the same process or different processes and
configure logging themselves; mwb should hide this complexity.
We can't quite do this completely/correctly yet, since the
manager expects to get a list of messages to log, but this
change fixes the worker side of it, at least.
This is just moving code around, there is no new functionality
and this should be covered by existing tests.
* Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
(AbstractWorker.__init__):
(_WorkerConnection.__init__):
(_WorkerConnection.post_message):
(_WorkerConnection):
(_WorkerConnection.set_up_logging):
(_WorkerConnection.clean_up_logging):
(_InlineWorkerConnection.run):
(_MultiProcessWorkerConnection.run):
(_WorkerLogHandler):
(_WorkerLogHandler.__init__):
(_WorkerLogHandler.emit):
* Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py:
(_TestWorker.run):
(_TestsMixin.handle_done):
* Scripts/webkitpy/layout_tests/controllers/worker.py:
(Worker.__init__):
(Worker.run):
(Worker._run_test):
(Worker.cleanup):
(Worker.run_single_test):
Modified Paths
Diff
Modified: trunk/Tools/ChangeLog (121806 => 121807)
--- trunk/Tools/ChangeLog 2012-07-03 22:57:00 UTC (rev 121806)
+++ trunk/Tools/ChangeLog 2012-07-03 23:11:18 UTC (rev 121807)
@@ -1,3 +1,42 @@
+2012-07-03 Dirk Pranke <dpra...@chromium.org>
+
+ nrwt: moving child process logging code into manager_worker_broker
+ https://bugs.webkit.org/show_bug.cgi?id=90408
+
+ Reviewed by Ojan Vafai.
+
+ Users of manager_worker_broker should not have to be aware of
+ whether they're in the same process or different processes and
+ configure logging themselves; mwb should hide this complexity.
+ We can't quite do this completely/correctly yet, since the
+ manager expects to get a list of messages to log, but this
+ change fixes the worker side of it, at least.
+
+ This is just moving code around, there is no new functionality
+ and this should be covered by existing tests.
+
+ * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py:
+ (AbstractWorker.__init__):
+ (_WorkerConnection.__init__):
+ (_WorkerConnection.post_message):
+ (_WorkerConnection):
+ (_WorkerConnection.set_up_logging):
+ (_WorkerConnection.clean_up_logging):
+ (_InlineWorkerConnection.run):
+ (_MultiProcessWorkerConnection.run):
+ (_WorkerLogHandler):
+ (_WorkerLogHandler.__init__):
+ (_WorkerLogHandler.emit):
+ * Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py:
+ (_TestWorker.run):
+ (_TestsMixin.handle_done):
+ * Scripts/webkitpy/layout_tests/controllers/worker.py:
+ (Worker.__init__):
+ (Worker.run):
+ (Worker._run_test):
+ (Worker.cleanup):
+ (Worker.run_single_test):
+
2012-07-03 Tony Chang <t...@chromium.org>
[chromium] Don't archive build files generated by VS2010
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py (121806 => 121807)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py 2012-07-03 22:57:00 UTC (rev 121806)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker.py 2012-07-03 23:11:18 UTC (rev 121807)
@@ -69,12 +69,14 @@
import logging
import multiprocessing
import optparse
+import os
import Queue
import sys
import traceback
from webkitpy.common.system import stack_utils
+from webkitpy.layout_tests.views import metered_stream
_log = logging.getLogger(__name__)
@@ -277,6 +279,7 @@
self._name = 'worker/%d' % worker_number
self._done = False
self._canceled = False
+ self._options = optparse.Values({'verbose': False})
def name(self):
return self._name
@@ -361,6 +364,9 @@
def __init__(self, host, broker, worker_factory, worker_number):
self._client = worker_factory(self, worker_number)
self._host = host
+ self._log_messages = []
+ self._logger = None
+ self._log_handler = None
_BrokerConnection.__init__(self, broker, self._client, ANY_WORKER_TOPIC, MANAGER_TOPIC)
def name(self):
@@ -378,7 +384,34 @@
def yield_to_broker(self):
pass
+ def post_message(self, *args):
+ # FIXME: This is a hack until we can remove the log_messages arg from the manager.
+ if args[0] in ('finished_test', 'done'):
+ log_messages = self._log_messages
+ self._log_messages = []
+ args = args + tuple([log_messages])
+ super(_WorkerConnection, self).post_message(*args)
+ def set_up_logging(self):
+ self._logger = logging.root
+ # The unix multiprocessing implementation clones the MeteredStream log handler
+ # into the child process, so we need to remove it to avoid duplicate logging.
+ for h in self._logger.handlers:
+ # log handlers don't have names until python 2.7.
+ if getattr(h, 'name', '') == metered_stream.LOG_HANDLER_NAME:
+ self._logger.removeHandler(h)
+ break
+ self._logger.setLevel(logging.DEBUG if self._client._options.verbose else logging.INFO)
+ self._log_handler = _WorkerLogHandler(self)
+ self._logger.addHandler(self._log_handler)
+
+ def clean_up_logging(self):
+ if self._log_handler and self._logger:
+ self._logger.removeHandler(self._log_handler)
+ self._log_handler = None
+ self._logger = None
+
+
class _InlineWorkerConnection(_WorkerConnection):
def __init__(self, host, broker, manager_client, worker_factory, worker_number):
_WorkerConnection.__init__(self, host, broker, worker_factory, worker_number)
@@ -397,7 +430,7 @@
def run(self):
self._alive = True
try:
- self._client.run(self._host, set_up_logging=False)
+ self._client.run(self._host)
finally:
self._alive = False
@@ -439,5 +472,18 @@
self._proc.start()
def run(self):
- # FIXME: set_up_logging shouldn't be needed.
- self._client.run(self._host, set_up_logging=True)
+ self.set_up_logging()
+ try:
+ self._client.run(self._host)
+ finally:
+ self.clean_up_logging()
+
+
+class _WorkerLogHandler(logging.Handler):
+ def __init__(self, worker):
+ logging.Handler.__init__(self)
+ self._worker = worker
+ self._pid = os.getpid()
+
+ def emit(self, record):
+ self._worker._log_messages.append(record)
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py (121806 => 121807)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py 2012-07-03 22:57:00 UTC (rev 121806)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager_worker_broker_unittest.py 2012-07-03 23:11:18 UTC (rev 121807)
@@ -69,7 +69,7 @@
assert a_str == "hello, world"
self._worker_connection.post_message('test', 2, 'hi, ' + self._thing_to_greet)
- def run(self, host, set_up_logging):
+ def run(self, host):
if self._starting_queue:
self._starting_queue.put('')
@@ -102,7 +102,7 @@
def is_done(self):
return self._done
- def handle_done(self, src):
+ def handle_done(self, src, log_messages):
self._done = True
def handle_test(self, src, an_int, a_str):
Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py (121806 => 121807)
--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-07-03 22:57:00 UTC (rev 121806)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py 2012-07-03 23:11:18 UTC (rev 121807)
@@ -29,8 +29,6 @@
"""Handle messages from the Manager and executes actual tests."""
import logging
-import os
-import sys
import threading
import time
@@ -39,7 +37,6 @@
from webkitpy.layout_tests.controllers import single_test_runner
from webkitpy.layout_tests.models import test_expectations
from webkitpy.layout_tests.models import test_results
-from webkitpy.layout_tests.views import metered_stream
_log = logging.getLogger(__name__)
@@ -57,9 +54,6 @@
self._driver = None
self._tests_run_file = None
self._tests_run_filename = None
- self._log_messages = []
- self._logger = None
- self._log_handler = None
def __del__(self):
self.cleanup()
@@ -76,29 +70,10 @@
tests_run_filename = self._filesystem.join(self._results_directory, "tests_run%d.txt" % self._worker_number)
self._tests_run_file = self._filesystem.open_text_file_for_writing(tests_run_filename)
- def _set_up_logging(self):
- self._logger = logging.getLogger()
-
- # The unix multiprocessing implementation clones the MeteredStream log handler
- # into the child process, so we need to remove it to avoid duplicate logging.
- for h in self._logger.handlers:
- # log handlers don't have names until python 2.7.
- if getattr(h, 'name', '') == metered_stream.LOG_HANDLER_NAME:
- self._logger.removeHandler(h)
- break
-
- self._logger.setLevel(logging.DEBUG if self._options.verbose else logging.INFO)
- self._log_handler = _WorkerLogHandler(self)
- self._logger.addHandler(self._log_handler)
-
- def run(self, host, set_up_logging):
+ def run(self, host):
if not host:
host = Host()
- # FIXME: this should move into manager_worker_broker.py.
- if set_up_logging:
- self._set_up_logging()
-
options = self._options
self._port = host.port_factory.get(options.platform, options)
@@ -110,7 +85,7 @@
self.kill_driver()
_log.debug("%s exiting" % self._name)
self.cleanup()
- self._worker_connection.post_message('done', self._log_messages)
+ self._worker_connection.post_message('done')
def handle_test_list(self, src, list_name, test_list):
start_time = time.time()
@@ -147,9 +122,7 @@
result = self.run_test_with_timeout(test_input, test_timeout_sec)
elapsed_time = time.time() - start
- log_messages = self._log_messages
- self._log_messages = []
- self._worker_connection.post_message('finished_test', result, elapsed_time, log_messages)
+ self._worker_connection.post_message('finished_test', result, elapsed_time)
self.clean_up_after_test(test_input, result)
@@ -159,10 +132,6 @@
if self._tests_run_file:
self._tests_run_file.close()
self._tests_run_file = None
- if self._log_handler and self._logger:
- self._logger.removeHandler(self._log_handler)
- self._log_handler = None
- self._logger = None
def timeout(self, test_input):
"""Compute the appropriate timeout value for a test."""
@@ -279,13 +248,3 @@
def run_single_test(self, driver, test_input):
return single_test_runner.run_single_test(self._port, self._options,
test_input, driver, self._name)
-
-
-class _WorkerLogHandler(logging.Handler):
- def __init__(self, worker):
- logging.Handler.__init__(self)
- self._worker = worker
- self._pid = os.getpid()
-
- def emit(self, record):
- self._worker._log_messages.append(record)
