Skip to content

stream: port remaining SonicBoom tests for Utf8Stream - #66275

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix/fast-utf8-stream-max-write-retries
Open

mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:fix/fast-utf8-stream-max-write-retries

Conversation

@mcollina

Copy link
Copy Markdown
Member

Port the SonicBoom tests that could not be imported directly because they depend on the third-party proxyquire module, along with the maxWriteRetries option they exercise, into the Utf8Stream module.

Adds maxWriteRetries support, which bounds consecutive EAGAIN/EBUSY retry attempts and resets the counter on forward progress, plus drop-event and flush-callback edge-case coverage.

Fixes: #58955


AI generated, reviewed by me

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/streams

@mcollina
mcollina requested a review from jasnell September 25, 2026 08:34
@nodejs-github-bot nodejs-github-bot added needs-ci PRs that need a full CI run. stream Issues and PRs related to Node.js streams. labels Sep 25, 2026
@mcollina
mcollina force-pushed the fix/fast-utf8-stream-max-write-retries branch from f803775 to 919e61f Compare September 25, 2026 08:34
@codecov

codecov Bot commented Sep 25, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 77.41935% with 7 lines in your changes missing coverage. Please review.
✅ Project coverage is 90.37%. Comparing base (ebef774) to head (7309c02).
⚠️ Report is 238 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/fast-utf8-stream.js 77.41% 7 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #66275      +/-   ##
==========================================
+ Coverage   90.28%   90.37%   +0.08%     
==========================================
  Files         790      792       +2     
  Lines      271642   275527    +3885     
  Branches    51846    52803     +957     
==========================================
+ Hits       245260   249008    +3748     
- Misses      16889    16929      +40     
- Partials     9493     9590      +97     
Files with missing lines Coverage Δ
lib/internal/streams/fast-utf8-stream.js 83.07% <77.41%> (+2.20%) ⬆️

... and 188 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.

Port the SonicBoom tests that could not be imported directly because
they depend on the third-party proxyquire module, along with the
maxWriteRetries option they exercise, into the Utf8Stream module.

Adds maxWriteRetries support, which bounds consecutive EAGAIN/EBUSY
retry attempts and resets the counter on forward progress, plus
drop-event and flush-callback edge-case coverage.

Fixes: nodejs#58955
Signed-off-by: Matteo Collina <hello@matteocollina.com>
PR-URL: nodejs#66275
Assisted-by: pi
@mcollina
mcollina force-pushed the fix/fast-utf8-stream-max-write-retries branch from 919e61f to 7309c02 Compare September 29, 2026 10:27
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. stream Issues and PRs related to Node.js streams.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tracking Issue: Port all SonicBoom tests for FastUtf8Stream

2 participants