wpt
wpt copied to clipboard
Remove import assertions tests
All the removes tests have an equivalent test using import attributes (with):
with keyword |
assert keyword |
|---|---|
./import-attributes |
./import-assertions |
./json-module |
./json-module-assertions |
./css-module |
./css-module-assertions |
The assert syntax is only supported by Chrome, which:
- is in the process of removing it (blink-dev email, chromium patch, v8 patch)
- also supports
with, so removingasserttests doesn't affect its coverage
Safari only supports with, and Firefox supports neither but is implementing with.
Given that the set of platforms supporting assert is a subset of those that support with, having both syntax variants doesn't help with coverage. The difference between with and assert is purely syntactical (it's like testing fetch("http://example.com") and fetch(`http://example.com`)). test262 still tests both syntaxes, given that there are some parsing differences between them.
I hope that eventually we can add a historical test for the assert syntax (i.e., to pass you need to not support it).
This is a good idea -- I'll submit one as soon as the proposal actually gets to stage 4 :)
Were these directories copied and edited, or how can we be confident the coverage is the same? Have there been any changes to the directories since that copying?
They were copied and edited in https://github.com/web-platform-tests/wpt/commit/b62c015e63c9bca18ab31c6739d1e0588b5bd299 (replacing assert with with).
The other commits to any of those folders (both with and assert versions) were:
- https://github.com/web-platform-tests/wpt/commit/577d22542f1321249653d695626f05aa42320ff7, https://github.com/web-platform-tests/wpt/commit/ac7ea8fb67727236ae4ddf2ec2d4922c47287b76, and https://github.com/web-platform-tests/wpt/commit/6593bded6f610c2616e53075d42b51d9566a7f54, which fixed some changes I missed while copy-pasting the tests
- https://github.com/web-platform-tests/wpt/commit/bcae71ef7480ab3ac284b22f7c3768ff366f9136 and https://github.com/web-platform-tests/wpt/commit/e3365d8440796f310ed38d4de3d4789fb611bb96, applied to both the
withandassertvariants
I can reproduce the CI error locally, but I'm also getting it on master
Thanks @nicolo-ribaudo, it sounds like things are in sync then.
@syg would removing these tests be a problem for Chromium/V8? Do we depend on them to avoid regressions?
@syg would removing these tests be a problem for Chromium/V8? Do we depend on them to avoid regressions?
@foolip Removing it is premature IMO. We've agreed to deprecate and remove support for import assertions in Chrome M126. What I'm doing now is to land CLs that changes the import assertions flag from default enabled to default disabled, but not removing code and tests until M126 hits stable for a milestone or two and we get the final evidence that it is indeed a web compatible change.
So my planned timeline is:
- In M126, land V8 and chromium CLs that changes
--harmony-import-assertionsto default disabled without removing code and tests. Move the current import assertion tests to a virtual test suite that passes the flag explicitly. - Wait for M126 to hit stable for a milestone or two.
- If there are no web incompatibility bugs, remove code and tests.
- Else, assess severity of breakage, possibly flipping flag back on and go back to TC39.
Marking this as draft for now, I'll come back in a couple months :)
assert has been fully removed in the July 2024 TC39 meeting.
From the mutation event removal I know that Chrome M126 is in the past, but maybe @syg could explicitly confirm he's okay with this?
Yep, confirm that this is good to land. After this V8 will remove code for supporting assert.