Attachment 347798 Details for Bug 188848 – Patch

[patch] Patch

bug-188848-20180822084919.patch (text/plain), 12.07 KB, created by youenn fablet on 2018-08-22 08:49:20 PDT

(hide)

Description:

Filename:

MIME Type:

Creator: youenn fablet

Created: 2018-08-22 08:49:20 PDT

Size: 12.07 KB

patch

obsolete

>Subversion Revision: 235093
>diff --git a/Tools/ChangeLog b/Tools/ChangeLog
>index cb2ec2b264af8b241d9fd523f875c1e49842c4c8..dc1dab03713f97f85d95461d55c1ac1379bd29ba 100644
>--- a/Tools/ChangeLog
>+++ b/Tools/ChangeLog
>@@ -1,3 +1,27 @@
>+2018-08-20  Youenn Fablet  <youenn@apple.com>
>+
>+        Use "wpt serve" to launch WPT server
>+        https://bugs.webkit.org/show_bug.cgi?id=188848
>+
>+        Reviewed by NOBODY (OOPS!).
>+
>+        Use "wpt serve" provided by WPT instead of using our custom launcher.
>+        This simplifies things and will avoid future breakage.
>+        Further simplify web_platform_test_server.py by removing no longer needed actions.
>+        - Do not copy files but use alias
>+        - Do not kill main pid, which leaves subprocesses alive, use interrupt instead.
>+        - Stop enumerating subprocess pids.
>+
>+        * Scripts/webkitpy/common/system/executive_mock.py:
>+        (MockExecutive.interrupt):
>+        * Scripts/webkitpy/layout_tests/servers/web_platform_test_launcher.py: Removed.
>+        * Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py:
>+        (WebPlatformTestServer.__init__):
>+        (WebPlatformTestServer.ports_to_forward):
>+        (WebPlatformTestServer._prepare_config):
>+        (WebPlatformTestServer._spawn_process):
>+        (WebPlatformTestServer._stop_running_server):
>+
> 2018-08-20  Thomas Denney  <tdenney@apple.com>
> 
>         Added Thomas Denney to contributors.json.
>diff --git a/Tools/Scripts/webkitpy/common/system/executive_mock.py b/Tools/Scripts/webkitpy/common/system/executive_mock.py
>index bc78ffe5a2d8b3b2a3abcd0d99a33a39d4da4167..dc063adfe4b667453c80dc59f25708c371491eea 100644
>--- a/Tools/Scripts/webkitpy/common/system/executive_mock.py
>+++ b/Tools/Scripts/webkitpy/common/system/executive_mock.py
>@@ -143,6 +143,9 @@ class MockExecutive(object):
>     def kill_process(self, pid):
>         pass
> 
>+    def interrupt(self, pid):
>+        pass
>+
>     def popen(self, args, cwd=None, env=None, **kwargs):
>         self.calls.append(args)
>         if self._should_log:
>diff --git a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_launcher.py b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_launcher.py
>deleted file mode 100755
>index eb5fa0918817508340553ec311dc92fb5473a03d..0000000000000000000000000000000000000000
>--- a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_launcher.py
>+++ /dev/null
>@@ -1,68 +0,0 @@
>-# This Source Code Form is subject to the terms of the 3-clause BSD License
>-# http://www.w3.org/Consortium/Legal/2008/03-bsd-license.html
>-
>-import json
>-import logging
>-import os
>-import sys
>-import uuid
>-
>-logger = logging.getLogger('web-platform-test-launcher')
>-logging.basicConfig(level=logging.DEBUG)
>-
>-try:
>-    sys.path.insert(0, os.getcwd())
>-    import tools.serve.serve as WebPlatformTestServer
>-    sys.path.insert(0, os.path.join(os.getcwd(), "tools", "wptserve", "wptserve"))
>-    import stash
>-except ImportError as e:
>-    logger.critical("Import of wpt serve module failed.\n"
>-        "Please check that the file serve.py is present in the web-platform-tests folder.\n"
>-        "Please also check that __init__.py files in the web-platform-tests/tools folder and subfolders are also present.")
>-    raise
>-# This script is used to launch the web platform test server main script (serve.py) and stop it when asked by run-webkit-tests
>-
>-
>-def build_routes(aliases):
>-    builder = WebPlatformTestServer.RoutesBuilder()
>-    for alias in aliases:
>-        url = alias["url-path"]
>-        directory = alias["local-dir"]
>-        if not url.startswith("/") or len(directory) == 0:
>-            logger.error("\"url-path\" value must start with '/'.")
>-            continue
>-        if url.endswith("/"):
>-            logger.info("\n\nadding mount point " + url + " " + directory + "\n\n")
>-            builder.add_mount_point(url, directory)
>-        else:
>-            builder.add_file_mount_point(url, directory)
>-    builder.add_mount_point("/WebKit/", "../../../http/wpt/")
>-    return builder.get_routes()
>-
>-
>-def main(argv, stdout, stderr):
>-    # This is a copy of serve.py main function, except for the wait step
>-    config = WebPlatformTestServer.load_config("config.default.json", "config.json")
>-    WebPlatformTestServer.setup_logger(config["log_level"])
>-    logged_servers = []
>-
>-    with stash.StashServer((config["browser_host"], WebPlatformTestServer.get_port()), authkey=str(uuid.uuid4())):
>-        with WebPlatformTestServer.get_ssl_environment(config) as ssl_env:
>-            ports = WebPlatformTestServer.get_ports(config, ssl_env)
>-            config_ = WebPlatformTestServer.normalise_config(config, ports)
>-            started_servers = WebPlatformTestServer.start(config_, ssl_env, build_routes(config["aliases"]))
>-
>-            for protocol, servers in started_servers.items():
>-                for port, process in servers:
>-                    logged_servers.append({"protocol": protocol, "port": port, "pid": process.proc.pid})
>-                    logger.info("%s, port:%d, pid:%d" % (protocol, port, process.proc.pid))
>-
>-            # Write pids in a file in case abrupt shutdown is needed
>-            with open(argv[0], "wb") as servers_file:
>-                json.dump(logged_servers, servers_file)
>-
>-            sys.stdin.read(1)
>-
>-
>-if __name__ == "__main__":
>-    sys.exit(main(sys.argv[1:], sys.stdout, sys.stderr))
>diff --git a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
>index 6444f5e59c89fadf920f0ed5b046f2b421b7625a..00b22f3825383755ecaab19663e7d6ed096ccc31 100755
>--- a/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
>+++ b/Tools/Scripts/webkitpy/layout_tests/servers/web_platform_test_server.py
>@@ -86,17 +86,16 @@ class WebPlatformTestServer(http_server_base.HttpServerBase):
>         self._pid_file = pidfile
>         if not self._pid_file:
>             self._pid_file = self._filesystem.join(self._runtime_path, '%s.pid' % self._name)
>-        self._servers_file = self._filesystem.join(self._runtime_path, '%s_servers.json' % (self._name))
> 
>         self._filesystem = port_obj.host.filesystem
>         self._layout_root = port_obj.layout_tests_dir()
>         self._doc_root = self._filesystem.join(self._layout_root, doc_root(port_obj))
> 
>-        self._resources_files_to_copy = []
>-
>-        current_dir_path = self._filesystem.abspath(self._filesystem.split(__file__)[0])
>-        self._start_cmd = ["python", self._filesystem.join(current_dir_path, "web_platform_test_launcher.py"), self._servers_file]
>         self._doc_root_path = self._filesystem.join(self._layout_root, self._doc_root)
>+        self._config_filename = self._filesystem.join(self._doc_root_path, "config.json")
>+
>+        wpt_file = self._filesystem.join(self._doc_root_path, "wpt.py")
>+        self._start_cmd = ["python", wpt_file, "serve", "--config", self._config_filename]
> 
>         self._mappings = []
>         config = wpt_config_json(port_obj)
>@@ -112,39 +111,16 @@ class WebPlatformTestServer(http_server_base.HttpServerBase):
>     def ports_to_forward(self):
>         return [mapping['port'] for mapping in self._mappings]
> 
>-    def _copy_webkit_test_files(self):
>-        _log.debug('Copying WebKit resources files')
>-        for f in self._resources_files_to_copy:
>-            webkit_filename = self._filesystem.join(self._layout_root, "resources", f)
>-            if self._filesystem.isfile(webkit_filename):
>-                self._filesystem.copyfile(webkit_filename, self._filesystem.join(self._doc_root, "resources", f))
>-        _log.debug('Copying WebKit web platform server config.json')
>-        config_wk_filename = self._filesystem.join(self._layout_root, "imported", "w3c", "resources", "config.json")
>-        if self._filesystem.isfile(config_wk_filename):
>-            config_json = self._filesystem.read_text_file(config_wk_filename).replace("%CERTS_DIR%", self._filesystem.join(self._output_dir, "_wpt_certs"))
>-            self._filesystem.write_text_file(self._filesystem.join(self._doc_root, "config.json"), config_json)
>-
>-        wpt_testharnessjs_file = self._filesystem.join(self._doc_root, "resources", "testharness.js")
>-        layout_tests_testharnessjs_file = self._filesystem.join(self._layout_root, "resources", "testharness.js")
>-        if (not self._filesystem.compare(wpt_testharnessjs_file, layout_tests_testharnessjs_file)):
>-            _log.warning("\n//////////\nWPT tests are not using the same testharness.js file as other WebKit Layout tests.\nWebKit testharness.js might need to be updated according WPT testharness.js.\n//////////\n")
>-
>-    def _clean_webkit_test_files(self):
>-        _log.debug('Cleaning WPT resources files')
>-        for f in self._resources_files_to_copy:
>-            wpt_filename = self._filesystem.join(self._doc_root, "resources", f)
>-            if self._filesystem.isfile(wpt_filename):
>-                self._filesystem.remove(wpt_filename)
>-        _log.debug('Cleaning WPT web platform server config.json')
>-        config_wpt_filename = self._filesystem.join(self._doc_root, "config.json")
>-        if self._filesystem.isfile(config_wpt_filename):
>-            self._filesystem.remove(config_wpt_filename)
>-
>     def _prepare_config(self):
>         self._filesystem.maybe_make_directory(self._output_dir)
>         self._output_log_path = self._filesystem.join(self._output_dir, self._log_file_name)
>         self._wsout = self._filesystem.open_text_file_for_writing(self._output_log_path)
>-        self._copy_webkit_test_files()
>+
>+        _log.debug('Copying WebKit web platform server config.json')
>+        config_wk_filename = self._filesystem.join(self._layout_root, "imported", "w3c", "resources", "config.json")
>+        if self._filesystem.isfile(config_wk_filename):
>+            config_json = self._filesystem.read_text_file(config_wk_filename).replace("%CERTS_DIR%", self._filesystem.join(self._output_dir, "_wpt_certs"))
>+            self._filesystem.write_text_file(self._config_filename, config_json)
> 
>     def _spawn_process(self):
>         self._process = self._executive.popen(self._start_cmd, cwd=self._doc_root_path, shell=False, stdin=self._executive.PIPE, stdout=self._wsout, stderr=self._wsout)
>@@ -165,37 +141,17 @@ class WebPlatformTestServer(http_server_base.HttpServerBase):
> 
>         return self._process.pid
> 
>-    def _stop_running_subservers(self):
>-        if self._filesystem.exists(self._servers_file):
>-            try:
>-                json_data = self._filesystem.read_text_file(self._servers_file)
>-                started_servers = json.loads(json_data)
>-                for server in started_servers:
>-                    if self._executive.check_running_pid(server['pid']):
>-                        _log.warning('Killing server process (protocol: %s , port: %d, pid: %d).' % (server['protocol'], server['port'], server['pid']))
>-                        self._executive.kill_process(server['pid'])
>-            finally:
>-                self._filesystem.remove(self._servers_file)
>-
>-    def stop(self):
>-        super(WebPlatformTestServer, self).stop()
>-        # In case of orphaned pid, kill the running subservers if any still alive.
>-        self._stop_running_subservers()
>-
>     def _stop_running_server(self):
>-        _log.debug('Stopping %s server' % (self._name))
>-        self._clean_webkit_test_files()
>+        _log.debug('Cleaning WPT web platform server config.json')
>+        if self._filesystem.isfile(self._config_filename):
>+            self._filesystem.remove(self._config_filename)
> 
>-        if self._process:
>-            self._process.communicate(input='\n')
>         if self._wsout:
>             self._wsout.close()
>             self._wsout = None
> 
>-        if self._pid and self._executive.check_running_pid(self._pid):
>-            _log.warning('Cannot stop %s server normally.' % (self._name))
>-            _log.warning('Killing server launcher process (pid: %d).' % (self._pid))
>-            self._executive.kill_process(self._pid)
>+        if self._pid:
>+            # kill_process will not kill the subprocesses, interrupt does the job.
>+            self._executive.interrupt(self._pid)
> 
>         self._remove_pid_file()
>-        self._stop_running_subservers()

Actions: View | Formatted Diff | Diff

Attachments on bug 188848: 347798 | 347803