Title: [120619] trunk/Tools
Revision
120619
Author
[email protected]
Date
2012-06-18 12:59:53 -0700 (Mon, 18 Jun 2012)

Log Message

new-run-webkit-tests output gets confused when logging
https://bugs.webkit.org/show_bug.cgi?id=63793

Reviewed by Adam Barth.

Ship all of the log messages generated in the worker back
to the manager; this allows the messages to be properly
serialized to stderr and for the meter to flush its output correctly.

Note however that this will likely result in messages to be
logged out of order between workers; I'm not sure that there's
an easy way to fix this short of buffering a potentially
unbounded amount of data. It might be better to just log through
stderr unless we're in 'metering' mode, but it's also worth
noting that we already get messages out of order through stderr
on Windows.

* Scripts/webkitpy/layout_tests/controllers/manager.py:
(Manager.handle_done):
(Manager.handle_finished_test):
(Manager._log_messages):
* Scripts/webkitpy/layout_tests/controllers/worker.py:
(Worker.__init__):
(Worker._set_up_logging):
(Worker.run):
(Worker._run_test):
(Worker.cleanup):
(Worker.run_single_test):
(_WorkerLogHandler):
(_WorkerLogHandler.__init__):
(_WorkerLogHandler.emit):
* Scripts/webkitpy/layout_tests/views/metered_stream.py:
(MeteredStream.write):
(MeteredStream.writeln):
* Scripts/webkitpy/layout_tests/views/printing.py:
(Printer.writeln):

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (120618 => 120619)


--- trunk/Tools/ChangeLog	2012-06-18 19:53:38 UTC (rev 120618)
+++ trunk/Tools/ChangeLog	2012-06-18 19:59:53 UTC (rev 120619)
@@ -1,3 +1,42 @@
+2012-06-18  Dirk Pranke  <[email protected]>
+
+        new-run-webkit-tests output gets confused when logging
+        https://bugs.webkit.org/show_bug.cgi?id=63793
+
+        Reviewed by Adam Barth.
+
+        Ship all of the log messages generated in the worker back
+        to the manager; this allows the messages to be properly
+        serialized to stderr and for the meter to flush its output correctly.
+
+        Note however that this will likely result in messages to be
+        logged out of order between workers; I'm not sure that there's
+        an easy way to fix this short of buffering a potentially
+        unbounded amount of data. It might be better to just log through
+        stderr unless we're in 'metering' mode, but it's also worth
+        noting that we already get messages out of order through stderr
+        on Windows.
+
+        * Scripts/webkitpy/layout_tests/controllers/manager.py:
+        (Manager.handle_done):
+        (Manager.handle_finished_test):
+        (Manager._log_messages):
+        * Scripts/webkitpy/layout_tests/controllers/worker.py:
+        (Worker.__init__):
+        (Worker._set_up_logging):
+        (Worker.run):
+        (Worker._run_test):
+        (Worker.cleanup):
+        (Worker.run_single_test):
+        (_WorkerLogHandler):
+        (_WorkerLogHandler.__init__):
+        (_WorkerLogHandler.emit):
+        * Scripts/webkitpy/layout_tests/views/metered_stream.py:
+        (MeteredStream.write):
+        (MeteredStream.writeln):
+        * Scripts/webkitpy/layout_tests/views/printing.py:
+        (Printer.writeln):
+
 2012-06-18  Zan Dobersek  <[email protected]>
 
         [garden-o-matic] Builder names without underscores cause incorrect BuildSelector behavior

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py (120618 => 120619)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py	2012-06-18 19:53:38 UTC (rev 120618)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/manager.py	2012-06-18 19:59:53 UTC (rev 120619)
@@ -1463,9 +1463,10 @@
         worker_state.current_test_name = test_info.test_name
         worker_state.next_timeout = time.time() + hang_timeout
 
-    def handle_done(self, source):
+    def handle_done(self, source, log_messages=None):
         worker_state = self._worker_states[source]
         worker_state.done = True
+        self._log_messages(log_messages)
 
     def handle_exception(self, source, exception_type, exception_value, stack):
         if exception_type in (KeyboardInterrupt, TestRunInterruptedException):
@@ -1492,16 +1493,21 @@
             if not self._remaining_locked_shards:
                 self.stop_servers_with_lock()
 
-    def handle_finished_test(self, source, result, elapsed_time):
+    def handle_finished_test(self, source, result, elapsed_time, log_messages=None):
         worker_state = self._worker_states[source]
         worker_state.next_timeout = None
         worker_state.current_test_name = None
         worker_state.stats['total_time'] += elapsed_time
         worker_state.stats['num_tests'] += 1
 
+        self._log_messages(log_messages)
         self._all_results.append(result)
         self._update_summary_with_result(self._current_result_summary, result)
 
+    def _log_messages(self, messages):
+        for message in messages:
+            self._printer.writeln(*message)
+
     def _log_worker_stack(self, stack):
         webkitpydir = self._port.path_from_webkit_base('Tools', 'Scripts', 'webkitpy') + self._filesystem.sep
         for filename, line_number, function_name, text in stack:

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py (120618 => 120619)


--- trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py	2012-06-18 19:53:38 UTC (rev 120618)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/controllers/worker.py	2012-06-18 19:59:53 UTC (rev 120619)
@@ -29,6 +29,7 @@
 """Handle messages from the Manager and executes actual tests."""
 
 import logging
+import os
 import sys
 import threading
 import time
@@ -65,7 +66,9 @@
         self._driver = None
         self._tests_run_file = None
         self._tests_run_filename = None
-        self._meter = None
+        self._log_messages = []
+        self._logger = None
+        self._log_handler = None
 
     def __del__(self):
         self.cleanup()
@@ -83,29 +86,19 @@
         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 before we can
-        # add a new one to get the correct pid logged.
-        root_logger = logging.getLogger()
-        handler_to_remove = None
-        for h in root_logger.handlers:
+        # 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:
-                handler_to_remove = h
+                self._logger.removeHandler(h)
                 break
-        if handler_to_remove:
-            root_logger.removeHandler(handler_to_remove)
 
-        # FIXME: This won't work if the calling process is logging
-        # somewhere other than sys.stderr, but I'm not sure
-        # if this will be an issue in practice. Also, it would be
-        # nice if we trapped all of the messages for a given test
-        # and sent them back in finished_test() rather than logging
-        # them to stderr.
-        if not root_logger.handlers:
-            options = self._options
-            root_logger.setLevel(logging.DEBUG if options.verbose else logging.INFO)
-            self._meter = metered_stream.MeteredStream(sys.stderr, options.verbose, logger=root_logger)
+        self._logger.setLevel(logging.DEBUG if self._options.verbose else logging.INFO)
+        self._log_handler = _WorkerLogHandler(self)
+        self._logger.addHandler(self._log_handler)
 
     def _set_up_host_and_port(self):
         options = self._options
@@ -132,9 +125,9 @@
             super(Worker, self).run()
         finally:
             self.kill_driver()
-            self._worker_connection.post_message('done')
             _log.debug("%s exiting" % self._name)
             self.cleanup()
+            self._worker_connection.post_message('done', self._log_messages)
 
     def handle_test_list(self, src, list_name, test_list):
         start_time = time.time()
@@ -171,7 +164,9 @@
         result = self.run_test_with_timeout(test_input, test_timeout_sec)
 
         elapsed_time = time.time() - start
-        self._worker_connection.post_message('finished_test', result, elapsed_time)
+        log_messages = self._log_messages
+        self._log_messages = []
+        self._worker_connection.post_message('finished_test', result, elapsed_time, log_messages)
 
         self.clean_up_after_test(test_input, result)
 
@@ -181,9 +176,10 @@
         if self._tests_run_file:
             self._tests_run_file.close()
             self._tests_run_file = None
-        if self._meter:
-            self._meter.cleanup()
-            self._meter = 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."""
@@ -300,3 +296,13 @@
     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(tuple([record.getMessage(), record.created, self._pid]))

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py (120618 => 120619)


--- trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py	2012-06-18 19:53:38 UTC (rev 120618)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py	2012-06-18 19:59:53 UTC (rev 120619)
@@ -91,14 +91,15 @@
         if self._erasing:
             self._last_partial_line = txt[txt.rfind('\n') + 1:]
 
-    def write(self, txt, now=None):
+    def write(self, txt, now=None, pid=None):
         now = now or self._time_fn()
+        pid = pid or self._pid
         self._last_write_time = now
         if self._last_partial_line:
             self._erase_last_partial_line()
         if self._verbose:
             now_tuple = time.localtime(now)
-            msg = '%02d:%02d:%02d.%03d %d %s' % (now_tuple.tm_hour, now_tuple.tm_min, now_tuple.tm_sec, int((now * 1000) % 1000), self._pid, self._ensure_newline(txt))
+            msg = '%02d:%02d:%02d.%03d %d %s' % (now_tuple.tm_hour, now_tuple.tm_min, now_tuple.tm_sec, int((now * 1000) % 1000), pid, self._ensure_newline(txt))
         elif self._isatty:
             msg = txt
         else:
@@ -106,8 +107,8 @@
 
         self._stream.write(msg)
 
-    def writeln(self, txt, now=None):
-        self.write(self._ensure_newline(txt), now)
+    def writeln(self, txt, now=None, pid=None):
+        self.write(self._ensure_newline(txt), now, pid)
 
     def _erase_last_partial_line(self):
         num_chars = len(self._last_partial_line)

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py (120618 => 120619)


--- trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py	2012-06-18 19:53:38 UTC (rev 120618)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/views/printing.py	2012-06-18 19:59:53 UTC (rev 120619)
@@ -407,6 +407,9 @@
             return
         self._write(msg)
 
+    def writeln(self, *args, **kwargs):
+        self._meter.writeln(*args, **kwargs)
+
     def _write(self, msg):
         self._meter.writeln(msg)
 
_______________________________________________
webkit-changes mailing list
[email protected]
http://lists.webkit.org/mailman/listinfo.cgi/webkit-changes

Reply via email to