[blinkpy] Redirect stderr of wptserve to a file

wptserve (especially its pywebsocket component) produces too many log
messages to stderr which can block the pipe since the parent process
(wptserve.py) does not read from the pipe until the server exits.

This change redirects the stderr of wptserve to a log file (in a way
that's still compatible with the error logging and cleanup logic in
ServerBase, see _log_errors_from_subprocess and _remove_stale_logs
respectively).

Bug: 968904
Change-Id: I08200bf7d895c844652cd50e627e270671f25369
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/1641436
Commit-Queue: Robert Ma <robertma@chromium.org>
Reviewed-by: Yuta Kitamura <yutak@chromium.org>
Cr-Commit-Position: refs/heads/master@{#666321}

third_party/blink/tools/blinkpy/web_tests/merge_results.py

@@ -619,10 +619,13 @@ class WebTestDirMerger(DirMerger):
             FilenameRegexMatch(r'error_log\.txt$'),
             MergeFilesLinesSorted(self.filesystem))
 
-        # pywebsocket files aren't particularly useful, so just save them.
+        # wptserve and pywebsocket files don't need to be merged, so just save them.
         self.add_helper(
             FilenameRegexMatch(r'pywebsocket\.ws\.log-.*-err\.txt$'),
             MergeFilesKeepFiles(self.filesystem))
+        self.add_helper(
+            FilenameRegexMatch(r'wptserve_stderr\.txt$'),
+            MergeFilesKeepFiles(self.filesystem))
 
         # These JSON files have "result style" JSON in them.
         results_json_file_merger = MergeFilesJSONP(

third_party/blink/tools/blinkpy/web_tests/servers/server_base.py

@@ -51,9 +51,8 @@ class ServerBase(object):
         self._platform = port_obj.host.platform
         self._output_dir = output_dir
 
-        # On Mac and Linux tmpdir is set to '/tmp' for (i) consistency
-        # and (ii) because it is hardcoded in the Apache
-        # ScoreBoardFile directive.
+        # On Mac and Linux tmpdir is set to '/tmp' for (i) consistency and
+        # (ii) because it is hardcoded in the Apache ScoreBoardFile directive.
         tmpdir = tempfile.gettempdir()
         if self._platform.is_mac() or self._platform.is_linux():
             tmpdir = '/tmp'
@@ -71,6 +70,10 @@ class ServerBase(object):
         # Subclasses may override these fields.
         self._env = None
         self._cwd = None
+        # TODO(robertma): There is a risk of deadlocks since we don't read from
+        # the pipes until the subprocess exits. For now, subclasses need to
+        # either make sure server processes don't spam on stdout/stderr, or
+        # redirect them to files.
         self._stdout = self._executive.PIPE
         self._stderr = self._executive.PIPE
         self._process = None

third_party/blink/tools/blinkpy/web_tests/servers/wptserve.py

@@ -23,7 +23,7 @@ class WPTServe(server_base.ServerBase):
         http_port, http_alt_port, https_port = (8001, 8081, 8444)
         ws_port, wss_port = (9001, 9444)
         self._name = 'wptserve'
-        self._log_prefixes = ('access_log', 'error_log')
+        self._log_prefixes = ('wptserve_stderr', )
         self._mappings = [{'port': http_port, 'scheme': 'http'},
                           {'port': http_alt_port, 'scheme': 'http'},
                           {'port': https_port, 'scheme': 'https', 'sslcert': True},
@@ -45,11 +45,8 @@ class WPTServe(server_base.ServerBase):
         start_cmd = [self._port_obj.host.executable,
                      '-u', wpt_script, 'serve',
                      '--config', self._config_file,
-                     '--doc_root', path_to_wpt_tests]
-
-        # TODO(burnik): Merge with default start_cmd once we roll in websockets.
-        if self._port_obj.host.filesystem.exists(path_to_ws_handlers):
-            start_cmd += ['--ws_doc_root', path_to_ws_handlers]
+                     '--doc_root', path_to_wpt_tests,
+                     '--ws_doc_root', path_to_ws_handlers]
 
         # TODO(burnik): We should stop setting the CWD once WPT can be run without it.
         self._cwd = path_to_wpt_root
@@ -57,6 +54,8 @@ class WPTServe(server_base.ServerBase):
         self._env.update({'PYTHONPATH': path_to_pywebsocket})
         self._start_cmd = start_cmd
 
+        self._error_log_path = self._filesystem.join(output_dir, 'wptserve_stderr.txt')
+
         expiration_date = datetime.date(2025, 1, 4)
         if datetime.date.today() > expiration_date - datetime.timedelta(30):
             _log.error(
@@ -78,6 +77,13 @@ class WPTServe(server_base.ServerBase):
         f.close()
         return temp_file
 
+    def _prepare_config(self):
+        # wptserve is spammy on stderr even at the INFO log level and will block
+        # the pipe, so we need to redirect it.
+        # The file is opened here instead in __init__ because _remove_stale_logs
+        # will try to delete the log file, which causes deadlocks on Windows.
+        self._stderr = self._filesystem.open_text_file_for_writing(self._error_log_path)
+
     def _stop_running_server(self):
         if not self._wait_for_action(self._check_and_kill):
             # This is mostly for POSIX systems. We send SIGINT in

third_party/blink/tools/blinkpy/web_tests/servers/wptserve_unittest.py

@@ -37,7 +37,9 @@ class TestWPTServe(LoggingTestCase):
                 '--config',
                 server._config_file,
                 '--doc_root',
-                '/test.checkout/wtests/external/wpt'
+                '/test.checkout/wtests/external/wpt',
+                '--ws_doc_root',
+                '/test.checkout/wtests/external/wpt/websockets/handlers'
             ])
 
     def test_init_gen_config(self):