From ef26d2e05212c173a5ce4460c949e169c63dec36 Mon Sep 17 00:00:00 2001 From: Jonathan Lee Date: Sat, 5 Oct 2024 00:33:56 -0400 Subject: [PATCH 1/2] [wptrunner] Add `testdriver` flag to non-testharness manifest items This will let wptrunner gracefully skip testdriver tests if the executor doesn't support them. --- tools/manifest/item.py | 18 +++++++++++++++++- tools/manifest/sourcefile.py | 7 +++++-- tools/manifest/tests/test_manifest.py | 26 ++++++++++++++++++++++++++ 3 files changed, 48 insertions(+), 3 deletions(-) diff --git a/tools/manifest/item.py b/tools/manifest/item.py index e25f7ca2c29adc..e1f509bbdab4db 100644 --- a/tools/manifest/item.py +++ b/tools/manifest/item.py @@ -166,7 +166,7 @@ def pac(self) -> Optional[Text]: return self._extras.get("pac") @property - def testdriver(self) -> Optional[Text]: + def testdriver(self) -> Optional[bool]: return self._extras.get("testdriver") @property @@ -240,6 +240,10 @@ def fuzzy(self) -> Fuzzy: rv[key] = v return rv + @property + def testdriver(self) -> Optional[bool]: + return self._extras.get("testdriver") + def to_json(self) -> Tuple[Optional[Text], List[Tuple[Text, Text]], Dict[Text, Any]]: # type: ignore rel_url = None if self._url == self.path else self._url rv: Tuple[Optional[Text], List[Tuple[Text, Text]], Dict[Text, Any]] = (rel_url, self.references, {}) @@ -252,6 +256,8 @@ def to_json(self) -> Tuple[Optional[Text], List[Tuple[Text, Text]], Dict[Text, A extras["dpi"] = self.dpi if self.fuzzy: extras["fuzzy"] = list(self.fuzzy.items()) + if self.testdriver: + extras["testdriver"] = self.testdriver return rv @classmethod @@ -315,6 +321,16 @@ class CrashTest(URLManifestItem): def timeout(self) -> Optional[Text]: return None + @property + def testdriver(self) -> Optional[bool]: + return self._extras.get("testdriver") + + def to_json(self): # type: ignore + rel_url, extras = super().to_json() + if self.testdriver: + extras["testdriver"] = self.testdriver + return rel_url, extras + class WebDriverSpecTest(URLManifestItem): __slots__ = () diff --git a/tools/manifest/sourcefile.py b/tools/manifest/sourcefile.py index 02ab1ad4fe6017..e5fd0294a546eb 100644 --- a/tools/manifest/sourcefile.py +++ b/tools/manifest/sourcefile.py @@ -946,7 +946,8 @@ def manifest_items(self) -> Tuple[Text, List[ManifestItem]]: self.tests_root, self.rel_path, self.url_base, - self.rel_url + self.rel_url, + testdriver=self.has_testdriver, )] elif self.name_is_print_reftest: @@ -965,6 +966,7 @@ def manifest_items(self) -> Tuple[Text, List[ManifestItem]]: viewport_size=self.viewport_size, fuzzy=self.fuzzy, page_ranges=self.page_ranges, + testdriver=self.has_testdriver, )] elif self.name_is_multi_global: @@ -1065,7 +1067,8 @@ def manifest_items(self) -> Tuple[Text, List[ManifestItem]]: timeout=self.timeout, viewport_size=self.viewport_size, dpi=self.dpi, - fuzzy=self.fuzzy + fuzzy=self.fuzzy, + testdriver=self.has_testdriver, )) elif self.content_is_css_visual and not self.name_is_reference: diff --git a/tools/manifest/tests/test_manifest.py b/tools/manifest/tests/test_manifest.py index 7511b21bc83589..621b71c091e713 100644 --- a/tools/manifest/tests/test_manifest.py +++ b/tools/manifest/tests/test_manifest.py @@ -335,3 +335,29 @@ def test_manifest_spec_to_json(): ]}}, } } + + +@pytest.mark.parametrize("testdriver,expected_extra", [ + (True, {"testdriver": True}), + # Don't bloat the manifest with the `testdriver=False` default. + (False, {}), +]) +def test_dump_testdriver(testdriver, expected_extra): + m = manifest.Manifest("") + source_file = SourceFileWithTest("a" + os.path.sep + "b", "0"*40, item.RefTest, + testdriver=testdriver) + + tree, sourcefile_mock = tree_and_sourcefile_mocks([(source_file, None, True)]) + with mock.patch("tools.manifest.manifest.SourceFile", side_effect=sourcefile_mock): + assert m.update(tree) is True + + assert m.to_json() == { + 'version': 9, + 'url_base': '/', + 'items': { + 'reftest': {'a': {'b': [ + '0000000000000000000000000000000000000000', + (mock.ANY, [], expected_extra) + ]}}, + } + } From 7727e0a45d609114c9fd85817e533a068aa59e18 Mon Sep 17 00:00:00 2001 From: Jonathan Lee Date: Sat, 5 Oct 2024 02:23:19 -0400 Subject: [PATCH 2/2] [wptrunner] Support testdriver.js in {ref,print-ref,crash}tests Non-testharness executors that don't support testdriver simply skip those tests. --- docs/writing-tests/testdriver.md | 8 +++- infrastructure/crashtests/testdriver.html | 13 ++++++ .../reftest/testdriver-in-ref.html.ini | 5 +++ infrastructure/reftest/testdriver-child.html | 19 ++++++++ .../reftest/testdriver-iframe.sub.html | 29 +++++++++++++ infrastructure/reftest/testdriver-in-ref.html | 6 +++ infrastructure/reftest/testdriver-print.html | 23 ++++++++++ infrastructure/reftest/testdriver.html | 21 +++++++++ lint.ignore | 6 +++ .../wptrunner/executors/executorwebdriver.py | 27 +++++++----- .../wptrunner/executors/message-queue.js | 22 ++++++---- .../wptrunner/executors/test-wait.js | 43 ++++++++++++------- tools/wptrunner/wptrunner/testdriver-extra.js | 9 ++++ tools/wptrunner/wptrunner/wptrunner.py | 33 +++++++------- tools/wptrunner/wptrunner/wpttest.py | 30 +++++++++++-- 15 files changed, 236 insertions(+), 58 deletions(-) create mode 100644 infrastructure/crashtests/testdriver.html create mode 100644 infrastructure/metadata/infrastructure/reftest/testdriver-in-ref.html.ini create mode 100644 infrastructure/reftest/testdriver-child.html create mode 100644 infrastructure/reftest/testdriver-iframe.sub.html create mode 100644 infrastructure/reftest/testdriver-in-ref.html create mode 100644 infrastructure/reftest/testdriver-print.html create mode 100644 infrastructure/reftest/testdriver.html diff --git a/docs/writing-tests/testdriver.md b/docs/writing-tests/testdriver.md index 462c7b85503cb9..cdf081ef318d33 100644 --- a/docs/writing-tests/testdriver.md +++ b/docs/writing-tests/testdriver.md @@ -13,8 +13,12 @@ written purely using web platform APIs. Outside of automation contexts, it allows human operators to provide expected input manually (for operations which may be described in simple terms). -It is currently supported only for [testharness.js](testharness) -tests. +testdriver.js supports the following test types: +* [testharness.js](testharness) tests +* [reftests](reftests) and [print-reftests](print-reftests) that use the + `class=reftest-wait` attribute on the root element to control completion +* [crashtests](crashtest) that use the `class=test-wait` attribute to control + completion ## Markup ## diff --git a/infrastructure/crashtests/testdriver.html b/infrastructure/crashtests/testdriver.html new file mode 100644 index 00000000000000..24bb1ca19e11c5 --- /dev/null +++ b/infrastructure/crashtests/testdriver.html @@ -0,0 +1,13 @@ + + +crashtests support testdriver.js + + + + diff --git a/infrastructure/metadata/infrastructure/reftest/testdriver-in-ref.html.ini b/infrastructure/metadata/infrastructure/reftest/testdriver-in-ref.html.ini new file mode 100644 index 00000000000000..05850761b16499 --- /dev/null +++ b/infrastructure/metadata/infrastructure/reftest/testdriver-in-ref.html.ini @@ -0,0 +1,5 @@ +[testdriver-in-ref.html] + disabled: + # https://github.com/web-platform-tests/wpt/issues/13183 + if product == "firefox" or product == "firefox_android": + "marionette executor doesn't currently implement testdriver for reftests" diff --git a/infrastructure/reftest/testdriver-child.html b/infrastructure/reftest/testdriver-child.html new file mode 100644 index 00000000000000..38ea532fee11b4 --- /dev/null +++ b/infrastructure/reftest/testdriver-child.html @@ -0,0 +1,19 @@ + + + + + + diff --git a/infrastructure/reftest/testdriver-iframe.sub.html b/infrastructure/reftest/testdriver-iframe.sub.html new file mode 100644 index 00000000000000..fb62009b9a1169 --- /dev/null +++ b/infrastructure/reftest/testdriver-iframe.sub.html @@ -0,0 +1,29 @@ + + +reftests support testdriver.js in iframes + + + + + + diff --git a/infrastructure/reftest/testdriver-in-ref.html b/infrastructure/reftest/testdriver-in-ref.html new file mode 100644 index 00000000000000..8a1af46db35462 --- /dev/null +++ b/infrastructure/reftest/testdriver-in-ref.html @@ -0,0 +1,6 @@ + +references support testdriver.js + + diff --git a/infrastructure/reftest/testdriver-print.html b/infrastructure/reftest/testdriver-print.html new file mode 100644 index 00000000000000..b4dc6c9760965f --- /dev/null +++ b/infrastructure/reftest/testdriver-print.html @@ -0,0 +1,23 @@ + + +print-reftests support testdriver.js + + + + + +
page 1
+ diff --git a/infrastructure/reftest/testdriver.html b/infrastructure/reftest/testdriver.html new file mode 100644 index 00000000000000..d890d8926273af --- /dev/null +++ b/infrastructure/reftest/testdriver.html @@ -0,0 +1,21 @@ + + +reftests support testdriver.js + + + + + + diff --git a/lint.ignore b/lint.ignore index 32c22fecfec65a..26baeafc96747a 100644 --- a/lint.ignore +++ b/lint.ignore @@ -769,6 +769,12 @@ HTML INVALID SYNTAX: quirks/percentage-height-calculation.html HTML INVALID SYNTAX: trusted-types/TrustedTypePolicyFactory-getAttributeType-namespace.html # Tests which include testdriver.js but aren't testharness.js tests +# TODO(web-platform-tests/wpt#13183): Remove this rule once support is added. +TESTDRIVER-IN-UNSUPPORTED-TYPE: infrastructure/crashtests/testdriver.html +TESTDRIVER-IN-UNSUPPORTED-TYPE: infrastructure/reftest/testdriver.html +TESTDRIVER-IN-UNSUPPORTED-TYPE: infrastructure/reftest/testdriver-child.html +TESTDRIVER-IN-UNSUPPORTED-TYPE: infrastructure/reftest/testdriver-iframe.sub.html +TESTDRIVER-IN-UNSUPPORTED-TYPE: infrastructure/reftest/testdriver-print.html TESTDRIVER-IN-UNSUPPORTED-TYPE: css/css-grid/grid-model/grid-layout-stale-001.html TESTDRIVER-IN-UNSUPPORTED-TYPE: css/css-grid/grid-model/grid-layout-stale-002.html TESTDRIVER-IN-UNSUPPORTED-TYPE: css/css-scroll-anchoring/fullscreen-crash.html diff --git a/tools/wptrunner/wptrunner/executors/executorwebdriver.py b/tools/wptrunner/wptrunner/executors/executorwebdriver.py index 6e03ba5a743d08..7513887998d6cd 100644 --- a/tools/wptrunner/wptrunner/executors/executorwebdriver.py +++ b/tools/wptrunner/wptrunner/executors/executorwebdriver.py @@ -1031,8 +1031,9 @@ def get_or_create_test_window(self, protocol): return test_window -class WebDriverRefTestExecutor(RefTestExecutor): +class WebDriverRefTestExecutor(RefTestExecutor, TestDriverExecutorMixin): protocol_cls = WebDriverProtocol + supports_testdriver = True def __init__(self, logger, browser, server_config, timeout_multiplier=1, screenshot_cache=None, close_after_done=True, @@ -1056,7 +1057,8 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1, self.debug_test = debug_test with open(os.path.join(here, "test-wait.js")) as f: - self.wait_script = f.read() % {"classname": "reftest-wait"} + wait_script = f.read() % {"classname": "reftest-wait"} + TestDriverExecutorMixin.__init__(self, wait_script) def reset(self): self.implementation.reset() @@ -1102,8 +1104,9 @@ def screenshot(self, test, viewport_size, dpi, page_ranges): self.extra_timeout).run() def _screenshot(self, protocol, url, timeout): - self.protocol.base.load(url) - self.protocol.base.execute_script(self.wait_script, True) + # There's nothing we want from the "complete" message, so discard the + # return value. + self.run_testdriver(protocol, url, timeout) screenshot = self.protocol.webdriver.screenshot() if screenshot is None: @@ -1148,8 +1151,9 @@ def screenshot(self, test, viewport_size, dpi, page_ranges): self.extra_timeout).run() def _render(self, protocol, url, timeout): - protocol.webdriver.url = url - protocol.base.execute_script(self.wait_script, asynchronous=True) + # There's nothing we want from the "complete" message, so discard the + # return value. + self.run_testdriver(protocol, url, timeout) pdf = protocol.pdf_print.render_as_pdf(*self.viewport_size) screenshots = protocol.pdf_print.pdf_to_png(pdf, self.page_ranges) @@ -1161,8 +1165,9 @@ def _render(self, protocol, url, timeout): return screenshots -class WebDriverCrashtestExecutor(CrashtestExecutor): +class WebDriverCrashtestExecutor(CrashtestExecutor, TestDriverExecutorMixin): protocol_cls = WebDriverProtocol + supports_testdriver = True def __init__(self, logger, browser, server_config, timeout_multiplier=1, screenshot_cache=None, close_after_done=True, @@ -1180,7 +1185,8 @@ def __init__(self, logger, browser, server_config, timeout_multiplier=1, capabilities=capabilities) with open(os.path.join(here, "test-wait.js")) as f: - self.wait_script = f.read() % {"classname": "test-wait"} + wait_script = f.read() % {"classname": "test-wait"} + TestDriverExecutorMixin.__init__(self, wait_script) def do_test(self, test): timeout = (test.timeout * self.timeout_multiplier if self.debug_info is None @@ -1199,8 +1205,9 @@ def do_test(self, test): return (test.make_result(*data), []) def do_crashtest(self, protocol, url, timeout): - protocol.base.load(url) - protocol.base.execute_script(self.wait_script, asynchronous=True) + # There's nothing we want from the "complete" message, so discard the + # return value. + self.run_testdriver(protocol, url, timeout) result = {"status": "PASS", "message": None} if (leak_part := getattr(protocol, "leak", None)) and (counters := leak_part.check()): result["extra"] = {"leak_counters": counters} diff --git a/tools/wptrunner/wptrunner/executors/message-queue.js b/tools/wptrunner/wptrunner/executors/message-queue.js index bbad17180e0a82..c79b96aee29ec7 100644 --- a/tools/wptrunner/wptrunner/executors/message-queue.js +++ b/tools/wptrunner/wptrunner/executors/message-queue.js @@ -54,15 +54,19 @@ 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); + if (tests && 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); + } else { + // Non-testharness test. + payload = []; + } break; case "action": payload = data; diff --git a/tools/wptrunner/wptrunner/executors/test-wait.js b/tools/wptrunner/wptrunner/executors/test-wait.js index ad08ad7d76fb02..fca6f8df3b856e 100644 --- a/tools/wptrunner/wptrunner/executors/test-wait.js +++ b/tools/wptrunner/wptrunner/executors/test-wait.js @@ -1,4 +1,6 @@ -var callback = arguments[arguments.length - 1]; +const initialized = !!window.__wptrunner_url; +window.__wptrunner_testdriver_callback = arguments[arguments.length - 1]; +window.__wptrunner_url = arguments.length > 1 ? arguments[0] : location.href; var observer = null; var root = document.documentElement; @@ -13,7 +15,6 @@ function wait_load() { } } - function wait_paints() { // As of 2017-04-05, the Chromium web browser exhibits a rendering bug // (https://bugs.chromium.org/p/chromium/issues/detail?id=708757) that @@ -32,24 +33,36 @@ function wait_paints() { } function screenshot_if_ready() { - if (root && - root.classList.contains("%(classname)s") && - observer === null) { - observer = new MutationObserver(wait_paints); - observer.observe(root, {attributes: true}); - var event = new Event("TestRendered", {bubbles: true}); - root.dispatchEvent(event); + if (root && root.classList.contains("%(classname)s")) { + if (observer === null) { + observer = new MutationObserver(wait_paints); + observer.observe(root, {attributes: true}); + var event = new Event("TestRendered", {bubbles: true}); + root.dispatchEvent(event); + } return; } if (observer !== null) { observer.disconnect(); } - callback(); + if (window.__wptrunner_message_queue) { + __wptrunner_message_queue.push({type: "complete"}); + } else { + // Not using `testdriver.js`, so manually post a raw completion message + // that the executor understands. + __wptrunner_testdriver_callback([__wptrunner_url, "complete", []]); + } } - -if (document.readyState != "complete") { - addEventListener('load', wait_load); -} else { - wait_load(); +// The `initialized` flag ensures only up to one `load` handler or +// `MutationObserver` is ever registered. +if (!initialized) { + if (document.readyState != "complete") { + addEventListener('load', wait_load, { once: true }); + } else { + wait_load(); + } +} +if (window.__wptrunner_process_next_event) { + window.__wptrunner_process_next_event(); } diff --git a/tools/wptrunner/wptrunner/testdriver-extra.js b/tools/wptrunner/wptrunner/testdriver-extra.js index fa1511188046ed..2dd9a70c829895 100644 --- a/tools/wptrunner/wptrunner/testdriver-extra.js +++ b/tools/wptrunner/wptrunner/testdriver-extra.js @@ -58,6 +58,15 @@ event.stopImmediatePropagation(); }); + const root_classes = document.documentElement.classList; + // For non-testharness tests, the presence of `(ref)test-wait` indicates + // it's the "main" browsing context through which testdriver actions are + // routed. Evaluate this eagerly before the test starts and removes these + // classes. + if (root_classes.contains("reftest-wait") || root_classes.contains("test-wait")) { + window.__wptrunner_is_test_context = true; + } + function is_test_context() { return !!window.__wptrunner_is_test_context; } diff --git a/tools/wptrunner/wptrunner/wptrunner.py b/tools/wptrunner/wptrunner/wptrunner.py index 7d26bb76d75c3c..fbaed2e52f78bf 100644 --- a/tools/wptrunner/wptrunner/wptrunner.py +++ b/tools/wptrunner/wptrunner/wptrunner.py @@ -258,24 +258,21 @@ def run_test_iteration(test_status, test_loader, test_queue_builder, logger.test_end(test.id, status="SKIP", subsuite=subsuite_name) test_status.skipped += 1 - if test_type == "testharness": - for test in test_loader.tests[subsuite_name][test_type]: - skip_reason = None - if test.testdriver and not executor_cls.supports_testdriver: - skip_reason = "Executor does not support testdriver.js" - elif test.jsshell and not executor_cls.supports_jsshell: - skip_reason = "Executor does not support jsshell" - if skip_reason: - logger.test_start(test.id, subsuite=subsuite_name) - logger.test_end(test.id, - status="SKIP", - subsuite=subsuite_name, - message=skip_reason) - test_status.skipped += 1 - else: - tests_to_run[(subsuite_name, test_type)].append(test) - else: - tests_to_run[(subsuite_name, test_type)] = test_loader.tests[subsuite_name][test_type] + for test in test_loader.tests[subsuite_name][test_type]: + skip_reason = None + if getattr(test, "testdriver", False) and not executor_cls.supports_testdriver: + skip_reason = "Executor does not support testdriver.js" + elif test_type == "testharness" and test.jsshell and not executor_cls.supports_jsshell: + skip_reason = "Executor does not support jsshell" + if skip_reason: + logger.test_start(test.id, subsuite=subsuite_name) + logger.test_end(test.id, + status="SKIP", + subsuite=subsuite_name, + message=skip_reason) + test_status.skipped += 1 + else: + tests_to_run[(subsuite_name, test_type)].append(test) unexpected_fail_tests = defaultdict(list) unexpected_pass_tests = defaultdict(list) diff --git a/tools/wptrunner/wptrunner/wpttest.py b/tools/wptrunner/wptrunner/wpttest.py index 2e3fd974d4d43e..42214f07e399f4 100644 --- a/tools/wptrunner/wptrunner/wpttest.py +++ b/tools/wptrunner/wptrunner/wpttest.py @@ -535,7 +535,7 @@ class ReftestTest(Test): def __init__(self, url_base, tests_root, url, inherit_metadata, test_metadata, references, timeout=None, path=None, viewport_size=None, dpi=None, fuzzy=None, - protocol="http", subdomain=False): + protocol="http", subdomain=False, testdriver=False): Test.__init__(self, url_base, tests_root, url, inherit_metadata, test_metadata, timeout, path, protocol, subdomain) @@ -546,6 +546,7 @@ def __init__(self, url_base, tests_root, url, inherit_metadata, test_metadata, r self.references = references self.viewport_size = self.get_viewport_size(viewport_size) self.dpi = dpi + self.testdriver = testdriver self._fuzzy = fuzzy or {} @classmethod @@ -553,7 +554,8 @@ def cls_kwargs(cls, manifest_test): return {"viewport_size": manifest_test.viewport_size, "dpi": manifest_test.dpi, "protocol": server_protocol(manifest_test), - "fuzzy": manifest_test.fuzzy} + "fuzzy": manifest_test.fuzzy, + "testdriver": bool(getattr(manifest_test, "testdriver", False))} @classmethod def from_manifest(cls, @@ -692,10 +694,10 @@ class PrintReftestTest(ReftestTest): def __init__(self, url_base, tests_root, url, inherit_metadata, test_metadata, references, timeout=None, path=None, viewport_size=None, dpi=None, fuzzy=None, - page_ranges=None, protocol="http", subdomain=False): + page_ranges=None, protocol="http", subdomain=False, testdriver=False): super().__init__(url_base, tests_root, url, inherit_metadata, test_metadata, references, timeout, path, viewport_size, dpi, - fuzzy, protocol, subdomain=subdomain) + fuzzy, protocol, subdomain=subdomain, testdriver=testdriver) self._page_ranges = page_ranges @classmethod @@ -726,6 +728,26 @@ class CrashTest(Test): result_cls = CrashtestResult test_type = "crashtest" + def __init__(self, url_base, tests_root, url, inherit_metadata, test_metadata, + timeout=None, path=None, protocol="http", subdomain=False, testdriver=False): + super().__init__(url_base, tests_root, url, inherit_metadata, test_metadata, + timeout, path, protocol, subdomain=subdomain) + self.testdriver = testdriver + + @classmethod + def from_manifest(cls, manifest_file, manifest_item, inherit_metadata, test_metadata): + timeout = cls.long_timeout if manifest_item.timeout == "long" else cls.default_timeout + return cls(manifest_file.url_base, + manifest_file.tests_root, + manifest_item.url, + inherit_metadata, + test_metadata, + timeout=timeout, + path=os.path.join(manifest_file.tests_root, manifest_item.path), + protocol=server_protocol(manifest_item), + subdomain=manifest_item.subdomain, + testdriver=bool(getattr(manifest_item, "testdriver", False))) + manifest_test_cls = {"reftest": ReftestTest, "print-reftest": PrintReftestTest,