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

test: fix wpt tests for webstreams #50779

Closed
wants to merge 4 commits into from
Closed

test: fix wpt tests for webstreams #50779

wants to merge 4 commits into from

Conversation

MattiasBuelens
Copy link
Contributor

@MattiasBuelens MattiasBuelens commented Nov 18, 2023

There were some expected WPT test failures for Web Streams that did not have a proper explanations as to why they were failing. This tries to improve that:

  • The tests for ReadableStream with the new "owning" type have already landed in upstream WPT, but the spec change hasn't landed yet (Add support for ReadableStream "owning" type whatwg/streams#1271). So this is indeed an expected failure until we implement that spec change.
  • Some of the IDL harness tests were failing because the new ReadableStream.prototype.values and ReadableStream.from methods were not enumerable. So I fixed that, and now all those tests are passing again. 🙂
  • The piping/general-addition.any.js test was added to WPT as something that we'd like to have, but it's still blocked on Can pipeTo() be synchronous? whatwg/streams#1243. So we should ignore this test failure for now.

@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. web streams labels Nov 18, 2023
test/wpt/status/streams.json Outdated Show resolved Hide resolved
test/wpt/status/streams.json Outdated Show resolved Hide resolved
@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. labels Nov 18, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 18, 2023
@nodejs-github-bot
Copy link
Collaborator

@MattiasBuelens
Copy link
Contributor Author

This "expected" test failure also concerns me:

"transferable/transform-stream-members.any.js": {
"fail": {
"expected": [
"Transferring [object TransformStream],[object ReadableStream] should fail",
"Transferring [object TransformStream],[object WritableStream] should fail"
]
}
},

For some reason, transferring [t, t.readable] and [t, t.writable] does not throw a DataCloneError. However, transferring them in the other order (i.e. [t.readable, t] and [t.writable, t]) does correctly throw. 🤔

I haven't figured out why this happens yet. But when I do, you can expect another PR. 😁

@panva panva added the commit-queue Add this label to land a pull request using GitHub Actions. label Nov 18, 2023
@panva
Copy link
Member

panva commented Nov 19, 2023

As you resolve the conflict, would you mind changing the commit message to be the stream subsystem and message related to the values being enumerable?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@panva panva removed author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. labels Nov 19, 2023
@MattiasBuelens
Copy link
Contributor Author

MattiasBuelens commented Nov 20, 2023

Rebased. I also fixed the enumerability of ReadableStream.from().

@panva Do the commit messages look okay now?

@panva panva added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. request-ci Add this label to start a Jenkins CI on a PR. labels Nov 20, 2023
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Nov 20, 2023
@nodejs-github-bot
Copy link
Collaborator

lucshi pushed a commit to lucshi/node that referenced this pull request Nov 27, 2023
PR-URL: nodejs#50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
lucshi pushed a commit to lucshi/node that referenced this pull request Nov 27, 2023
PR-URL: nodejs#50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@RafaelGSS RafaelGSS mentioned this pull request Nov 28, 2023
RafaelGSS pushed a commit that referenced this pull request Nov 29, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 29, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 29, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 29, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 30, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 30, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 30, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
RafaelGSS pushed a commit that referenced this pull request Nov 30, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 11, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
@UlisesGascon UlisesGascon mentioned this pull request Dec 12, 2023
UlisesGascon pushed a commit that referenced this pull request Dec 12, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 12, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 13, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 13, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 15, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Dec 19, 2023
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
UlisesGascon pushed a commit that referenced this pull request Jan 9, 2024
PR-URL: #50779
Reviewed-By: Filip Skokan <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Benjamin Gruenbaum <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-rebase Add this label to allow the Commit Queue to land a PR in several commits. needs-ci PRs that need a full CI run. web streams
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants