Title: [231679] trunk/Tools
Revision
231679
Author
[email protected]
Date
2018-05-10 17:42:03 -0700 (Thu, 10 May 2018)

Log Message

REGRESSION (r230998): Cannot stream API test output
https://bugs.webkit.org/show_bug.cgi?id=185090

Reviewed by Ryosuke Niwa.

Change run-api-tests so that the stdout and stderr of a test will be printed
before it's result. If -v is passed, the output of single test runs will be
streamed.

* Scripts/webkitpy/api_tests/run_api_tests.py:
(run): Use print_timestamps flag.
(parse_args): Add timestamps option to print timestamps in the log.
* Scripts/webkitpy/api_tests/runner.py:
(Runner): Add running status.
(Runner.__init__): Add number of workers and a flag to detect if a newline
is needed at the beginning of a log line.
(Runner.run): Turn off most logging coming from server process since it is
not constructive for API tests.
(Runner.handle): Add new log message to stream stdout and stderr instead of
caching values until the test run finished.
(_Worker._run_single_test): Log stdout and stderr in real time.
(_Worker._run_shard_with_binary): Log stderr and stdout buffer before ending
the test.
(_Worker.post): Provide a post with explicitly enumerated argument names.
* Scripts/webkitpy/layout_tests/views/metered_stream.py:
(MeteredStream.__init__): Differentiate verbose logging and logging with timestamps.
(MeteredStream.write): Allow for verbose logging without timestamps.
* Scripts/webkitpy/layout_tests/views/metered_stream_unittest.py:
(RegularTest): Add print_timestamps flag.
(RegularTest.setUp):
(RegularTest.test_logging_not_included):
(VerboseTest.test_log_args):
(VerboseWithOutTimestamp):
(VerboseWithOutTimestamp.test_basic): Add test for verbose logging without timestamps.

Modified Paths

Diff

Modified: trunk/Tools/ChangeLog (231678 => 231679)


--- trunk/Tools/ChangeLog	2018-05-11 00:23:19 UTC (rev 231678)
+++ trunk/Tools/ChangeLog	2018-05-11 00:42:03 UTC (rev 231679)
@@ -1,3 +1,40 @@
+2018-05-10  Jonathan Bedard  <[email protected]>
+
+        REGRESSION (r230998): Cannot stream API test output
+        https://bugs.webkit.org/show_bug.cgi?id=185090
+
+        Reviewed by Ryosuke Niwa.
+
+        Change run-api-tests so that the stdout and stderr of a test will be printed
+        before it's result. If -v is passed, the output of single test runs will be
+        streamed. 
+
+        * Scripts/webkitpy/api_tests/run_api_tests.py:
+        (run): Use print_timestamps flag.
+        (parse_args): Add timestamps option to print timestamps in the log.
+        * Scripts/webkitpy/api_tests/runner.py:
+        (Runner): Add running status.
+        (Runner.__init__): Add number of workers and a flag to detect if a newline
+        is needed at the beginning of a log line.
+        (Runner.run): Turn off most logging coming from server process since it is
+        not constructive for API tests.
+        (Runner.handle): Add new log message to stream stdout and stderr instead of
+        caching values until the test run finished.
+        (_Worker._run_single_test): Log stdout and stderr in real time.
+        (_Worker._run_shard_with_binary): Log stderr and stdout buffer before ending
+        the test.
+        (_Worker.post): Provide a post with explicitly enumerated argument names.
+        * Scripts/webkitpy/layout_tests/views/metered_stream.py:
+        (MeteredStream.__init__): Differentiate verbose logging and logging with timestamps.
+        (MeteredStream.write): Allow for verbose logging without timestamps.
+        * Scripts/webkitpy/layout_tests/views/metered_stream_unittest.py:
+        (RegularTest): Add print_timestamps flag.
+        (RegularTest.setUp):
+        (RegularTest.test_logging_not_included):
+        (VerboseTest.test_log_args):
+        (VerboseWithOutTimestamp):
+        (VerboseWithOutTimestamp.test_basic): Add test for verbose logging without timestamps.
+
 2018-05-10  Ryosuke Niwa  <[email protected]>
 
         build-webkit can erroneously warn about Xcode being too old

Modified: trunk/Tools/Scripts/webkitpy/api_tests/run_api_tests.py (231678 => 231679)


--- trunk/Tools/Scripts/webkitpy/api_tests/run_api_tests.py	2018-05-11 00:23:19 UTC (rev 231678)
+++ trunk/Tools/Scripts/webkitpy/api_tests/run_api_tests.py	2018-05-11 00:42:03 UTC (rev 231679)
@@ -70,7 +70,7 @@
     logger.setLevel(logging.DEBUG if options.verbose else logging.ERROR if options.quiet else logging.INFO)
 
     try:
-        stream = MeteredStream(logging_stream, options.verbose, logger=logger, number_of_columns=port.host.platform.terminal_width())
+        stream = MeteredStream(logging_stream, options.verbose, logger=logger, number_of_columns=port.host.platform.terminal_width(), print_timestamps=options.timestamps)
         manager = Manager(port, options, stream)
 
         result = manager.run(args)
@@ -90,6 +90,8 @@
                              help='Run quietly (errors, warnings, and progress only)'),
         optparse.make_option('-v', '--verbose', action='', default=False,
                              help='Enable verbose printing'),
+        optparse.make_option('--timestamps', action='', default=False,
+                             help='Print timestamps for each logged line'),
     ]))
 
     option_group_definitions.append(('WebKit Options', [

Modified: trunk/Tools/Scripts/webkitpy/api_tests/runner.py (231678 => 231679)


--- trunk/Tools/Scripts/webkitpy/api_tests/runner.py	2018-05-11 00:23:19 UTC (rev 231678)
+++ trunk/Tools/Scripts/webkitpy/api_tests/runner.py	2018-05-11 00:42:03 UTC (rev 231679)
@@ -21,10 +21,11 @@
 # SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
 
 import os
+import logging
 import time
 
 from webkitpy.common import message_pool
-from webkitpy.port.server_process import ServerProcess
+from webkitpy.port.server_process import ServerProcess, _log as server_process_logger
 from webkitpy.xcode.simulated_device import SimulatedDeviceManager
 
 
@@ -34,6 +35,7 @@
     STATUS_CRASHED = 2
     STATUS_TIMEOUT = 3
     STATUS_DISABLED = 4
+    STATUS_RUNNING = 5
 
     NAME_FOR_STATUS = [
         'Passed',
@@ -47,6 +49,8 @@
         self.port = port
         self.printer = printer
         self.tests_run = 0
+        self._num_workers = 1
+        self._has_logged_for_test = True  # Suppress an empty line between "Running tests" and the first test's output.
         self.results = {}
 
     @staticmethod
@@ -78,25 +82,49 @@
         self.printer.write_update('Sharding tests ...')
         shards = Runner._shard_tests(tests)
 
-        with message_pool.get(self, lambda caller: _Worker(caller, self.port, shards), min(num_workers, len(shards))) as pool:
-            pool.run(('test', shard) for shard, _ in shards.iteritems())
+        original_level = server_process_logger.level
+        server_process_logger.setLevel(logging.CRITICAL)
 
+        try:
+            self._num_workers = min(num_workers, len(shards))
+            with message_pool.get(self, lambda caller: _Worker(caller, self.port, shards), self._num_workers) as pool:
+                pool.run(('test', shard) for shard, _ in shards.iteritems())
+        finally:
+            server_process_logger.setLevel(original_level)
+
+
     def handle(self, message_name, source, test_name=None, status=0, output=''):
         if message_name == 'did_spawn_worker':
             return
+
+        source = '' if self._num_workers == 1 else source + ' '
+        will_stream_logs = self._num_workers == 1 and self.port.get_option('verbose')
         if message_name == 'ended_test':
-            update = '{} {} {}'.format(source, test_name, Runner.NAME_FOR_STATUS[status])
+            update = '{}{} {}'.format(source, test_name, Runner.NAME_FOR_STATUS[status])
 
             # Don't print test output if --quiet.
             if status != Runner.STATUS_PASSED or (output and not self.port.get_option('quiet')):
+                if not will_stream_logs:
+                    for line in output.splitlines():
+                        if not self._has_logged_for_test:
+                            self._has_logged_for_test = True
+                            self.printer.writeln(source)
+                        self.printer.writeln('{}    {}'.format(source, line))
                 self.printer.writeln(update)
-                for line in output.splitlines():
-                    self.printer.writeln('    {}'.format(line))
             else:
                 self.printer.write_update(update)
             self.tests_run += 1
             self.results[test_name] = (status, output)
+            self._has_logged_for_test = False
 
+        if message_name == 'log' and will_stream_logs:
+            for line in output.splitlines():
+                if not self._has_logged_for_test:
+                    self._has_logged_for_test = True
+                    self.printer.writeln(source)
+                self.printer.writeln('{}    {}'.format(source, line))
+
+
     def result_map_by_status(self, status=None):
         map = {}
         for test_name, result in self.results.iteritems():
@@ -130,31 +158,51 @@
             Runner.command_for_port(self._port, [self._port._build_path(binary_name), '--gtest_filter={}'.format(test)]),
             env=self._port.environment_for_api_tests())
 
+        status = Runner.STATUS_RUNNING
+        if test.split('.')[1].startswith('DISABLED_'):
+            status = Runner.STATUS_DISABLED
+
         try:
             deadline = time.time() + self._timeout
-            server_process.start()
+            if status != Runner.STATUS_DISABLED:
+                server_process.start()
 
-            if not test.split('.')[1].startswith('DISABLED_'):
-                stdout_line = server_process.read_stdout_line(deadline)
-            else:
-                stdout_line = None
+            stdout_buffer = ''
+            stderr_buffer = ''
+            while status == Runner.STATUS_RUNNING:
+                stdout_line, stderr_line = server_process.read_either_stdout_or_stderr_line(deadline)
+                if not stderr_line and not stdout_line:
+                    break
 
-            if not stdout_line and server_process.timed_out:
+                if stderr_line:
+                    stderr_buffer += stderr_line
+                    self.post('log', output=stderr_line[:-1])
+                if stdout_line:
+                    if '**PASS**' in stdout_line:
+                        status = Runner.STATUS_PASSED
+                    elif '**FAIL**' in stdout_line:
+                        status = Runner.STATUS_FAILED
+                    else:
+                        stdout_buffer += stdout_line
+                        self.post('log', output=stdout_line[:-1])
+
+            if status == Runner.STATUS_DISABLED:
+                pass
+            elif server_process.timed_out:
                 status = Runner.STATUS_TIMEOUT
-            elif not stdout_line and server_process.has_crashed():
+            elif server_process.has_crashed():
                 status = Runner.STATUS_CRASHED
-            elif not stdout_line:
-                status = Runner.STATUS_DISABLED
-            elif '**PASS**' in stdout_line:
-                status = Runner.STATUS_PASSED
-            else:
+            elif status == Runner.STATUS_RUNNING:
                 status = Runner.STATUS_FAILED
 
         finally:
-            output_buffer = server_process.pop_all_buffered_stdout() + server_process.pop_all_buffered_stderr()
+            remaining_stderr = server_process.pop_all_buffered_stderr()
+            remaining_stdout = server_process.pop_all_buffered_stdout()
+            self.post('log', output=remaining_stderr + remaining_stdout)
+            output_buffer = stderr_buffer + stdout_buffer + remaining_stderr + remaining_stdout
             server_process.stop()
 
-        self._caller.post('ended_test', '{}.{}'.format(binary_name, test), status, self._filter_noisy_output(output_buffer))
+        self.post('ended_test', '{}.{}'.format(binary_name, test), status, self._filter_noisy_output(output_buffer))
 
     def _run_shard_with_binary(self, binary_name, tests):
         remaining_tests = list(tests)
@@ -196,7 +244,7 @@
                         continue
                     if last_test is not None:
                         remaining_tests.remove(last_test)
-                        self._caller.post('ended_test', '{}.{}'.format(binary_name, last_test), last_status, stdout_buffer)
+                        self.post('ended_test', '{}.{}'.format(binary_name, last_test), last_status, stdout_buffer)
                         deadline = time.time() + self._timeout
                         stdout_buffer = ''
 
@@ -211,7 +259,8 @@
                     remaining_tests.remove(last_test)
                     stdout_buffer += server_process.pop_all_buffered_stdout()
                     stderr_buffer = server_process.pop_all_buffered_stderr() if last_status == Runner.STATUS_CRASHED else ''
-                    self._caller.post('ended_test', '{}.{}'.format(binary_name, last_test), last_status, self._filter_noisy_output(stdout_buffer + stderr_buffer))
+                    self.post('log', output=stdout_buffer + stderr_buffer)
+                    self.post('ended_test', '{}.{}'.format(binary_name, last_test), last_status, self._filter_noisy_output(stdout_buffer + stderr_buffer))
 
                 if server_process.timed_out:
                     break
@@ -226,9 +275,12 @@
         for test in remaining_tests:
             self._run_single_test(binary_name, test)
 
+    def post(self, message_name, test_name=None, status=0, output=''):
+        self._caller.post(message_name, test_name, status, output)
+
     def handle(self, message_name, source, shard_name):
         assert message_name == 'test'
-        self._caller.post('started_shard', shard_name)
+        self.post('started_shard', shard_name)
 
         binary_map = {}
         for test in self._shard_map[shard_name]:

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


--- trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py	2018-05-11 00:23:19 UTC (rev 231678)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream.py	2018-05-11 00:42:03 UTC (rev 231679)
@@ -51,9 +51,10 @@
     def _ensure_newline(txt):
         return txt if txt.endswith('\n') else txt + '\n'
 
-    def __init__(self, stream=None, verbose=False, logger=None, time_fn=None, pid=None, number_of_columns=None):
+    def __init__(self, stream=None, verbose=False, logger=None, time_fn=None, pid=None, number_of_columns=None, print_timestamps=None):
         self._stream = stream or sys.stderr
         self._verbose = verbose
+        self._print_timestamps = verbose if print_timestamps is None else print_timestamps
         self._time_fn = time_fn or time.time
         self._pid = pid or os.getpid()
         self._isatty = self._stream.isatty()
@@ -97,13 +98,16 @@
         self._last_write_time = now
         if self._last_partial_line:
             self._erase_last_partial_line()
-        if self._verbose:
+
+        timestamp_string = ''
+        if self._print_timestamps:
             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), pid, self._ensure_newline(txt))
-        elif self._isatty:
-            msg = txt
+            timestamp_string = '%02d:%02d:%02d.%03d %d ' % (now_tuple.tm_hour, now_tuple.tm_min, now_tuple.tm_sec, int((now * 1000) % 1000), pid)
+
+        if self._isatty and not self._verbose:
+            msg = '{}{}'.format(timestamp_string, txt)
         else:
-            msg = self._ensure_newline(txt)
+            msg = '{}{}'.format(timestamp_string, self._ensure_newline(txt))
 
         self._stream.write(msg)
 

Modified: trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream_unittest.py (231678 => 231679)


--- trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream_unittest.py	2018-05-11 00:23:19 UTC (rev 231678)
+++ trunk/Tools/Scripts/webkitpy/layout_tests/views/metered_stream_unittest.py	2018-05-11 00:42:03 UTC (rev 231679)
@@ -37,6 +37,7 @@
 class RegularTest(unittest.TestCase):
     verbose = False
     isatty = False
+    print_timestamps = None
 
     def setUp(self):
         self.stream = StringIO.StringIO()
@@ -51,7 +52,7 @@
         # add a dummy time counter for a default behavior.
         self.times = range(10)
 
-        self.meter = MeteredStream(self.stream, self.verbose, self.logger, self.time_fn, 8675)
+        self.meter = MeteredStream(self.stream, self.verbose, self.logger, self.time_fn, 8675, print_timestamps=self.print_timestamps)
 
     def tearDown(self):
         if self.meter:
@@ -71,7 +72,7 @@
         root_logger.addHandler(handler)
         root_logger.setLevel(logging.DEBUG)
         try:
-            self.meter = MeteredStream(self.stream, self.verbose, None, self.time_fn, 8675)
+            self.meter = MeteredStream(self.stream, self.verbose, None, self.time_fn, 8675, print_timestamps=self.print_timestamps)
             self.meter.write_throttled_update('foo')
             self.meter.write_update('bar')
             self.meter.write('baz')
@@ -152,3 +153,17 @@
         self.logger.info('foo %s %d', 'bar', 2)
         self.assertEqual(len(self.buflist), 1)
         self.assertTrue(self.buflist[0].endswith('foo bar 2\n'))
+
+
+class VerboseWithOutTimestamp(RegularTest):
+    isatty = True
+    verbose = True
+    print_timestamps = False
+
+    def test_basic(self):
+        buflist = self._basic([0, 1, 2.1, 13, 14.1234])
+        self.assertTrue(re.match('foo\n', buflist[0]))
+        self.assertTrue(re.match('bar\n', buflist[1]))
+        self.assertTrue(re.match('baz 2\n', buflist[2]))
+        self.assertTrue(re.match('done\n', buflist[3]))
+        self.assertEqual(len(buflist), 4)
_______________________________________________
webkit-changes mailing list
[email protected]
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to