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.

Sign up for GitHub

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

Jump to bottom

test: update wpt #54468

Closed
wants to merge 2 commits into from
Closed

test: update wpt #54468

wants to merge 2 commits into from

Conversation

avivkeller
Copy link
Member

@avivkeller avivkeller commented Aug 20, 2024 •
edited
Loading

This PR updates the WPT files.

Note the following behavioral differences between Node.js and WPT:

test-structured-clone.js

Comparison: https://wpt.fyi/results/html/webappapis/structured-clone/structured-clone.any.html
See: #55120

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/web-standards

@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Aug 20, 2024
@avivkeller avivkeller added web-standards Issues and PRs related to Web APIs commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed test Issues and PRs related to the tests. needs-ci PRs that need a full CI run. labels Aug 20, 2024
@avivkeller avivkeller added the wip Issues and PRs that are still a work in progress. label Aug 20, 2024
@avivkeller avivkeller force-pushed the wpt branch 3 times, most recently from 1608018 to dfc14dd Compare August 22, 2024 18:51
This was referenced Aug 23, 2024
Merged
Open
@avivkeller avivkeller force-pushed the wpt branch 2 times, most recently from 241ee32 to 4d67590 Compare August 26, 2024 20:53
panva added a commit to panva/node that referenced this pull request Aug 27, 2024
@panva
test,crypto: update WebCryptoAPI WPT
e36c683 
Refs: nodejs#54572
Refs: nodejs#54468
@panva panva mentioned this pull request Aug 27, 2024
Merged
@avivkeller avivkeller added the blocked PRs that are blocked by other issues or PRs. label Aug 27, 2024
@avivkeller
Copy link
Member Author

avivkeller commented Aug 27, 2024 •
edited
Loading

This PR is a draft, and is no where near completion, but for record-keeping, this can't land until #54468 #54593 does.

@panva
Copy link
Member

panva commented Aug 27, 2024

This PR is a draft, and is no where near completion, but for record-keeping, this can't land until #54468 does.

Not necessarily, just stay clear of updating WebCryptoAPI as that will be done separately in #54593

@avivkeller avivkeller removed the blocked PRs that are blocked by other issues or PRs. label Aug 27, 2024
@avivkeller
Copy link
Member Author

Not necessarily, just stay clear of updating WebCryptoAPI as that will be done separately in #54593

Right, haha. I just to remove the WebCryptoAPI changes, and then fix the other failures...

@avivkeller avivkeller mentioned this pull request Aug 28, 2024
Closed
@avivkeller avivkeller force-pushed the wpt branch 2 times, most recently from 1d013e6 to cfea3eb Compare August 28, 2024 01:07
@avivkeller avivkeller mentioned this pull request Aug 28, 2024
Closed
nodejs-github-bot pushed a commit that referenced this pull request Aug 29, 2024
@panva
test,crypto: update WebCryptoAPI WPT
fc02b88 
Refs: #54572
Refs: #54468
PR-URL: #54593
Refs: #54572
Refs: #54468
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@panva @RafaelGSS
test,crypto: update WebCryptoAPI WPT
67c598a 
Refs: #54572
Refs: #54468
PR-URL: #54593
Refs: #54572
Refs: #54468
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@panva @RafaelGSS
test,crypto: update WebCryptoAPI WPT
c2d7c67 
Refs: #54572
Refs: #54468
PR-URL: #54593
Refs: #54572
Refs: #54468
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Aug 30, 2024
@panva @RafaelGSS
test,crypto: update WebCryptoAPI WPT
5ee234a 
Refs: #54572
Refs: #54468
PR-URL: #54593
Refs: #54572
Refs: #54468
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
sendoru pushed a commit to sendoru/node that referenced this pull request Sep 1, 2024
@panva @sendoru
test,crypto: update WebCryptoAPI WPT
2b43487 
Refs: nodejs#54572
Refs: nodejs#54468
PR-URL: nodejs#54593
Refs: nodejs#54572
Refs: nodejs#54468
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
@jazelly jazelly mentioned this pull request Sep 7, 2024
Closed
@avivkeller avivkeller force-pushed the wpt branch 2 times, most recently from 9d1d270 to f03c104 Compare September 7, 2024 23:13
@avivkeller avivkeller removed the commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. label Sep 7, 2024
@jasnell jasnell added the request-ci Add this label to start a Jenkins CI on a PR. label Sep 7, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Sep 7, 2024
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/62129/

@avivkeller avivkeller force-pushed the wpt branch 5 times, most recently from e1a5f83 to fe20ad8 Compare September 25, 2024 20:02
@avivkeller avivkeller added test Issues and PRs related to the tests. and removed wip Issues and PRs that are still a work in progress. labels Sep 25, 2024
@avivkeller avivkeller marked this pull request as ready for review September 25, 2024 20:08
avivkeller
avivkeller commented Sep 25, 2024
View reviewed changes
test/wpt/status/html/webappapis/structured-clone.json Outdated
Comment on lines 4 to 12
"note12": "(1, 2) File is not (yet) transferrable. (3) SharedArrayBuffer does not extend ArrayBuffer",
"note3": "(3) SharedArrayBuffer does not extend ArrayBuffer",
"note4": "(4) https://github.com/nodejs/node/issues/55120",
"expected": [
"File basic",
"A subclass instance will deserialize as its closest serializable superclass",
"Growable SharedArrayBuffer",
"Serializing a non-serializable platform object fails"
]
Copy link
Member Author

@avivkeller avivkeller Sep 25, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review note:
File basic and A subclass instance will deserialize as its closest serializable superclass are being monitored at #47612

Growable SharedArrayBuffer is failing WPT because the test expects that SharedArrayBuffer extends ArrayBuffer, but it does not (AFAICT in Chromium and Node.js). I've manually tested that the behavior the WPT expects (regarding the clone, not the extension) passes on Node.js.

Serializing a non-serializable platform object fails is being monitored at #55120

@avivkeller avivkeller added the needs-ci PRs that need a full CI run. label Sep 25, 2024
@codecov Codecov
Copy link

codecov bot commented Sep 25, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 88.24%. Comparing base (bdddc04) to head (341b6c2).
Report is 60 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #54468      +/-   ##
==========================================
- Coverage   88.25%   88.24%   -0.01%     
==========================================
  Files         651      651              
  Lines      183856   183856              
  Branches    35856    35853       -3     
==========================================
- Hits       162255   162252       -3     
+ Misses      14895    14883      -12     
- Partials     6706     6721      +15

see 24 files with indirect coverage changes

@avivkeller
Copy link
Member Author

I'm closing this in favor of opening each update seperately, so some of them can be backported (if needed?)

@avivkeller avivkeller closed this Sep 30, 2024
louwers pushed a commit to louwers/node that referenced this pull request Nov 2, 2024
@panva @louwers
test,crypto: update WebCryptoAPI WPT
2359a6c 
Refs: nodejs#54572
Refs: nodejs#54468
PR-URL: nodejs#54593
Refs: nodejs#54572
Refs: nodejs#54468
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@panva panva panva left review comments

Assignees
No one assigned
Labels
needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. web-standards Issues and PRs related to Web APIs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@avivkeller @nodejs-github-bot @panva @jasnell