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

RedYetiDev
Copy link
Member

@RedYetiDev RedYetiDev 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
@RedYetiDev RedYetiDev 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
@RedYetiDev RedYetiDev added the wip Issues and PRs that are still a work in progress. label Aug 20, 2024
This was referenced Aug 23, 2024
Open
Open
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
@RedYetiDev RedYetiDev added the blocked PRs that are blocked by other issues or PRs. label Aug 27, 2024
@RedYetiDev
Copy link
Member Author

RedYetiDev 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

@RedYetiDev RedYetiDev removed the blocked PRs that are blocked by other issues or PRs. label Aug 27, 2024
@RedYetiDev
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...

@RedYetiDev RedYetiDev mentioned this pull request Aug 28, 2024
Closed
@RedYetiDev RedYetiDev 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 <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@RedYetiDev RedYetiDev mentioned this pull request Aug 29, 2024
Merged
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 <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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 <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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 <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
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 <jasnell@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Michaël Zasso <targos@protonmail.com>
Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com>
Reviewed-By: Luigi Pinca <luigipinca@gmail.com>
@jazelly jazelly mentioned this pull request Sep 7, 2024
Closed
@RedYetiDev RedYetiDev 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/

@RedYetiDev RedYetiDev 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
@RedYetiDev RedYetiDev marked this pull request as ready for review September 25, 2024 20:08
RedYetiDev
RedYetiDev 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

@RedYetiDev RedYetiDev 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

@RedYetiDev RedYetiDev 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

@RedYetiDev
Copy link
Member Author

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

@RedYetiDev RedYetiDev closed this Sep 30, 2024
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
@RedYetiDev @nodejs-github-bot @panva @jasnell