Skip to content

Conversation

@mcollina
Copy link
Member

Summary

Fixes flaky test test/parallel/test-http2-close-while-writing.js which was timing out on macOS.

Problem

The test uses stream.on('data', ...) which can fire multiple times when receiving the 64KB data chunk sent by the client. Each data event triggers client_stream.destroy() via process.nextTick(), leading to multiple destroy calls and intermittent timeouts.

Solution

Changed stream.on('data', ...) to stream.once('data', ...) to ensure the callback only fires once, regardless of how many data chunks arrive. This pattern is consistent with similar tests like test-http2-many-writes-and-destroy.js.

Verification

Verified with python3 tools/test.py --repeat 100 without failures.

Refs: nodejs/reliability#1459

Use stream.once() instead of stream.on() for the 'data' event
handler to prevent multiple calls when receiving large data chunks.
The test sends 64KB of data which may arrive in multiple chunks,
causing the destroy callback to fire multiple times and leading
to intermittent timeouts on macOS.

Refs: nodejs/reliability#1459
@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 Feb 10, 2026
@codecov
Copy link

codecov bot commented Feb 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 89.74%. Comparing base (3819c7f) to head (c0acd73).
⚠️ Report is 8 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #61764      +/-   ##
==========================================
- Coverage   89.74%   89.74%   -0.01%     
==========================================
  Files         675      675              
  Lines      204642   204642              
  Branches    39322    39320       -2     
==========================================
- Hits       183657   183647      -10     
- Misses      13257    13283      +26     
+ Partials     7728     7712      -16     

see 48 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.

Copy link
Member

@lpinca lpinca left a comment

Choose a reason for hiding this comment

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

See #58252, there is a deeper issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-ci PRs that need a full CI run. test Issues and PRs related to the tests.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants