test: deflake stream-readable-to-web test #58948

Ethan-Arrowood · 2025-07-03T16:29:25Z

Ethan-Arrowood · 2025-07-03T16:32:48Z

cc @nodejs/streams

github-actions · 2025-07-03T16:37:32Z

Failed to start CI

   ⚠  No approving reviews found
   ✘  Refusing to run CI on potentially unsafe PR

https://github.com/nodejs/node/actions/runs/16055944616

Ethan-Arrowood · 2025-07-03T16:44:54Z

I guess I appreciate the CI not running without a review lol

mcollina

lgtm

nodejs-github-bot · 2025-07-03T16:58:51Z

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

test/parallel/test-stream-readable-to-web.mjs

nodejs-github-bot · 2025-07-03T17:09:07Z

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

codecov · 2025-07-03T18:27:58Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.06%. Comparing base (1c4fe6d) to head (5b5f9dc).
Report is 14 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #58948      +/-   ##
==========================================
- Coverage   90.08%   90.06%   -0.02%     
==========================================
  Files         641      641              
  Lines      188718   188718              
  Branches    37022    37025       +3     
==========================================
- Hits       170000   169965      -35     
- Misses      11417    11462      +45     
+ Partials     7301     7291      -10

see 33 files with indirect coverage changes

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Ethan-Arrowood · 2025-07-03T18:59:32Z

I think somethings wrong with how I'm checking for crypto support

https://ci.nodejs.org/job/node-test-commit-linux-containered/51526/nodes=ubuntu2204_sharedlibs_withoutssl_x64/testReport/junit/(root)/parallel/test_stream_readable_to_web/

Will fix

targos · 2025-07-03T19:27:17Z

@Ethan-Arrowood The problem is that all static imports will be evaluated before your condition (it doesn't matter where they are in the file).

Ethan-Arrowood · 2025-07-03T19:31:14Z

I had a feeling that was it. Doesn't seem like we have any other occurrences of this in our tests. I could switch back to CJS if that is preferred, or I think a dynamic import is okay?

lpinca · 2025-07-03T19:34:14Z

Going back to cjs would make the diff useful. It currently looks like a new test.

Ethan-Arrowood · 2025-07-03T19:36:02Z

My preference is to move towards using ESM but if anyone else feels strongly it should stay as CJS I will happily switch back. Just let me know if that is what is wanted

nodejs-github-bot · 2025-07-03T19:42:29Z

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

Ethan-Arrowood · 2025-07-03T21:26:14Z

node:test is not needed, will remove.
You're also right - i think an empty buffer will be okay too. Modifying now
I'll work on timing so that the read happens after some delay. But I do believe the flakiness had to do with the plethora of timers so I was trying to reduce that as much as possible.

joyeecheung · 2025-07-03T21:57:31Z

test/parallel/test-stream-readable-to-web.mjs

+    checkMemoryUsage();
+    // eslint-disable-next-line no-unused-vars
+    for await (const _ of randomWebStream) {
+      // Yield event loop to allow garbage collection


FWIW, this behavior likely relies on the bug described in #54918 - the event loop is not supposed to block on just any garbage collection, which can cause a deadlock, and in the future it might go away (if someone manages to eradicate the deadlock by rewriting the task runner).

Alternatively if there's a specific leaky object with a known class that this tries to test, checkIfCollectableByCounting() from test/common/gc.js might be less flaky. Though I can't really tell whether that's applicable from a look of this test.

Good to know. I agree that issue seems related here; particularly with the previous heavy use of timers. I believe that the Readable / GC is doing what they are supposed to regarding memory management.

nodejs-github-bot · 2025-07-07T17:35:39Z

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

Ethan-Arrowood · 2025-07-08T17:04:47Z

Please rereview when you get a chance; I rebased on main so that CI might play nicer. I was only hitting other flaky test haha

Co-authored-by: Luigi Pinca <[email protected]>

nodejs-github-bot · 2025-07-08T19:35:38Z

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

nodejs-github-bot · 2025-07-09T22:37:49Z

Commit Queue failed

- Loading data for nodejs/node/pull/58948
✔  Done loading data for nodejs/node/pull/58948
----------------------------------- PR info ------------------------------------
Title      test: deflake stream-readable-to-web test (#58948)
Author     Ethan Arrowood <[email protected]> (@Ethan-Arrowood)
Branch     Ethan-Arrowood:deflake-stream-readable-to-web -> nodejs:main
Labels     test, flaky-test, needs-ci
Commits    3
 - test: deflake stream-readable-to-web test
 - test: dynamic import cypto
 - test: simplify but keep accurate
Committers 1
 - Ethan Arrowood <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/58948
Fixes: https://github.com/nodejs/node/issues/58949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Mattias Buelens <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58948
Fixes: https://github.com/nodejs/node/issues/58949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Mattias Buelens <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 03 Jul 2025 16:29:25 GMT
   ✔  Approvals: 5
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/58948#pullrequestreview-2983898238
   ✔  - Mattias Buelens (@MattiasBuelens): https://github.com/nodejs/node/pull/58948#pullrequestreview-2983969659
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/58948#pullrequestreview-2984451374
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58948#pullrequestreview-2985657415
   ✔  - Zeyu "Alex" Yang (@himself65): https://github.com/nodejs/node/pull/58948#pullrequestreview-2989658746
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2025-07-08T19:35:38Z: https://ci.nodejs.org/job/node-test-pull-request/67889/
- Querying data for job/node-test-pull-request/67889/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

https://github.com/nodejs/node/actions/runs/16181664591

nodejs-github-bot · 2025-07-09T22:57:46Z

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

nodejs-github-bot · 2025-07-10T16:56:05Z

Commit Queue failed

- Loading data for nodejs/node/pull/58948
✔  Done loading data for nodejs/node/pull/58948
----------------------------------- PR info ------------------------------------
Title      test: deflake stream-readable-to-web test (#58948)
Author     Ethan Arrowood <[email protected]> (@Ethan-Arrowood)
Branch     Ethan-Arrowood:deflake-stream-readable-to-web -> nodejs:main
Labels     test, flaky-test, needs-ci
Commits    3
 - test: deflake stream-readable-to-web test
 - test: dynamic import cypto
 - test: simplify but keep accurate
Committers 1
 - Ethan Arrowood <[email protected]>
PR-URL: https://github.com/nodejs/node/pull/58948
Fixes: https://github.com/nodejs/node/issues/58949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Mattias Buelens <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/58948
Fixes: https://github.com/nodejs/node/issues/58949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Mattias Buelens <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Thu, 03 Jul 2025 16:29:25 GMT
   ✔  Approvals: 5
   ✔  - Matteo Collina (@mcollina) (TSC): https://github.com/nodejs/node/pull/58948#pullrequestreview-2983898238
   ✔  - Mattias Buelens (@MattiasBuelens): https://github.com/nodejs/node/pull/58948#pullrequestreview-2983969659
   ✔  - Juan José Arboleda (@juanarbol): https://github.com/nodejs/node/pull/58948#pullrequestreview-2984451374
   ✔  - Luigi Pinca (@lpinca): https://github.com/nodejs/node/pull/58948#pullrequestreview-2985657415
   ✔  - Zeyu "Alex" Yang (@himself65): https://github.com/nodejs/node/pull/58948#pullrequestreview-2989658746
   ✔  Last GitHub CI successful
   ℹ  Last Full PR CI on 2025-07-09T22:57:46Z: https://ci.nodejs.org/job/node-test-pull-request/67916/
- Querying data for job/node-test-pull-request/67916/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  No git cherry-pick in progress
   ✔  No git am in progress
   ✔  No git rebase in progress
--------------------------------------------------------------------------------
- Bringing origin/main up to date...
From https://github.com/nodejs/node
 * branch                  main       -> FETCH_HEAD
✔  origin/main is now up-to-date
- Downloading patch for 58948
From https://github.com/nodejs/node
 * branch                  refs/pull/58948/merge -> FETCH_HEAD
✔  Fetched commits as c44fa8d0b65d..5b5f9dc85b63
--------------------------------------------------------------------------------
[main f797193ad8] test: deflake stream-readable-to-web test
 Author: Ethan Arrowood <[email protected]>
 Date: Thu Jul 3 10:27:03 2025 -0600
 2 files changed, 69 insertions(+), 62 deletions(-)
 delete mode 100644 test/parallel/test-stream-readable-to-web.js
 create mode 100644 test/parallel/test-stream-readable-to-web.mjs
[main 1eca5dbc83] test: dynamic import cypto
 Author: Ethan Arrowood <[email protected]>
 Date: Thu Jul 3 13:30:28 2025 -0600
 1 file changed, 54 insertions(+), 48 deletions(-)
[main 15def1f185] test: simplify but keep accurate
 Author: Ethan Arrowood <[email protected]>
 Date: Thu Jul 3 15:43:55 2025 -0600
 1 file changed, 36 insertions(+), 47 deletions(-)
   ✔  Patches applied
There are 3 commits in the PR. Attempting autorebase.
Rebasing (2/6)
Executing: git node land --amend --yes
--------------------------------- New Message ----------------------------------
test: deflake stream-readable-to-web test
Co-authored-by: Luigi Pinca <[email protected]>

PR-URL: #58948

Fixes: #58949

Reviewed-By: Matteo Collina <[email protected]>

Reviewed-By: Mattias Buelens <[email protected]>

Reviewed-By: Juan José Arboleda <[email protected]>

Reviewed-By: Luigi Pinca <[email protected]>

Reviewed-By: Zeyu "Alex" Yang <[email protected]>
[detached HEAD e8edf49e2b] test: deflake stream-readable-to-web test

Author: Ethan Arrowood <[email protected]>

Date: Thu Jul 3 10:27:03 2025 -0600

2 files changed, 69 insertions(+), 62 deletions(-)

delete mode 100644 test/parallel/test-stream-readable-to-web.js

create mode 100644 test/parallel/test-stream-readable-to-web.mjs

Rebasing (3/6)

Rebasing (4/6)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: dynamic import cypto
PR-URL: #58948

Fixes: #58949

Reviewed-By: Matteo Collina <[email protected]>

Reviewed-By: Mattias Buelens <[email protected]>

Reviewed-By: Juan José Arboleda <[email protected]>

Reviewed-By: Luigi Pinca <[email protected]>

Reviewed-By: Zeyu "Alex" Yang <[email protected]>
[detached HEAD 0b2dba40da] test: dynamic import cypto

Author: Ethan Arrowood <[email protected]>

Date: Thu Jul 3 13:30:28 2025 -0600

1 file changed, 54 insertions(+), 48 deletions(-)

Rebasing (5/6)

Rebasing (6/6)

Executing: git node land --amend --yes

--------------------------------- New Message ----------------------------------

test: simplify but keep accurate
PR-URL: #58948

Fixes: #58949

Reviewed-By: Matteo Collina <[email protected]>

Reviewed-By: Mattias Buelens <[email protected]>

Reviewed-By: Juan José Arboleda <[email protected]>

Reviewed-By: Luigi Pinca <[email protected]>

Reviewed-By: Zeyu "Alex" Yang <[email protected]>
[detached HEAD 936c7f6cc9] test: simplify but keep accurate

Author: Ethan Arrowood <[email protected]>

Date: Thu Jul 3 15:43:55 2025 -0600

1 file changed, 36 insertions(+), 47 deletions(-)

Successfully rebased and updated refs/heads/main.
ℹ  Add commit-queue-squash label to land the PR as one commit, or commit-queue-rebase to land as separate commits.

https://github.com/nodejs/node/actions/runs/16201169771

Ethan-Arrowood added the flaky-test Issues and PRs related to the tests with unstable failures on the CI. label Jul 3, 2025

nodejs-github-bot added needs-ci PRs that need a full CI run. test Issues and PRs related to the tests. labels Jul 3, 2025

Ethan-Arrowood added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

github-actions bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 3, 2025

mcollina approved these changes Jul 3, 2025

View reviewed changes

mcollina added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 3, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

lpinca approved these changes Jul 3, 2025

View reviewed changes

test/parallel/test-stream-readable-to-web.mjs Outdated Show resolved Hide resolved

lpinca previously approved these changes Jul 3, 2025

View reviewed changes

mcollina added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

MattiasBuelens approved these changes Jul 3, 2025

View reviewed changes

juanarbol approved these changes Jul 3, 2025

View reviewed changes

Ethan-Arrowood added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 3, 2025

joyeecheung reviewed Jul 3, 2025

View reviewed changes

lpinca approved these changes Jul 4, 2025

View reviewed changes

himself65 approved these changes Jul 5, 2025

View reviewed changes

Ethan-Arrowood added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 7, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 7, 2025

Ethan-Arrowood force-pushed the deflake-stream-readable-to-web branch from f728218 to 4cd524b Compare July 8, 2025 17:03

Ethan-Arrowood and others added 3 commits July 8, 2025 11:05

test: deflake stream-readable-to-web test

6080362

Co-authored-by: Luigi Pinca <[email protected]>

test: dynamic import cypto

3775dc9

test: simplify but keep accurate

5b5f9dc

Ethan-Arrowood force-pushed the deflake-stream-readable-to-web branch from 4cd524b to 5b5f9dc Compare July 8, 2025 17:06

Ethan-Arrowood added the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 8, 2025

Ethan-Arrowood added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 9, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 9, 2025

Ethan-Arrowood added request-ci Add this label to start a Jenkins CI on a PR. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 9, 2025

github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jul 9, 2025

Ethan-Arrowood added the commit-queue Add this label to land a pull request using GitHub Actions. label Jul 10, 2025

nodejs-github-bot added commit-queue-failed An error occurred while landing this pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jul 10, 2025

Ethan-Arrowood added commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. and removed commit-queue-failed An error occurred while landing this pull request using GitHub Actions. labels Jul 10, 2025

Uh oh!

test: deflake stream-readable-to-web test #58948

Are you sure you want to change the base?

test: deflake stream-readable-to-web test #58948

Uh oh!

Conversation

Ethan-Arrowood commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ethan-Arrowood commented Jul 3, 2025

Uh oh!

github-actions bot commented Jul 3, 2025

Uh oh!

Ethan-Arrowood commented Jul 3, 2025

Uh oh!

mcollina left a comment

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

codecov bot commented Jul 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Ethan-Arrowood commented Jul 3, 2025

Uh oh!

targos commented Jul 3, 2025

Uh oh!

Ethan-Arrowood commented Jul 3, 2025

Uh oh!

lpinca commented Jul 3, 2025

Uh oh!

Ethan-Arrowood commented Jul 3, 2025

Uh oh!

nodejs-github-bot commented Jul 3, 2025

Uh oh!

Ethan-Arrowood commented Jul 3, 2025

Uh oh!

joyeecheung Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

Ethan-Arrowood Jul 3, 2025

Choose a reason for hiding this comment

Uh oh!

nodejs-github-bot commented Jul 7, 2025

Uh oh!

Ethan-Arrowood commented Jul 8, 2025

Uh oh!

nodejs-github-bot commented Jul 8, 2025

Uh oh!

nodejs-github-bot commented Jul 9, 2025

Uh oh!

nodejs-github-bot commented Jul 9, 2025

Uh oh!

nodejs-github-bot commented Jul 10, 2025

PR-URL: #58948 Fixes: #58949 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mattias Buelens <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Zeyu "Alex" Yang <[email protected]>

PR-URL: #58948 Fixes: #58949 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Mattias Buelens <[email protected]> Reviewed-By: Juan José Arboleda <[email protected]> Reviewed-By: Luigi Pinca <[email protected]> Reviewed-By: Zeyu "Alex" Yang <[email protected]>

[detached HEAD 936c7f6cc9] test: simplify but keep accurate Author: Ethan Arrowood <[email protected]> Date: Thu Jul 3 15:43:55 2025 -0600 1 file changed, 36 insertions(+), 47 deletions(-) Successfully rebased and updated refs/heads/main.

Uh oh!

Uh oh!

Ethan-Arrowood commented Jul 3, 2025 •

edited

Loading

codecov bot commented Jul 3, 2025 •

edited

Loading

PR-URL: #58948
Fixes: #58949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Mattias Buelens <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>

PR-URL: #58948
Fixes: #58949
Reviewed-By: Matteo Collina <[email protected]>
Reviewed-By: Mattias Buelens <[email protected]>
Reviewed-By: Juan José Arboleda <[email protected]>
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Zeyu "Alex" Yang <[email protected]>

[detached HEAD 936c7f6cc9] test: simplify but keep accurate
Author: Ethan Arrowood <[email protected]>
Date: Thu Jul 3 15:43:55 2025 -0600
1 file changed, 36 insertions(+), 47 deletions(-)
Successfully rebased and updated refs/heads/main.