Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove tests that rely on undefined testing APIs #172

Open
gsnedders opened this issue Oct 27, 2023 · 5 comments
Open

Remove tests that rely on undefined testing APIs #172

gsnedders opened this issue Oct 27, 2023 · 5 comments

Comments

@gsnedders
Copy link
Member

WPT contains a variety of tests that rely on MojoJS to run in Chromium-based browsers.

Chromium's own WPT docs say:

Some specs may define testing APIs (e.g. WebUSB), which may be polyfilled with internal API like MojoJS. MojoJS is only allowed in WPT for this purpose. Please reach out to blink-dev@chromium.org before following the process below for adding a new test-only API: [.…]

However, we have many cases where things are "polyfilled" via MojoJS where the spec doesn't define the testing API, and nor has there been any attempt within WPT to document what the testing API the tests expect is. (Furthermore, https://groups.google.com/a/chromium.org/g/blink-dev/search?q=mojojs makes it look like basically nobody has ever followed those steps, as nobody has done the "reach out" step.)

The tests are, thus, practically Chromium-specific, short of someone reverse-engineering what the testing API is.

For example, the Shape Detection tests that were added to WPT in web-platform-tests/wpt@dfc0080 rely on /shape-detection/resources/shapedetection-helpers.js to load testing APIs, except nowhere in https://wicg.github.io/shape-detection-api/ (or any other document) are FaceDetectionTest, BarcodeDetectionTest, or TextDetectionTest actually defined.

Notwithstanding questions of whether any such testing API would ship in the browser, this means wpt.fyi will forever show Safari as failing many of the shape detection tests, even if the feature flag were enabled by default, unless we invest in reverse-engineering Chromium's testing API. Part of the very goal of standards and WPT is to avoid vendors having to invest in reverse-engineering one-another.

In extreme, /serial/README.md even notes:

there is no separate specification of the API other than the tests themselves and the Chromium implementation

WPT cannot be a dumping ground for inherently vendor-specific tests, written with little-to-no regard for other vendors using them, as this undermines the very premise of it being a shared test suite. It is unreasonable and unhelpful for Chromium engineers to export tests which other vendors must reverse-engineer the testing API to gain any benefit from. Chromium can maintain these tests in their own, non-exported tree, and do so until such point as they define what the testing APIs actually are.

As such, we should remove all tests that rely on undefined testing APIs from WPT.

@gsnedders
Copy link
Member Author

FWIW:

gsnedders@gsnedders-marsha web-platform-tests % rg -F resources/chromium --files-with-matches | sort                                                        
battery-status/resources/battery-status-helpers.js
bluetooth/README.md
bluetooth/resources/bluetooth-test.js
compute-pressure/resources/pressure-helpers.js
contacts/resources/helpers.js
content-index/resources.js
generic-sensor/README.md
html/semantics/links/downloading-resources/header-origin-no-referrer-when-downgrade.html
html/semantics/links/downloading-resources/header-origin-no-referrer.html
html/semantics/links/downloading-resources/header-origin-origin-when-cross-origin.html
html/semantics/links/downloading-resources/header-origin-origin.html
html/semantics/links/downloading-resources/header-origin-same-origin.html
html/semantics/links/downloading-resources/header-origin-strict-origin-when-cross-origin.html
html/semantics/links/downloading-resources/header-origin-strict-origin.html
html/semantics/links/downloading-resources/header-origin-unsafe-url.html
html/semantics/links/downloading-resources/header-origin.html
html/semantics/links/downloading-resources/header-referrer-no-referrer-when-downgrade.html
html/semantics/links/downloading-resources/header-referrer-no-referrer.html
html/semantics/links/downloading-resources/header-referrer-origin-when-cross-origin.html
html/semantics/links/downloading-resources/header-referrer-origin.html
html/semantics/links/downloading-resources/header-referrer-same-origin.html
html/semantics/links/downloading-resources/header-referrer-strict-origin-when-cross-origin.html
html/semantics/links/downloading-resources/header-referrer-strict-origin.html
html/semantics/links/downloading-resources/header-referrer-unsafe-url.html
html/semantics/links/downloading-resources/header-referrer.html
idle-detection/resources/idle-detection-helper.js
lint.ignore
managed/resources/managed-configuration-helper.js
mediacapture-image/resources/imagecapture-helpers.js
orientation-event/README.md
orientation-event/resources/orientation-event-helpers.js
serial/README.md
serial/resources/automation.js
shape-detection/README.md
shape-detection/resources/shapedetection-helpers.js
subapps/resources/subapps-helpers.js
web-nfc/README.md
web-nfc/resources/nfc-helpers.js
webhid/resources/automation.js
webusb/README.md
webusb/resources/usb-helpers.js
webxr/resources/webxr_util.js

@jgraham
Copy link
Contributor

jgraham commented Oct 27, 2023

I think at least some of those are things where the spec does define the testing API.

But I agree that tests which depend on undefined browser internal APIs aren't a good fit for wpt, and vendors should find a way to run those outside of the shared suite (note that doesn't necessarily mean they can't be written to use the wpt harness and infrastructure; we have gecko-specific tests that may use gecko's internal SpecialPowers API as a separate set of web-platform-tests in mozilla-central, and these are not synced with upstream).

@gsnedders
Copy link
Member Author

I think at least some of those are things where the spec does define the testing API.

Yes, that list of files certainly is superset of what is implicated in this issue (e.g., WebUSB and WebXR do define testing APIs, and are thus fine for WPT), but they are probably what needs to be audited.

@foolip
Copy link
Member

foolip commented Mar 15, 2024

Has this been discussed somewhere, or are there proposed next steps? I don't think anyone who has written any of these tests is aware of this issue, and I came across it again today while looking at compute pressure, which jogged my memory.

cc @past

@past
Copy link
Member

past commented Apr 22, 2024

Sorry for the delay in responding here, I just saw this ping. I had reached out to the test owners and they agreed to move those tests to wpt_internal in the near term (2024) and create test APIs in the longer term (2025+).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants