Skip to content

Commit

Permalink
[wptrunner] Run --enable-sanitizer tests with their own executors (#…
Browse files Browse the repository at this point in the history
…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
```
  • Loading branch information
jonathan-j-lee authored Nov 22, 2024
1 parent 0dc40f3 commit d19c517
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 63 deletions.
5 changes: 1 addition & 4 deletions tools/wptrunner/wptrunner/browsers/chrome.py
Original file line number Diff line number Diff line change
Expand Up @@ -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 = {
Expand Down
78 changes: 28 additions & 50 deletions tools/wptrunner/wptrunner/executors/executorchrome.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand Down Expand Up @@ -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
Expand All @@ -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):
Expand Down Expand Up @@ -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
12 changes: 3 additions & 9 deletions tools/wptrunner/wptrunner/executors/executoredge.py
Original file line number Diff line number Diff line change
Expand Up @@ -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


Expand Down

0 comments on commit d19c517

Please sign in to comment.