fs: apply exclude function to root path

[Trott]

In at least some situations, the root path was
being added to the results of globbing even if
it matched the optional exclude function.

In the relevant test file, a variable was renamed
for consistency. (There was a patterns and pattern2, rather than patterns2.) The test case is added to
patterns2.

Fixes: #56260

[codecov]

Codecov Report

Attention: Patch coverage is 88.00000% with 3 lines in your changes missing coverage. Please review.

Project coverage is 90.21%. Comparing base (3329efe) to head (53d6b31).
Report is 20 commits behind head on main.

Files with missing lines	Patch %	Lines
lib/internal/fs/glob.js	88.00%	3 Missing ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main   #57420   +/-   ##
=======================================
  Coverage   90.20%   90.21%           
=======================================
  Files         629      629           
  Lines      184948   184973   +25     
  Branches    36204    36220   +16     
=======================================
+ Hits       166837   166876   +39     
+ Misses      11057    11042   -15     
- Partials     7054     7055    +1     

Files with missing lines	Coverage Δ
lib/internal/fs/glob.js	92.00% <88.00%> (-0.15%)	⬇️

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

[nodejs-github-bot]

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

[Trott]

All tests pass. This could use some reviews. @nodejs/fs

[LiviaMedeiros]

The changes LGTM in context of exclude function per se, but I'm not sure if it fixes the issue if we consider arrays of strings.

[Trott]

If someone could review (or re-review), it would let me start Jenkins CI on this.....

[nodejs-github-bot]

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

[LiviaMedeiros]

LGTM with or without the nit.

[nodejs-github-bot]

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

[nodejs-github-bot]

Landed in 8a5a849

[MoLow]

I am not sure if there is a benchmark for glob, but I am curios what the performance impact is here

[Trott]

I am not sure if there is a benchmark for glob, but I am curios what the performance impact is here

There is a bench-glob.js in our benchmarks, if you want to try locally and/or with the Jenkins benchmark job.

[Trott]

I am not sure if there is a benchmark for glob, but I am curios what the performance impact is here

There is a bench-glob.js in our benchmarks, if you want to try locally and/or with the Jenkins benchmark job.

I ran it locally and found a small negative performance effect. I think it's worth it to fix a bug and make the globbing correct, especially on an Experimental feature. But reasonable people might disagree. Here are my results.

                                                                                    confidence improvement accuracy (*)   (**)   (***)
fs/bench-glob.js recursive='false' mode='async' pattern='*.js' dir='lib' n=1000                    -3.77 %       ±3.77% ±5.03%  ±6.56%
fs/bench-glob.js recursive='false' mode='async' pattern='**/*' dir='lib' n=1000                     0.76 %       ±3.36% ±4.48%  ±5.84%
fs/bench-glob.js recursive='false' mode='async' pattern='**/**.js' dir='lib' n=1000                -1.36 %       ±3.50% ±4.65%  ±6.06%
fs/bench-glob.js recursive='false' mode='sync' pattern='*.js' dir='lib' n=1000                     -0.15 %       ±0.95% ±1.27%  ±1.65%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/*' dir='lib' n=1000             ***     -1.83 %       ±0.48% ±0.64%  ±0.83%
fs/bench-glob.js recursive='false' mode='sync' pattern='**/**.js' dir='lib' n=1000         ***     -1.91 %       ±0.52% ±0.69%  ±0.90%
fs/bench-glob.js recursive='true' mode='async' pattern='*.js' dir='lib' n=1000                      0.40 %       ±4.89% ±6.52%  ±8.50%
fs/bench-glob.js recursive='true' mode='async' pattern='**/*' dir='lib' n=1000                      0.78 %       ±3.29% ±4.38%  ±5.70%
fs/bench-glob.js recursive='true' mode='async' pattern='**/**.js' dir='lib' n=1000                  0.02 %       ±6.81% ±9.06% ±11.79%
fs/bench-glob.js recursive='true' mode='sync' pattern='*.js' dir='lib' n=1000                       0.11 %       ±0.80% ±1.06%  ±1.38%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/*' dir='lib' n=1000              ***     -1.56 %       ±0.65% ±0.86%  ±1.12%
fs/bench-glob.js recursive='true' mode='sync' pattern='**/**.js' dir='lib' n=1000          ***     -1.74 %       ±0.48% ±0.64%  ±0.84%

Be aware that when doing many comparisons the risk of a false-positive result increases.
In this case, there are 12 comparisons, you can thus expect the following amount of false-positive results:
  0.60 false positives, when considering a   5% risk acceptance (*, **, ***),
  0.12 false positives, when considering a   1% risk acceptance (**, ***),
  0.01 false positives, when considering a 0.1% risk acceptance (***)