From d19c517ad41e04bb2c02c351a8056d7129eead2b Mon Sep 17 00:00:00 2001 From: Jonathan Lee Date: Fri, 22 Nov 2024 14:04:26 -0800 Subject: [PATCH] [wptrunner] Run `--enable-sanitizer` tests with their own executors (#49235) Running non-crashtests with the crashtest executor likely doesn't provide high-fidelity coverage because the test completes on the `load` event by default. This means that code paths normally exercised after the `load` event aren't actually run under sanitization. Also, some tests require testdriver, which the crashtest executor doesn't implement yet, to progress. Now, simply run each test with its corresponding type's executor and suppress functional failures (i.e., not timeout or crash) at the end. This also simplifies the result coercion, which didn't interact correctly with #47903. Tested locally: ```sh ./wpt run -y chrome infrastructure --metadata infrastructure/metadata --enable-sanitizer ``` --- tools/wptrunner/wptrunner/browsers/chrome.py | 5 +- .../wptrunner/executors/executorchrome.py | 78 +++++++------------ .../wptrunner/executors/executoredge.py | 12 +-- 3 files changed, 32 insertions(+), 63 deletions(-) diff --git a/tools/wptrunner/wptrunner/browsers/chrome.py b/tools/wptrunner/wptrunner/browsers/chrome.py index b39b8deb76c958..73b3b5344b8125 100644 --- a/tools/wptrunner/wptrunner/browsers/chrome.py +++ b/tools/wptrunner/wptrunner/browsers/chrome.py @@ -67,13 +67,10 @@ def browser_kwargs(logger, test_type, run_info_data, config, **kwargs): def executor_kwargs(logger, test_type, test_environment, run_info_data, subsuite, **kwargs): - sanitizer_enabled = kwargs.get("sanitizer_enabled") - if sanitizer_enabled: - test_type = "crashtest" executor_kwargs = base_executor_kwargs(test_type, test_environment, run_info_data, subsuite, **kwargs) executor_kwargs["close_after_done"] = True - executor_kwargs["sanitizer_enabled"] = sanitizer_enabled + executor_kwargs["sanitizer_enabled"] = kwargs.get("sanitizer_enabled", False) executor_kwargs["reuse_window"] = kwargs.get("reuse_window", False) capabilities = { diff --git a/tools/wptrunner/wptrunner/executors/executorchrome.py b/tools/wptrunner/wptrunner/executors/executorchrome.py index fa593ce4023106..e92d45094d1571 100644 --- a/tools/wptrunner/wptrunner/executors/executorchrome.py +++ b/tools/wptrunner/wptrunner/executors/executorchrome.py @@ -6,14 +6,10 @@ import re import time import uuid -from typing import Mapping, MutableMapping, Type +from typing import Mapping, MutableMapping from webdriver import error -from .base import ( - CrashtestExecutor, - TestharnessExecutor, -) from .executorwebdriver import ( WebDriverBaseProtocolPart, WebDriverCrashtestExecutor, @@ -29,41 +25,6 @@ here = os.path.dirname(__file__) -def make_sanitizer_mixin(crashtest_executor_cls: Type[CrashtestExecutor]): # type: ignore[no-untyped-def] - class SanitizerMixin: - def __new__(cls, logger, browser, **kwargs): - # Overriding `__new__` is the least worst way we can force tests to run - # as crashtests at runtime while still supporting: - # * Class attributes (e.g., `extra_timeout`) - # * Pickleability for `multiprocessing` transport - # * The `__wptrunner__` product interface - # - # These requirements rule out approaches with `functools.partial(...)` - # or global variables. - if kwargs.get("sanitizer_enabled"): - executor = crashtest_executor_cls(logger, browser, **kwargs) - - def convert_from_crashtest_result(test, result): - if issubclass(cls, TestharnessExecutor): - status = result["status"] - if status == "PASS": - status = "OK" - harness_result = test.make_result(status, result["message"]) - # Don't report subtests. - return harness_result, [] - # `crashtest` statuses are a subset of `(print-)reftest` - # ones, so no extra conversion necessary. - return cls.convert_result(executor, test, result) - - executor.convert_result = convert_from_crashtest_result - return executor - return super().__new__(cls) - return SanitizerMixin - - -_SanitizerMixin = make_sanitizer_mixin(WebDriverCrashtestExecutor) - - class ChromeDriverBaseProtocolPart(WebDriverBaseProtocolPart): def create_window(self, type="tab", **kwargs): try: @@ -211,7 +172,7 @@ def __init__(self, executor, browser, capabilities, **kwargs): super().__init__(executor, browser, capabilities, **kwargs) -def _evaluate_leaks(executor_cls): +def _evaluate_sanitized_result(executor_cls): if hasattr(executor_cls, "base_convert_result"): # Don't wrap more than once, which can cause unbounded recursion. return executor_cls @@ -226,28 +187,42 @@ def convert_result(self, test, result, **kwargs): test_result.extra, test_result.stack, test_result.known_intermittent) + if self.sanitizer_enabled: + # Coerce functional failures to OK/PASS, and discard any subtest results. + if test_result.status in {"ERROR", "FAIL", "INTERNAL-ERROR", "PRECONDITION_FAILED"}: + test_result.status = test_result.default_expected + return test_result, [] return test_result, subtest_results executor_cls.convert_result = convert_result return executor_cls -@_evaluate_leaks +@_evaluate_sanitized_result class ChromeDriverCrashTestExecutor(WebDriverCrashtestExecutor): protocol_cls = ChromeDriverProtocol + def __init__(self, *args, sanitizer_enabled=False, **kwargs): + super().__init__(*args, **kwargs) + self.sanitizer_enabled = sanitizer_enabled + -@_evaluate_leaks -class ChromeDriverRefTestExecutor(WebDriverRefTestExecutor, _SanitizerMixin): # type: ignore +@_evaluate_sanitized_result +class ChromeDriverRefTestExecutor(WebDriverRefTestExecutor): protocol_cls = ChromeDriverProtocol + def __init__(self, *args, sanitizer_enabled=False, **kwargs): + super().__init__(*args, **kwargs) + self.sanitizer_enabled = sanitizer_enabled -@_evaluate_leaks -class ChromeDriverTestharnessExecutor(WebDriverTestharnessExecutor, _SanitizerMixin): # type: ignore + +@_evaluate_sanitized_result +class ChromeDriverTestharnessExecutor(WebDriverTestharnessExecutor): protocol_cls = ChromeDriverProtocol - def __init__(self, *args, reuse_window=False, **kwargs): + def __init__(self, *args, sanitizer_enabled=False, reuse_window=False, **kwargs): super().__init__(*args, **kwargs) + self.sanitizer_enabled = sanitizer_enabled self.reuse_window = reuse_window def get_or_create_test_window(self, protocol): @@ -284,7 +259,10 @@ def _get_next_message_classic(self, protocol, url, test_window): raise -@_evaluate_leaks -class ChromeDriverPrintRefTestExecutor(WebDriverPrintRefTestExecutor, - _SanitizerMixin): # type: ignore +@_evaluate_sanitized_result +class ChromeDriverPrintRefTestExecutor(WebDriverPrintRefTestExecutor): protocol_cls = ChromeDriverProtocol + + def __init__(self, *args, sanitizer_enabled=False, **kwargs): + super().__init__(*args, **kwargs) + self.sanitizer_enabled = sanitizer_enabled diff --git a/tools/wptrunner/wptrunner/executors/executoredge.py b/tools/wptrunner/wptrunner/executors/executoredge.py index 95b40bd8db14f6..2ddfdb3ad8f47a 100644 --- a/tools/wptrunner/wptrunner/executors/executoredge.py +++ b/tools/wptrunner/wptrunner/executors/executoredge.py @@ -3,31 +3,25 @@ import os from .executorwebdriver import ( - WebDriverCrashtestExecutor, WebDriverRefTestExecutor, WebDriverRun, WebDriverTestharnessExecutor, ) -from .executorchrome import ( - ChromeDriverProtocol, - make_sanitizer_mixin, -) +from .executorchrome import ChromeDriverProtocol here = os.path.dirname(__file__) -_SanitizerMixin = make_sanitizer_mixin(WebDriverCrashtestExecutor) - class EdgeDriverProtocol(ChromeDriverProtocol): vendor_prefix = "ms" -class EdgeDriverRefTestExecutor(WebDriverRefTestExecutor, _SanitizerMixin): # type: ignore +class EdgeDriverRefTestExecutor(WebDriverRefTestExecutor): protocol_cls = EdgeDriverProtocol -class EdgeDriverTestharnessExecutor(WebDriverTestharnessExecutor, _SanitizerMixin): # type: ignore +class EdgeDriverTestharnessExecutor(WebDriverTestharnessExecutor): protocol_cls = EdgeDriverProtocol