lib: resolve the issue of not adhering to the specified buffersize

[0hmX]

fixes #55764

We create a queueHandler, and in every iteration we execute the handlers in the queueHandler until we get a non-null result.

[0hmX]

@Ethan-Arrowood, please take a look at the PR when you have a chance. It should be ready for merge unless some any issues is found!

[codecov]

Codecov Report

Attention: Patch coverage is 80.48780% with 8 lines in your changes missing coverage. Please review.

Project coverage is 88.53%. Comparing base (556f1ae) to head (dba2d27).
Report is 1003 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/fs/dir.js	80.48%	8 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main   #55896      +/-   ##
==========================================
+ Coverage   88.00%   88.53%   +0.53%     
==========================================
  Files         656      657       +1     
  Lines      188999   190413    +1414     
  Branches    35989    36554     +565     
==========================================
+ Hits       166331   168589    +2258     
+ Misses      15848    15007     -841     
+ Partials     6820     6817       -3     

Files with missing lines	Coverage Δ
lib/internal/fs/dir.js	95.05% <80.48%> (-2.17%)	⬇️

... and 117 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]

Looking good. Have you tested it locally by running the relevant fs readdir and opendir recursive test files?

I'm not really sure if this needs a unique test as you are more just improving an existing solution.

I requested a CI run now and we'll see how this does.

Thanks for the contribution!

[nodejs-github-bot]

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

[0hmX]

Looking good. Have you tested it locally by running the relevant fs readdir and opendir recursive test files?

Yes, I've tested it. It should be good unless I missed something.

I'm not really sure if this needs a unique test as you are more just improving an existing solution.

Agreed. I don't think we need a new test since, as you mentioned, nothing fundamentally new was added.

[nodejs-github-bot]

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

[Ethan-Arrowood]

Looks good to me, and CI is passing! Thank you, lets gets this merged.

[nodejs-github-bot]

Commit Queue failed

- Loading data for nodejs/node/pull/55896
✔  Done loading data for nodejs/node/pull/55896
----------------------------------- PR info ------------------------------------
Title      lib: resolve the issue of not adhering to the specified buffersize (#55896)
   ⚠  Could not retrieve the email or name of the PR author's from user's GitHub profile!
Branch     cu8code:55723 -> nodejs:main
Labels     fs, needs-ci
Commits    1
 - lib: resolve the issue of not adhering to the specified buffer size
Committers 1
 - cu8code <git8ankanroy@gmail.com>
PR-URL: https://github.com/nodejs/node/pull/55896
Refs: https://github.com/nodejs/node/issues/55764
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
------------------------------ Generated metadata ------------------------------
PR-URL: https://github.com/nodejs/node/pull/55896
Refs: https://github.com/nodejs/node/issues/55764
Reviewed-By: Ethan Arrowood <ethan@arrowood.dev>
--------------------------------------------------------------------------------
   ℹ  This PR was created on Sun, 17 Nov 2024 17:27:03 GMT
   ✔  Approvals: 1
   ✔  - Ethan Arrowood (@Ethan-Arrowood): https://github.com/nodejs/node/pull/55896#pullrequestreview-2470703507
   ✘  Last GitHub CI failed
   ℹ  Last Full PR CI on 2024-11-30T01:14:02Z: https://ci.nodejs.org/job/node-test-pull-request/63800/
- Querying data for job/node-test-pull-request/63800/
   ✔  Last Jenkins CI successful
--------------------------------------------------------------------------------
   ✔  Aborted `git node land` session in /home/runner/work/node/node/.ncu

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

[Ethan-Arrowood]

Hm i thought the CI run succeeded. I'll see if a retry helps.

[nodejs-github-bot]

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

[nodejs-github-bot]

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

[Ethan-Arrowood]

Keep an eye on that CI run. Lets make sure nothing is failing because of these changes. I haven't ran the CI recently so I'm unaware of current known flakey tests, but they are usually pretty obvious.

[edilson258]

@0hmX, are U planing to move with this fix?? i would like to help

[0hmX]

@edilson258 I do not plan to / know how to fix the failing tests. If you can help, I would appreciate it!

[edilson258]

Is there a way of accessing the CI logs or any other way of seeing the failure reason?? @Ethan-Arrowood ?

[Ethan-Arrowood]

Once the CI runs (again fresh) it will comment the result here. That link will be show any failures and details as to why

[nodejs-github-bot]

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

[0hmX]

@Ethan-Arrowood I think this is ready to be merged!

[nodejs-github-bot]

Landed in a8a86b3

[Ethan-Arrowood]

Great work @0hmX ! Thank you for your contribution - apologies it took so long to land! Next one should be faster 🏎️