From e51161a84e07661f8699839c511c89767ec498a8 Mon Sep 17 00:00:00 2001 From: Jonathan Lee Date: Tue, 10 Dec 2024 12:27:36 -0800 Subject: [PATCH] [wptrunner] Decouple testdriver infrastructure from testharness (#49044) * [wptrunner] Extract executor testdriver logic into mixin This no-op refactor will allow other WebDriver executors, not just the testharness executor, to perform testdriver actions. * [wptrunner] Split `message-queue.js` from `testharnessreport.js` This will allow non-testharness tests to use `testdriver.js` without needing extra scripts. Evaluating `message-queue.js` is idempotent so that, when using testharness with testdriver, the second inclusion is a no-op. Because resource scripts are cached, the size increase should not meaningfully affect test performance. --- tools/wptrunner/wptrunner/environment.py | 67 +++---- .../wptrunner/executors/executorchrome.py | 4 +- .../wptrunner/executors/executorwebdriver.py | 188 ++++++++++-------- .../wptrunner/executors/message-queue.js | 77 +++++++ tools/wptrunner/wptrunner/testdriver-extra.js | 4 +- .../wptrunner/wptrunner/testharnessreport.js | 77 +------ tools/wptserve/wptserve/handlers.py | 14 +- 7 files changed, 229 insertions(+), 202 deletions(-) create mode 100644 tools/wptrunner/wptrunner/executors/message-queue.js diff --git a/tools/wptrunner/wptrunner/environment.py b/tools/wptrunner/wptrunner/environment.py index 071ff6df716594..9a6d3c8fb60334 100644 --- a/tools/wptrunner/wptrunner/environment.py +++ b/tools/wptrunner/wptrunner/environment.py @@ -23,8 +23,6 @@ sys.path.insert(0, repo_root) from tools import localpaths # noqa: F401 -from wptserve.handlers import StringHandler - serve = None @@ -226,29 +224,47 @@ def get_routes(self): self.config.aliases, self.config) + testharnessreport_format_args = { + "output": self.pause_after_test, + "timeout_multiplier": self.testharness_timeout_multipler, + "explicit_timeout": "true" if self.debug_info is not None else "false", + "debug": "true" if self.debug_test else "false", + } for path, format_args, content_type, route in [ ("testharness_runner.html", {}, "text/html", "/testharness_runner.html"), ("print_pdf_runner.html", {}, "text/html", "/print_pdf_runner.html"), - (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.js"), None, + (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.js"), {}, "text/javascript", "/_pdf_js/pdf.js"), - (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.worker.js"), None, + (os.path.join(here, "..", "..", "third_party", "pdf_js", "pdf.worker.js"), {}, "text/javascript", "/_pdf_js/pdf.worker.js"), - (self.options.get("testharnessreport", "testharnessreport.js"), - {"output": self.pause_after_test, - "timeout_multiplier": self.testharness_timeout_multipler, - "explicit_timeout": "true" if self.debug_info is not None else "false", - "debug": "true" if self.debug_test else "false"}, - "text/javascript;charset=utf8", - "/resources/testharnessreport.js")]: - path = os.path.normpath(os.path.join(here, path)) + ( + self.options.get("testharnessreport", [ + # All testharness tests, even those that don't use testdriver, require + # `message-queue.js` to signal completion. + os.path.join("executors", "message-queue.js"), + "testharnessreport.js"]), + testharnessreport_format_args, + "text/javascript;charset=utf8", + "/resources/testharnessreport.js", + ), + ( + [os.path.join(repo_root, "resources", "testdriver.js"), + # Include `message-queue.js` to support testdriver in non-testharness tests. + os.path.join("executors", "message-queue.js"), + "testdriver-extra.js"], + {}, + "text/javascript", + "/resources/testdriver.js", + ), + ]: + paths = [path] if isinstance(path, str) else path + abs_paths = [os.path.normpath(os.path.join(here, path)) for path in paths] # Note that .headers. files don't apply to static routes, so we need to # readd any static headers here. headers = {"Cache-Control": "max-age=3600"} - route_builder.add_static(path, format_args, content_type, route, + route_builder.add_static(abs_paths, format_args, content_type, route, headers=headers) - route_builder.add_handler("GET", "/resources/testdriver.js", TestdriverLoader()) - for url_base, test_root in self.test_paths.items(): if url_base == "/": continue @@ -316,27 +332,6 @@ def test_servers(self): return failed, pending -class TestdriverLoader: - """A special static handler for serving `/resources/testdriver.js`. - - This handler lazily reads `testdriver{,-extra}.js` so that wptrunner doesn't - need to pass the entire file contents to child `wptserve` processes, which - can slow `wptserve` startup by several seconds (crbug.com/1479850). - """ - def __init__(self): - self._handler = None - - def __call__(self, request, response): - if not self._handler: - data = b"" - with open(os.path.join(repo_root, "resources", "testdriver.js"), "rb") as fp: - data += fp.read() - with open(os.path.join(here, "testdriver-extra.js"), "rb") as fp: - data += fp.read() - self._handler = StringHandler(data, "text/javascript") - return self._handler(request, response) - - def wait_for_service(logger: StructuredLogger, host: str, port: int, diff --git a/tools/wptrunner/wptrunner/executors/executorchrome.py b/tools/wptrunner/wptrunner/executors/executorchrome.py index 9534eaea17146a..8f9d45d6f6f429 100644 --- a/tools/wptrunner/wptrunner/executors/executorchrome.py +++ b/tools/wptrunner/wptrunner/executors/executorchrome.py @@ -111,10 +111,10 @@ def send_message(self, cmd_id, message_type, status, message=None): else: self.webdriver.execute_script(message_script) - def _get_next_message_classic(self, url): + def _get_next_message_classic(self, url, script_resume): try: message_script, self._pending_message = self._pending_message, "" - return self.parent.base.execute_script(message_script + self.script_resume, + return self.parent.base.execute_script(message_script + script_resume, asynchronous=True, args=[strip_server(url)]) except error.JavascriptErrorException as js_error: diff --git a/tools/wptrunner/wptrunner/executors/executorwebdriver.py b/tools/wptrunner/wptrunner/executors/executorwebdriver.py index 26b73b7deb7af2..6e03ba5a743d08 100644 --- a/tools/wptrunner/wptrunner/executors/executorwebdriver.py +++ b/tools/wptrunner/wptrunner/executors/executorwebdriver.py @@ -437,24 +437,22 @@ def release(self): class WebDriverTestDriverProtocolPart(TestDriverProtocolPart): def setup(self): self.webdriver = self.parent.webdriver - with open(os.path.join(here, "testharness_webdriver_resume.js")) as f: - self.script_resume = f.read() - def get_next_message(self, url, test_window): + def get_next_message(self, url, script_resume, test_window): if hasattr(self.parent, "bidi_script"): # If `bidi_script` is available, the messages can be handled via BiDi. - return self._get_next_message_bidi(url, test_window) + return self._get_next_message_bidi(url, script_resume, test_window) else: - return self._get_next_message_classic(url) + return self._get_next_message_classic(url, script_resume) - def _get_next_message_classic(self, url): + def _get_next_message_classic(self, url, script_resume): """ Get the next message from the test_driver using the classic WebDriver async script execution. This will block the event loop until the test_driver send a message. """ - return self.parent.base.execute_script(self.script_resume, asynchronous=True, args=[strip_server(url)]) + return self.parent.base.execute_script(script_resume, asynchronous=True, args=[strip_server(url)]) - def _get_next_message_bidi(self, url, test_window): + def _get_next_message_bidi(self, url, script_resume, test_window): """ Get the next message from the test_driver using async call. This will not block the event loop, which allows for processing the events from the test_runner to test_driver while waiting for the next test_driver commands. @@ -470,7 +468,7 @@ def _get_next_message_bidi(self, url, test_window): %s }).apply(null, args); }) - }""" % self.script_resume + }""" % script_resume bidi_url_argument = { "type": "string", @@ -837,64 +835,19 @@ def run_func(self): self.result_flag.set() -class WebDriverTestharnessExecutor(TestharnessExecutor): - supports_testdriver = True - protocol_cls = WebDriverProtocol - - def __init__(self, logger, browser, server_config, timeout_multiplier=1, - close_after_done=True, capabilities=None, debug_info=None, - cleanup_after_test=True, **kwargs): - """WebDriver-based executor for testharness.js tests""" - TestharnessExecutor.__init__(self, logger, browser, server_config, - timeout_multiplier=timeout_multiplier, - debug_info=debug_info) - self.protocol = self.protocol_cls(self, browser, capabilities) - with open(os.path.join(here, "window-loaded.js")) as f: - self.window_loaded_script = f.read() - - - self.close_after_done = close_after_done - self.cleanup_after_test = cleanup_after_test - - def is_alive(self): - return self.protocol.is_alive() - - def on_environment_change(self, new_environment): - if new_environment["protocol"] != self.last_environment["protocol"]: - self.protocol.testharness.load_runner(new_environment["protocol"]) - - def do_test(self, test): - url = self.test_url(test) - - success, data = WebDriverRun(self.logger, - self.do_testharness, - self.protocol, - url, - test.timeout * self.timeout_multiplier, - self.extra_timeout).run() - - if success: - data, extra = data - return self.convert_result(test, data, extra=extra) - - return (test.make_result(*data), []) - - def do_testharness(self, protocol, url, timeout): - # The previous test may not have closed its old windows (if something - # went wrong or if cleanup_after_test was False), so clean up here. - protocol.testharness.close_old_windows() +# TODO(web-platform-tests/wpt#13183): Add testdriver support to the other +# executors. +class TestDriverExecutorMixin: + def __init__(self, script_resume: str): + self.script_resume = script_resume + def run_testdriver(self, protocol, url, timeout): # If protocol implements `bidi_events`, remove all the existing subscriptions. if hasattr(protocol, 'bidi_events'): # Use protocol loop to run the async cleanup. protocol.loop.run_until_complete(protocol.bidi_events.unsubscribe_all()) - # Now start the test harness test_window = self.get_or_create_test_window(protocol) - self.protocol.base.set_window(test_window) - # Wait until about:blank has been loaded - protocol.base.execute_script(self.window_loaded_script, asynchronous=True) - # Exceptions occurred outside the main loop. unexpected_exceptions = [] @@ -948,7 +901,8 @@ async def process_bidi_event(method, params): # TODO: what to do if there are more then 1 unexpected exceptions? raise unexpected_exceptions[0] - test_driver_message = protocol.testdriver.get_next_message(url, test_window) + test_driver_message = protocol.testdriver.get_next_message(url, self.script_resume, + test_window) self.logger.debug("Receive message from testdriver: %s" % test_driver_message) # As of 2019-03-29, WebDriver does not define expected behavior for @@ -981,36 +935,100 @@ async def process_bidi_event(method, params): # Use protocol loop to run the async cleanup. protocol.loop.run_until_complete(protocol.bidi_events.unsubscribe_all()) - extra = {} - if leak_part := getattr(protocol, "leak", None): - testharness_window = protocol.base.current_window - extra_windows = set(protocol.base.window_handles()) - extra_windows -= {protocol.testharness.runner_handle, testharness_window} - protocol.testharness.close_windows(extra_windows) - try: - protocol.base.set_window(testharness_window) - if counters := leak_part.check(): - extra["leak_counters"] = counters - except webdriver_error.NoSuchWindowException: - pass - finally: - protocol.base.set_window(protocol.testharness.runner_handle) - - # Attempt to clean up any leftover windows, if allowed. This is - # preferable as it will blame the correct test if something goes wrong - # closing windows, but if the user wants to see the test results we - # have to leave the window(s) open. - if self.cleanup_after_test: - protocol.testharness.close_old_windows() - if len(unexpected_exceptions) > 0: # TODO: what to do if there are more then 1 unexpected exceptions? raise unexpected_exceptions[0] - return rv, extra + return rv + + def get_or_create_test_window(self, protocol): + return protocol.base.current_window + + +class WebDriverTestharnessExecutor(TestharnessExecutor, TestDriverExecutorMixin): + supports_testdriver = True + protocol_cls = WebDriverProtocol + + def __init__(self, logger, browser, server_config, timeout_multiplier=1, + close_after_done=True, capabilities=None, debug_info=None, + cleanup_after_test=True, **kwargs): + """WebDriver-based executor for testharness.js tests""" + TestharnessExecutor.__init__(self, logger, browser, server_config, + timeout_multiplier=timeout_multiplier, + debug_info=debug_info) + self.protocol = self.protocol_cls(self, browser, capabilities) + with open(os.path.join(here, "testharness_webdriver_resume.js")) as f: + script_resume = f.read() + TestDriverExecutorMixin.__init__(self, script_resume) + with open(os.path.join(here, "window-loaded.js")) as f: + self.window_loaded_script = f.read() + + self.close_after_done = close_after_done + self.cleanup_after_test = cleanup_after_test + + def is_alive(self): + return self.protocol.is_alive() + + def on_environment_change(self, new_environment): + if new_environment["protocol"] != self.last_environment["protocol"]: + self.protocol.testharness.load_runner(new_environment["protocol"]) + + def do_test(self, test): + url = self.test_url(test) + + success, data = WebDriverRun(self.logger, + self.do_testharness, + self.protocol, + url, + test.timeout * self.timeout_multiplier, + self.extra_timeout).run() + + if success: + data, extra = data + return self.convert_result(test, data, extra=extra) + + return (test.make_result(*data), []) + + def do_testharness(self, protocol, url, timeout): + try: + # The previous test may not have closed its old windows (if something + # went wrong or if cleanup_after_test was False), so clean up here. + protocol.testharness.close_old_windows() + raw_results = self.run_testdriver(protocol, url, timeout) + extra = {} + if counters := self._check_for_leaks(protocol): + extra["leak_counters"] = counters + return raw_results, extra + finally: + # Attempt to clean up any leftover windows, if allowed. This is + # preferable as it will blame the correct test if something goes + # wrong closing windows, but if the user wants to see the test + # results we have to leave the window(s) open. + if self.cleanup_after_test: + protocol.testharness.close_old_windows() + + def _check_for_leaks(self, protocol): + leak_part = getattr(protocol, "leak", None) + if not leak_part: + return None + testharness_window = protocol.base.current_window + extra_windows = set(protocol.base.window_handles()) + extra_windows -= {protocol.testharness.runner_handle, testharness_window} + protocol.testharness.close_windows(extra_windows) + try: + protocol.base.set_window(testharness_window) + return leak_part.check() + except webdriver_error.NoSuchWindowException: + return None + finally: + protocol.base.set_window(protocol.testharness.runner_handle) def get_or_create_test_window(self, protocol): - return protocol.base.create_window() + test_window = protocol.base.create_window() + protocol.base.set_window(test_window) + # Wait until about:blank has been loaded + protocol.base.execute_script(self.window_loaded_script, asynchronous=True) + return test_window class WebDriverRefTestExecutor(RefTestExecutor): diff --git a/tools/wptrunner/wptrunner/executors/message-queue.js b/tools/wptrunner/wptrunner/executors/message-queue.js new file mode 100644 index 00000000000000..bbad17180e0a82 --- /dev/null +++ b/tools/wptrunner/wptrunner/executors/message-queue.js @@ -0,0 +1,77 @@ +(function() { + if (window.__wptrunner_message_queue && window.__wptrunner_process_next_event) { + // Another script already set up the testdriver infrastructure. + return; + } + + class MessageQueue { + constructor() { + this.item_id = 0; + this._queue = []; + } + + push(item) { + let cmd_id = this.item_id++; + item.id = cmd_id; + this._queue.push(item); + __wptrunner_process_next_event(); + return cmd_id; + } + + shift() { + return this._queue.shift(); + } + } + + window.__wptrunner_testdriver_callback = null; + window.__wptrunner_message_queue = new MessageQueue(); + window.__wptrunner_url = null; + + window.__wptrunner_process_next_event = function() { + /* This function handles the next testdriver event. The presence of + window.testdriver_callback is used as a switch; when that function + is present we are able to handle the next event and when is is not + present we must wait. Therefore to drive the event processing, this + function must be called in two circumstances: + * Every time there is a new event that we may be able to handle + * Every time we set the callback function + This function unsets the callback, so no further testdriver actions + will be run until it is reset, which wptrunner does after it has + completed handling the current action. + */ + + if (!window.__wptrunner_testdriver_callback) { + return; + } + var data = window.__wptrunner_message_queue.shift(); + if (!data) { + return; + } + + var payload = undefined; + + switch(data.type) { + case "complete": + var tests = data.tests; + var status = data.status; + + var subtest_results = tests.map(function(x) { + return [x.name, x.status, x.message, x.stack]; + }); + payload = [status.status, + status.message, + status.stack, + subtest_results]; + clearTimeout(window.__wptrunner_timer); + break; + case "action": + payload = data; + break; + default: + return; + } + var callback = window.__wptrunner_testdriver_callback; + window.__wptrunner_testdriver_callback = null; + callback([__wptrunner_url, data.type, payload]); + }; +})(); diff --git a/tools/wptrunner/wptrunner/testdriver-extra.js b/tools/wptrunner/wptrunner/testdriver-extra.js index c4a02ef40a7eda..fa1511188046ed 100644 --- a/tools/wptrunner/wptrunner/testdriver-extra.js +++ b/tools/wptrunner/wptrunner/testdriver-extra.js @@ -59,7 +59,7 @@ }); function is_test_context() { - return window.__wptrunner_message_queue !== undefined; + return !!window.__wptrunner_is_test_context; } // Code copied from /common/utils.js @@ -242,7 +242,7 @@ }; window.test_driver_internal.set_test_context = function(context) { - if (window.__wptrunner_message_queue) { + if (is_test_context()) { throw new Error("Tried to set testharness context in a window containing testharness.js"); } testharness_context = context; diff --git a/tools/wptrunner/wptrunner/testharnessreport.js b/tools/wptrunner/wptrunner/testharnessreport.js index d385692445c508..fb9e8c678de63a 100644 --- a/tools/wptrunner/wptrunner/testharnessreport.js +++ b/tools/wptrunner/wptrunner/testharnessreport.js @@ -1,75 +1,9 @@ -class MessageQueue { - constructor() { - this.item_id = 0; - this._queue = []; - } - - push(item) { - let cmd_id = this.item_id++; - item.id = cmd_id; - this._queue.push(item); - __wptrunner_process_next_event(); - return cmd_id; - } - - shift() { - return this._queue.shift(); - } -} - -window.__wptrunner_testdriver_callback = null; -window.__wptrunner_message_queue = new MessageQueue(); -window.__wptrunner_url = null; - -window.__wptrunner_process_next_event = function() { - /* This function handles the next testdriver event. The presence of - window.testdriver_callback is used as a switch; when that function - is present we are able to handle the next event and when is is not - present we must wait. Therefore to drive the event processing, this - function must be called in two circumstances: - * Every time there is a new event that we may be able to handle - * Every time we set the callback function - This function unsets the callback, so no further testdriver actions - will be run until it is reset, which wptrunner does after it has - completed handling the current action. - */ - - if (!window.__wptrunner_testdriver_callback) { - return; - } - var data = window.__wptrunner_message_queue.shift(); - if (!data) { - return; - } - - var payload = undefined; - - switch(data.type) { - case "complete": - var tests = data.tests; - var status = data.status; - - var subtest_results = tests.map(function(x) { - return [x.name, x.status, x.message, x.stack]; - }); - payload = [status.status, - status.message, - status.stack, - subtest_results]; - clearTimeout(window.__wptrunner_timer); - break; - case "action": - payload = data; - break; - default: - return; - } - var callback = window.__wptrunner_testdriver_callback; - window.__wptrunner_testdriver_callback = null; - callback([__wptrunner_url, data.type, payload]); -}; - (function() { + // Signal to `testdriver.js` that this is the "main" test browsing context, + // meaning testdriver actions should be queued for retrieval instead of + // `postMessage()`d elsewhere. + window.__wptrunner_is_test_context = true; + var props = {output: %(output)d, timeout_multiplier: %(timeout_multiplier)s, explicit_timeout: %(explicit_timeout)s, @@ -85,4 +19,3 @@ window.__wptrunner_process_next_event = function() { }); setup(props); })(); - diff --git a/tools/wptserve/wptserve/handlers.py b/tools/wptserve/wptserve/handlers.py index 62faf47d645692..cde04b13cbb088 100644 --- a/tools/wptserve/wptserve/handlers.py +++ b/tools/wptserve/wptserve/handlers.py @@ -513,11 +513,13 @@ def __init__(self, path, format_args, content_type, **headers): Note that *.headers files have no effect in this handler. - :param path: Path to the template file to use + :param path: Path(s) to template files to use. If a sequence of paths is provided instead + of a single path, the contents of each file will be concatenated together before the + `format_args` are interpolated. :param format_args: Dictionary of values to substitute into the template file :param content_type: Content type header to server the response with :param headers: List of headers to send with responses""" - self._path = path + self._paths = [path] if isinstance(path, str) else path self._format_args = format_args self._content_type = content_type self._headers = headers @@ -525,7 +527,7 @@ def __init__(self, path, format_args, content_type, **headers): def __getnewargs_ex__(self): # Do not pickle `self._handler`, which can be arbitrarily large. - args = self._path, self._format_args, self._content_type + args = self._paths, self._format_args, self._content_type return args, self._headers def __call__(self, request, response): @@ -534,8 +536,10 @@ def __call__(self, request, response): # contents across processes can slow `wptserve` startup by several # seconds (crbug.com/1479850). if not self._handler: - with open(self._path) as f: - data = f.read() + data = "" + for path in self._paths: + with open(path) as f: + data += f.read() if self._format_args: data = data % self._format_args self._handler = StringHandler(data, self._content_type, **self._headers)