Skip to content

fs: fix glob early return skipping sibling entries#62901

Open
semimikoh wants to merge 1 commit into
nodejs:mainfrom
semimikoh:fix-glob-seen-return
Open

fs: fix glob early return skipping sibling entries#62901
semimikoh wants to merge 1 commit into
nodejs:mainfrom
semimikoh:fix-glob-seen-return

Conversation

@semimikoh

Copy link
Copy Markdown
Contributor

Summary

  • Remove cache.seen check from children iteration loop in #addSubpatterns (sync) and #iterateSubpatterns (async)
  • The return inside the loop exits the entire method, skipping all remaining siblings
  • The cache.add at the top of each method already prevents reprocessing and infinite recursion

Fixes: #62897

Test plan

  • Run test-fs-glob.mjs multiple times to verify flakiness is resolved
  • Verify the reproduction script from the issue passes consistently

@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. labels Apr 22, 2026
@codecov

codecov Bot commented Apr 23, 2026

Copy link
Copy Markdown

Codecov Report

βœ… All modified and coverable lines are covered by tests.
βœ… Project coverage is 89.65%. Comparing base (d080801) to head (4f3a919).
⚠️ Report is 903 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #62901      +/-   ##
==========================================
- Coverage   91.55%   89.65%   -1.90%     
==========================================
  Files         355      708     +353     
  Lines      149381   220407   +71026     
  Branches    23364    42272   +18908     
==========================================
+ Hits       136765   197614   +60849     
- Misses      12354    14655    +2301     
- Partials      262     8138    +7876     
Files with missing lines Coverage Ξ”
lib/internal/fs/glob.js 92.06% <ΓΈ> (+5.38%) ⬆️

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

@ZYSzys ZYSzys added the request-ci Add this label to start a Jenkins CI on a PR. label Apr 23, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Apr 23, 2026
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Signed-off-by: semimikoh <ejffjeosms@gmail.com>
@semimikoh semimikoh force-pushed the fix-glob-seen-return branch from 91565af to 4f3a919 Compare April 30, 2026 18:35
@semimikoh

Copy link
Copy Markdown
Contributor Author

@ZYSzys Could you re-run CI on this when you get a chance? πŸ™

@trivikr trivikr added the request-ci Add this label to start a Jenkins CI on a PR. label Jun 9, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jun 9, 2026
@github-actions

github-actions Bot commented Jun 9, 2026

Copy link
Copy Markdown
Contributor
Failed to start CI
- Validating Jenkins credentials
βœ”  Jenkins credentials valid
- Querying data for job/node-test-pull-request/72881/
[SyntaxError: Unexpected token '<', ..."    
  https://github.com/nodejs/node/actions/runs/27184867612

@semimikoh

semimikoh commented Jul 9, 2026

Copy link
Copy Markdown
Contributor Author

@ZYSzys could you run CI on this?

@trivikr trivikr added request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. labels Jul 9, 2026
@github-actions github-actions Bot added request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jul 9, 2026
@github-actions

github-actions Bot commented Jul 9, 2026

Copy link
Copy Markdown
Contributor
Failed to start CI
- Validating Jenkins credentials
βœ”  Jenkins credentials valid
- Querying data for job/node-test-pull-request/72881/
[SyntaxError: Unexpected token '<', ..."    
  https://github.com/nodejs/node/actions/runs/28997064065

@webdevelopersrinu

Copy link
Copy Markdown

Hi @semimikoh, are you still working on this? It looks like CI hasn't passed and there's been no activity for a while. If you don't have time to continue, I'd be happy to take this over and finish the fix (with credit to your work here). Thanks!

@semimikoh

Copy link
Copy Markdown
Contributor Author

@trivikr Sure, feel free to take it over and continue the work. Thanks!

@webdevelopersrinu

Copy link
Copy Markdown

Thanks @semimikoh Just to confirm I think you meant me (@webdevelopersrinu), as I'm the one who asked above πŸ™‚. I'll take this over, keep your work credited with Co-authored-by, and open a PR with the fix + regression test + green CI.

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

Labels

fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. request-ci-failed An error occurred while starting CI via request-ci label, and manual interventon is needed.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.glob: return in cache-seen check aborts sibling processing, makes test-fs-glob.mjs flaky

5 participants