Skip to content

Split oversized test file test/parallel/test-fs-promises-file-handle-writer.js (1117 lines ->170 lines ) - #65384

Open
BboyAkers wants to merge 12 commits into
nodejs:mainfrom
BboyAkers:main
Open

Split oversized test file test/parallel/test-fs-promises-file-handle-writer.js (1117 lines ->170 lines )#65384
BboyAkers wants to merge 12 commits into
nodejs:mainfrom
BboyAkers:main

Conversation

@BboyAkers

Copy link
Copy Markdown

Summary

Split oversized test file test/parallel/test-fs-promises-file-handle-writer.js (1117 lines)
Split the code into 5 files:

  1. test-fs-promises-file-handle-writer-lifecycle.js
  2. test-fs-promises-file-handle-writer-pipeline.js
  3. test-fs-promises-file-handle-writer-abort.js
  4. test-fs-promises-file-handle-writer-sync.js
  5. test-fs-promises-file-handle-writer-limits-validation.js

Was AI used? If yes, why?

Yes, I used AI to:

  • Help me figure out what I should name the files since I kept encountering naming conflicts and I'm not great at naming πŸ˜…
  • Create the empty files from the results of the prompt for this. I didn't want to create these files manually. Too tedious πŸ˜‚

Sidenote

Everything else, including the code, was broken up by hand and then validated by running the test scripts. For clarity, I also used AI as an additional layer of validation to improve the integrity of this PR, since this is my first PR to Node and wanted ya'lls lives to be as easy as possible while reviewing. I also broke up all the changes per file into individual commits, for clearer version/commit history.

@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 Aug 18, 2026
@@ -0,0 +1,81 @@
const {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These need the 'use strict'; at the top and the require('../common')

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

These should be caught by running make lint

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Thanks!!!! I'll do that!!

@jasnell

jasnell commented Aug 18, 2026

Copy link
Copy Markdown
Member

Your commits will need to be squashed to a single commit that has the test: prefix in the name and includes a Signed-off-by attestation

@codecov

codecov Bot commented Aug 19, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 90.07%. Comparing base (b79dcdf) to head (41ad57c).
⚠️ Report is 215 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65384      +/-   ##
==========================================
- Coverage   90.29%   90.07%   -0.22%     
==========================================
  Files         759      752       -7     
  Lines      247628   251861    +4233     
  Branches    46687    47309     +622     
==========================================
+ Hits       223593   226869    +3276     
- Misses      15483    16323     +840     
- Partials     8552     8669     +117     

see 197 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

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