Skip to content

test: add FastUtf8Stream tests #58964

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
Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

test: add FastUtf8Stream tests #58964

wants to merge 5 commits into from

Conversation

mertcanaltin
Copy link
Member

@mertcanaltin mertcanaltin commented Jul 5, 2025 •
edited
Loading

I added the tests in sonicBoom to Node.js core accordingly

#58897, #58955

tests added:

  • test-fastutf8stream-destroy.js - Stream destruction and cleanup
  • test-fastutf8stream-end.js - Stream ending and file operations
  • test-fastutf8stream-flush.js - Basic flush operations
  • test-fastutf8stream-flush-mocks.js - Mock-based flush testing
  • test-fastutf8stream-flush-sync.js - Synchronous flush operations
  • test-fastutf8stream-fsync.js - File sync operations
  • test-fastutf8stream-minlength.js - Buffer minLength handling
  • test-fastutf8stream-mode.js - File mode and permissions
  • test-fastutf8stream-periodicflush.js - Periodic flush functionality
  • test-fastutf8stream-reopen.js - File reopening for log rotation
  • test-fastutf8stream-retry.js - EAGAIN/EBUSY retry logic
  • test-fastutf8stream-sync.js - Synchronous write operations
  • test-fastutf8stream-write.js - Core write functionality

jasnell and others added 2 commits July 4, 2025 11:24
@jasnell
fs: port SonicBoom module to fs module as FastUtf8Stream
da471d7 
As a first step to porting portions of the pino structured
logger into the runtime, this commit ports the SonicBoom
module to the fs module as FastUtf8Stream.

This is a faithful port of the SonicBoom module with some
modern updates, such as converting to a Class and using
Symbol.dispose. The bulk of the implementation is unchanged
from the original.
@mertcanaltin
test: add FastUtf8Stream tests
c98448e
@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/security-wg
  • @nodejs/streams

@nodejs-github-bot nodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. tools Issues and PRs related to the tools directory. labels Jul 5, 2025
@jasnell
Copy link
Member

jasnell commented Jul 5, 2025

Thank you! I went ahead and added these to the original PR here. #58897

@mertcanaltin
Copy link
Member Author

Thank you! I went ahead and added these to the original PR here. #58897

you're welcome, then I'm closing this pr ❤️ 🚀

@mertcanaltin mertcanaltin reopened this Jul 7, 2025
@codecov Codecov
Copy link

codecov bot commented Jul 7, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 70.86331% with 243 lines in your changes missing coverage. Please review.

Project coverage is 90.02%. Comparing base (813b4e8) to head (7ac821d).
Report is 24 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/streams/fast-utf8-stream.js 70.63% 242 Missing ⚠️
lib/fs.js 90.00% 1 Missing ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #58964      +/-   ##
==========================================
- Coverage   90.07%   90.02%   -0.05%     
==========================================
  Files         640      642       +2     
  Lines      188442   189500    +1058     
  Branches    36971    37191     +220     
==========================================
+ Hits       169735   170604     +869     
- Misses      11424    11629     +205     
+ Partials     7283     7267      -16
Files with missing lines Coverage Δ
lib/fs.js 98.18% <90.00%> (+0.02%) ⬆️
lib/internal/streams/fast-utf8-stream.js 70.63% <70.63%> (ø)

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. stream Issues and PRs related to the stream subsystem. tools Issues and PRs related to the tools directory.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@mertcanaltin @nodejs-github-bot @jasnell