From 4707f2a40d750f5d5c0780b84c14d745bb85617e Mon Sep 17 00:00:00 2001 From: James Graham Date: Wed, 13 Nov 2024 12:54:20 +0000 Subject: [PATCH] Allow specifying extra websockets handler directories This allows vendors to write their own websockets handlers for non-shared tests. Note that the handlers still all share the same namespace, so vendor handlers must have a globally unique name, not just unique in the vendor directory. Differential Revision: https://phabricator.services.mozilla.com/D222758 bugzilla-url: https://bugzilla.mozilla.org/show_bug.cgi?id=1919741 gecko-commit: 91b6ceb89eae8391c7b913f52e600e3bb04c412e gecko-reviewers: Sasha --- tools/serve/serve.py | 16 +++- tools/wptrunner/docs/config.rst | 86 +++++++++++++++++++++ tools/wptrunner/wptrunner/config.py | 15 +++- tools/wptrunner/wptrunner/environment.py | 5 +- tools/wptrunner/wptrunner/wptcommandline.py | 40 ++++++---- tools/wptrunner/wptrunner/wptrunner.py | 3 +- 6 files changed, 140 insertions(+), 25 deletions(-) create mode 100644 tools/wptrunner/docs/config.rst diff --git a/tools/serve/serve.py b/tools/serve/serve.py index bbe2b53b470ba7..6ad06cafbd147a 100644 --- a/tools/serve/serve.py +++ b/tools/serve/serve.py @@ -886,7 +886,8 @@ def start_http2_server(logger, host, port, paths, routes, bind_address, config, class WebSocketDaemon: - def __init__(self, host, port, doc_root, handlers_root, bind_address, ssl_config): + def __init__(self, host, port, doc_root, handlers_root, bind_address, ssl_config, + extra_handler_paths=None): logger = logging.getLogger() self.host = host cmd_args = ["-p", port, @@ -904,6 +905,9 @@ def __init__(self, host, port, doc_root, handlers_root, bind_address, ssl_config opts.cgi_directories = [] opts.is_executable_method = None self.server = pywebsocket.WebSocketServer(opts) + if extra_handler_paths: + for path in extra_handler_paths: + self.server.websocket_server_options.dispatcher._source_handler_files_in_dir(path, path, False, None) ports = [item[0].getsockname()[1] for item in self.server._sockets] if not ports: # TODO: Fix the logging configuration in WebSockets processes @@ -947,7 +951,8 @@ def start_ws_server(logger, host, port, paths, routes, bind_address, config, **k repo_root, config.paths["ws_doc_root"], bind_address, - ssl_config=None) + ssl_config=None, + extra_handler_paths=config.paths["ws_extra"]) except Exception as error: logger.critical(f"start_ws_server: Caught exception from WebSocketDomain: {error}") startup_failed(logger) @@ -960,7 +965,8 @@ def start_wss_server(logger, host, port, paths, routes, bind_address, config, ** repo_root, config.paths["ws_doc_root"], bind_address, - config.ssl_config) + config.ssl_config, + extra_handler_paths=config.paths["ws_extra"]) except Exception as error: logger.critical(f"start_wss_server: Caught exception from WebSocketDomain: {error}") startup_failed(logger) @@ -1033,6 +1039,7 @@ class ConfigBuilder(config.ConfigBuilder): }, "doc_root": repo_root, "ws_doc_root": os.path.join(repo_root, "websockets", "handlers"), + "ws_extra": None, "server_host": None, "ports": { "http": [8000, "auto"], @@ -1101,6 +1108,7 @@ def _get_ws_doc_root(self, data): def _get_paths(self, data): rv = super()._get_paths(data) rv["ws_doc_root"] = data["ws_doc_root"] + rv["ws_extra"] = data["ws_extra"] return rv @@ -1156,6 +1164,8 @@ def get_parser(): help="Path to document root. Overrides config.") parser.add_argument("--ws_doc_root", action="store", dest="ws_doc_root", help="Path to WebSockets document root. Overrides config.") + parser.add_argument("--ws_extra", action="append", dest="ws_extra", default=[], + help="Path to extra directory containing ws handlers. Overrides config.") parser.add_argument("--inject-script", default=None, help="Path to script file to inject, useful for testing polyfills.") parser.add_argument("--alias_file", action="store", dest="alias_file", diff --git a/tools/wptrunner/docs/config.rst b/tools/wptrunner/docs/config.rst new file mode 100644 index 00000000000000..83ed938ed220d4 --- /dev/null +++ b/tools/wptrunner/docs/config.rst @@ -0,0 +1,86 @@ +wptrunner Configuration +======================= + +wptrunner can be configured using two mechanisms: + + * Command line arguments + + * A ``wptrunner.ini`` configuration file + +Command Line Arguments +---------------------- + +Command line arguments are the most common way of configuring +wptrunner. The current list of command line arguments can be seen by +starting wptrunner with the ``--help`` command line argument. + +Command line arguments override options given in the configuration file. + + +Configuration File +------------------ + +A configuration file can be passed using the ``--config`` command line +argument. If no argument is supplied then ``wptrunner.ini`` in the +current working directory will be used, if it exists, otherwise +``wptrunner.default.ini`` in the wptrunner directory. Only a single +configuration file is used. + +Typicaly frontends to wptrunner are expected to pass in their own +configuration file. + +The configuration file contains the following known paths and sections: + +:paths: + Data about default paths to use. + + :prefs: + Path to profile root directory. Equivalent to the + ``--profile-root`` command line argument. + + :run_info: + Path to the directory containing extra run info JSON + files to add to the run info data. Equivalent to the ``--run-info`` + command line argument. + + :ws_extra: + Semicolon-separated list of extra paths to use for + websockets handlers. Equivalent to the ``--ws-extra`` command line + argument. + +:web-platform-tests: + Data about the web-platform-tests repository. This is only used by the + repository sync code and can be considered deprecated. + + :remote_url: URL of the wpt repository to sync from + :branch: Branch name to sync from + :sync_path: Directory to use when performing a sync + +In addition the command line allows specifying *multiple* sections +each corresponding to a test manifest. These are named +``manifest:[name]``. The ``name`` is arbitary, but must be unique in +the file. At least one such section is required so that wptrunner +knows where to find some tests. + +:manifest\:[name]: + Data about tests in a given subtree. + + :tests: Path to the root of the subtree containing tests. + :meta: Path to the corresponding metadata directory. + :url_base: URL prefix to for the tests in this manifest. This + should be ``/`` for the default manifest but must be + different for other manifests. + +For example a vendor with both upstream web-platform-tests under an +``upstream`` subtree, and vendor-specific web-platform-tests under a +``local`` substree, might have a configuration like:: + + [manifest:upstream] + tests = upstream/tests + metadata = upstream/meta + url_base = / + + [manifest:vendor] + tests = local/tests + metadata = local/meta + url_base = /_local/ diff --git a/tools/wptrunner/wptrunner/config.py b/tools/wptrunner/wptrunner/config.py index d78efbfd2dff09..3ef4c0d03460ec 100644 --- a/tools/wptrunner/wptrunner/config.py +++ b/tools/wptrunner/wptrunner/config.py @@ -4,7 +4,7 @@ import os import sys from collections import OrderedDict -from typing import Dict, Mapping, Optional +from typing import Dict, Mapping, Optional, List here = os.path.dirname(__file__) @@ -14,12 +14,19 @@ def __init__(self, base_path: str, *args: str, **kwargs: str): self.base_path = base_path dict.__init__(self, *args, **kwargs) + def _normalize_path(self, path: str) -> str: + os.path.expanduser(path) + return os.path.abspath(os.path.join(self.base_path, path)) + def get_path(self, key: str, default:Optional[str] = None) -> Optional[str]: if key not in self: return default - path = self[key] - os.path.expanduser(path) - return os.path.abspath(os.path.join(self.base_path, path)) + return self._normalize_path(self[key]) + + def get_paths(self, key: str, default:Optional[List[str]] = None) -> Optional[List[str]]: + if key not in self: + return default + return [self._normalize_path(item.strip()) for item in self[key].split(";")] def read(config_path: str) -> Mapping[str, ConfigDict]: diff --git a/tools/wptrunner/wptrunner/environment.py b/tools/wptrunner/wptrunner/environment.py index f8538b7da1d7f4..071ff6df716594 100644 --- a/tools/wptrunner/wptrunner/environment.py +++ b/tools/wptrunner/wptrunner/environment.py @@ -98,7 +98,7 @@ class TestEnvironment: def __init__(self, test_paths, testharness_timeout_multipler, pause_after_test, debug_test, debug_info, options, ssl_config, env_extras, enable_webtransport=False, mojojs_path=None, inject_script=None, - suppress_handler_traceback=None): + suppress_handler_traceback=None, ws_extra=None): self.test_paths = test_paths self.server = None @@ -124,6 +124,7 @@ def __init__(self, test_paths, testharness_timeout_multipler, self.mojojs_path = mojojs_path self.inject_script = inject_script self.suppress_handler_traceback = suppress_handler_traceback + self.ws_extra = ws_extra def __enter__(self): server_log_handler = self._stack.enter_context(self.server_logging_ctx) @@ -177,7 +178,7 @@ def ignore_interrupts(self): def build_config(self): override_path = os.path.join(serve_path(self.test_paths), "config.json") - config = serve.ConfigBuilder(self.server_logger) + config = serve.ConfigBuilder(self.server_logger, ws_extra=self.ws_extra) ports = { "http": [8000, 8001], diff --git a/tools/wptrunner/wptrunner/wptcommandline.py b/tools/wptrunner/wptrunner/wptcommandline.py index e45ed0807f6af2..421c003071e45f 100644 --- a/tools/wptrunner/wptrunner/wptcommandline.py +++ b/tools/wptrunner/wptrunner/wptcommandline.py @@ -290,6 +290,9 @@ def create_parser(product_choices=None): config_group.add_argument("--no-suppress-handler-traceback", action="store_false", dest="supress_handler_traceback", help="Write the stacktrace for exceptions in server handlers") + config_group.add_argument("--ws-extra", action="append", default=None, + dest="ws_extra", + help="Extra paths containing websockets handlers") build_type = parser.add_mutually_exclusive_group() build_type.add_argument("--debug-build", dest="debug", action="store_true", @@ -469,24 +472,31 @@ def set_from_config(kwargs): kwargs["product"] = products.Product(kwargs["config"], kwargs["product"]) - keys = {"paths": [("prefs", "prefs_root", True), - ("run_info", "run_info", True)], - "web-platform-tests": [("remote_url", "remote_url", False), - ("branch", "branch", False), - ("sync_path", "sync_path", True)], - "SSL": [("openssl_binary", "openssl_binary", True), - ("certutil_binary", "certutil_binary", True), - ("ca_cert_path", "ca_cert_path", True), - ("host_cert_path", "host_cert_path", True), - ("host_key_path", "host_key_path", True)]} + keys = {"paths": [("prefs", "prefs_root", "path"), + ("run_info", "run_info", "path"), + ("ws_extra", "ws_extra", "paths")], + "web-platform-tests": [("remote_url", "remote_url", "str"), + ("branch", "branch", "str"), + ("sync_path", "sync_path", "path")], + "SSL": [("openssl_binary", "openssl_binary", "path"), + ("certutil_binary", "certutil_binary", "path"), + ("ca_cert_path", "ca_cert_path", "path"), + ("host_cert_path", "host_cert_path", "path"), + ("host_key_path", "host_key_path", "path")]} + + getters = { + "str": "get", + "path": "get_path", + "paths": "get_paths" + } for section, values in keys.items(): - for config_value, kw_value, is_path in values: + for config_value, kw_value, prop_type in values: + if prop_type not in getters: + raise ValueError(f"Unknown config property type {prop_type}") + getter_name = getters[prop_type] if kw_value in kwargs and kwargs[kw_value] is None: - if not is_path: - new_value = kwargs["config"].get(section, config.ConfigDict({})).get(config_value) - else: - new_value = kwargs["config"].get(section, config.ConfigDict({})).get_path(config_value) + new_value = getattr(kwargs["config"].get(section, config.ConfigDict({})), getter_name)(config_value) kwargs[kw_value] = new_value test_paths = get_test_paths(kwargs["config"], diff --git a/tools/wptrunner/wptrunner/wptrunner.py b/tools/wptrunner/wptrunner/wptrunner.py index f390f29c0794f0..7d26bb76d75c3c 100644 --- a/tools/wptrunner/wptrunner/wptrunner.py +++ b/tools/wptrunner/wptrunner/wptrunner.py @@ -456,7 +456,8 @@ def run_tests(config, product, test_paths, **kwargs): kwargs["enable_webtransport_h3"], mojojs_path, inject_script, - kwargs["suppress_handler_traceback"]) as test_environment: + kwargs["suppress_handler_traceback"], + kwargs["ws_extra"]) as test_environment: recording.set(["startup", "ensure_environment"]) try: test_environment.ensure_started()