Skip to content

test_runner: match dotfiles in default coverage exclude - #63401

Open
semimikoh wants to merge 1 commit into
nodejs:mainfrom
semimikoh:test_runner/coverage-dotfile-exclude
Open

test_runner: match dotfiles in default coverage exclude#63401
semimikoh wants to merge 1 commit into
nodejs:mainfrom
semimikoh:test_runner/coverage-dotfile-exclude

Conversation

@semimikoh

@semimikohsemimikoh commented May 18, 2026

Copy link
Copy Markdown
Contributor

The default coverage exclude patterns ... (본문)

Fixes: #63397

Problem

node --experimental-test-coverage includes dotfile test files (e.g. test/.foo.cjs) in the coverage report even though the default exclude patterns are intended to drop everything under test/. Non-dotfile siblings are correctly excluded.

This is caused by matchGlobPattern calling minimatch without dot: true; minimatch's default behavior is to not match dot-prefixed entries unless the pattern itself starts with a dot.

Reported in #63397.

Fix

  • lib/internal/fs/glob.js: extend matchGlobPattern with an optional options argument forwarded to minimatch. Fixed options (nocase, windowsPathsNoEscape, etc.) still take precedence so callers cannot accidentally override them.
  • lib/internal/test_runner/coverage.js: route the four exclude/include match calls through a small helper that passes { dot: true }. Applied to both exclude and include for consistency.

Test

Added a new scenario to test-runner-coverage-default-exclusion.mjs that runs test/.dotfile.cjs explicitly and asserts the dotfile does not appear in the coverage report under the default exclude patterns. The new fixture test/.dotfile.cjs exercises the same logic-file.js as the existing fixtures.

Fixes: #63397

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Review requested:

  • @nodejs/test_runner

@nodejs-github-botnodejs-github-bot added fs Issues and PRs related to the fs subsystem / file system. needs-ci PRs that need a full CI run. test_runner Issues and PRs related to the test runner subsystem. labels May 18, 2026
@codecov

codecovBot commented May 18, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.23810% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.25%. Comparing base (0032189) to head (8437db0).
⚠️ Report is 217 commits behind head on main.

Files with missing linesPatch %Lines
lib/internal/test_runner/coverage.js95.23%1 Missing ⚠️
Additional details and impacted files
@@ Coverage Diff @@## main #63401 +/- ##
==========================================
+ Coverage 90.23% 90.25% +0.02% 
==========================================
Files 741 741 Lines 241194 241210 +16 Branches 45432 45430 -2 ==========================================
+ Hits 217640 217704 +64 + Misses 15129 15080 -49 - Partials 8425 8426 +1 
Files with missing linesCoverage Δ
lib/internal/test_runner/coverage.js63.26% <95.23%> (+0.73%)⬆️

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

Comment threadlib/internal/test_runner/utils.js Outdated
@@ -403,7 +405,7 @@ function parseCommandLine() {
if (!coverageExcludeGlobs || coverageExcludeGlobs.length === 0) {
// TODO(pmarchini): this default should follow something similar to c8 defaults
// Default exclusions should be also exported to be used by other tools / users
coverageExcludeGlobs = [kDefaultPattern];
coverageExcludeGlobs = [kDefaultPattern, kDefaultCoverageDotfilePattern];

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.

Can't we just set dot: true?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Good catch — reverted to the dot: true approach (which the PR body originally described). Pushed as a follow-up fixup. This also makes user-supplied --test-coverage-exclude globs match dotfiles, which the extra default pattern wouldn't have covered.

@semimikoh
semimikohforce-pushed the test_runner/coverage-dotfile-exclude branch 3 times, most recently from d807464 to b0e1f2cCompareMay 21, 2026 03:50
The default coverage exclude globs did not match dotfiles, so test
files such as `test/.foo.test.js` were incorrectly included in coverage
reports. Apply the `dot: true` minimatch option when matching the
relative path so the default exclude patterns cover dotfiles, while
keeping plain matching for the absolute path to avoid misinterpreting
dot segments in the filesystem path (e.g. tmp dirs like `test/.tmp.0`).
Fixes: nodejs#63397
Signed-off-by: semimikoh <ejffjeosms@gmail.com>
@semimikoh
semimikohforce-pushed the test_runner/coverage-dotfile-exclude branch from b0e1f2c to 8437db0CompareJuly 9, 2026 00:55
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fsIssues and PRs related to the fs subsystem / file system.needs-ciPRs that need a full CI run.test_runnerIssues and PRs related to the test runner subsystem.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Default behavior of --test-coverage-exclude globbing does NOT apply to "dotfiles" like .mytest.cjs (common globbing oversight)

3 participants

@semimikoh@nodejs-github-bot@avivkeller