Introduce WPTServe for running W3C Blink Layout tests

Tools/Scripts/webkitpy/layout_tests/servers/wptserve.py

+# Copyright 2015 The Chromium Authors. All rights reserved.
+# Use of this source code is governed by a BSD-style license that can be
+# found in the LICENSE file.
+
+"""Start and stop the WPTserve servers as they're used by the layout tests."""
+
+import logging
+import os
+import sys
+import socket
+
+from webkitpy.layout_tests.servers import server_base
+
+
+_log = logging.getLogger(__name__)
+
+
+class WPTServe(server_base.ServerBase):
+    def __init__(self, port_obj, output_dir):
+        super(WPTServe, self).__init__(port_obj, output_dir)
+        # These ports must match wpt_support/wpt.config.json
+        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._mappings = [{'port': http_port},
+                          {'port': http_alt_port},
+                          {'port': https_port, 'sslcert': True},
+                          {'port': ws_port},
+                          {'port': wss_port, 'sslcert': True}]
+
+        # TODO(burnik): We can probably avoid PID files for WPT in the future.
+        self._pid_file = self._filesystem.join(self._runtime_path, '%s.pid' % self._name)
+
+        self._path_to_thirdparty = self._port_obj.path_from_webkit_base('Tools', 'Scripts', 'webkitpy', 'thirdparty')
+        self._executive.pythonpath.append(self._path_to_thirdparty)
+        self._executive.pythonpath.update()

[Dirk Pranke, 2015/06/12 22:25:02]

You don't need this.

If you set 

self._env = {'PYTHONPATH': self._path_to_thirdparty} then
you can pass env=self._env in the popen() call.

In addition, if you set self._stdout = self._stderr = self._executive.DEVNULL, 
you should be able to use ServerBases's implementation of _spawn_process()
directly
and not need to override it.

You might need to add a way to specify the cwd, but it seems like that's a bug
in wptserve if it needs to be started from a particular directory.

[burnik, 2015/06/15 10:35:09]

Done.

+        path_to_wpt_support = self._port_obj.path_from_webkit_base('Tools', 'Scripts', 'webkitpy', 'thirdparty', 'wpt')
+        self._path_to_wpt_root = os.path.join(path_to_wpt_support, 'wpt')

[Dirk Pranke, 2015/06/12 22:25:01]

Use self._filesystemm.join() instead of os.path.join(), here and elsewhere. 

Modules outside of webkitpy.common.system should generally not need to 
import sys or os.

[burnik, 2015/06/15 10:35:10]

Done.

+        self._executable = os.path.join(self._path_to_wpt_root, 'serve')
+        path_to_wpt_config = os.path.join(path_to_wpt_support, 'wpt.config.json')
+        path_to_wpt_tests = os.path.abspath(os.path.join(self._port_obj.layout_tests_dir(),
+                                                         'imported', 'web-platform-tests'))
+        path_to_ws_handlers = os.path.join(path_to_wpt_tests, 'websockets', 'handlers')
+        start_cmd = [self._executable,

[Dirk Pranke, 2015/06/12 22:25:02]

This should be 

    start_cmd = [self.port_obj.host.executable, self._executable, ...]

and you should rename self.executable to something else.

You don't want to run the python script directly; that won't work on Windows.

Also, if you make the change I suggest you don't need to call popen(...,
shell=True).

[burnik, 2015/06/15 10:35:09]

Done.

+                     '--config', path_to_wpt_config,
+                     '--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]
+
+        self._start_cmd = start_cmd
+
+    def _spawn_process(self):

[Dirk Pranke, 2015/06/12 22:25:02]

As noted above, let's see if we can make the base class impl work for you.

[burnik, 2015/06/15 10:35:10]

Done.

+        _log.debug('Starting %s server, cmd="%s"' % (self._name, str(self._start_cmd)))
+        self._process = self._executive.popen(' '.join(self._start_cmd),
+                                              cwd=self._path_to_wpt_root,
+                                              shell=True,
+                                              stdout=self._executive.DEVNULL,
+                                              stderr=self._executive.DEVNULL,
+                                              preexec_fn=os.setsid)

[Dirk Pranke, 2015/06/12 22:25:01]

AFAICT, you don't need (and don't want) os.setsid to be called.

Why did you think you needed this?

[burnik, 2015/06/15 10:35:10]

It had to do with kill_process when killing a group. Removed.

+
+        # We probably won't need a PID file, but server_base needs it.
+        fs = self._port_obj.host.filesystem
+        fs.write_text_file(self._pid_file, str(self._process.pid))
+        return self._process.pid
+
+    def stop(self):
+        self._stop_running_server()

[Dirk Pranke, 2015/06/12 22:25:01]

I don't think you need to override the base class here.

[burnik, 2015/06/15 10:35:09]

Done.

+
+    def _stop_running_server(self):
+        # Clean up the pid file.
+        if self._pid and not self._executive.check_running_pid(self._pid):
+            self._filesystem.remove(self._pid_file)
+            return
+
+        self._executive.pythonpath.remove(self._path_to_thirdparty)
+        self._executive.pythonpath.update()
+
+        # TODO(burnik): Figure out a cleaner way of stopping wptserve.
+        self._executive.interrupt(self._pid)

[Dirk Pranke, 2015/06/12 22:25:01]

We might need to add another hook to ServerBase so that you can override it
to call executive.interrupt() + self._process.wait() instead of 
executive.kill_process().

I think you can get rid of the rest of the code in this method and 
just use the base class impl otherwise.

[burnik, 2015/06/15 10:35:09]

Actually we would need two hooks. 
One telling the base to keep the reference to the process (see _spawn_process)
so we can wait on it when stopping and the other one to signify we want to call
interrupt+wait. 

As I see it, the ServerBase is more tuned to the Apache approach than WPTServe.
I would therefore keep this _stop_running_server method and probably make
attempt to fix this start/stop mechanism on the WPTServe side.
There is already a TODO saying that I should find a cleaner way of doing it.

+
+        # According to Popen.wait(), this can deadlock when using stdout=PIPE and/or stderr=PIPE.
+        # We're using DEVNULL for both so that should not occur.
+        self._process.wait()