refactor(test): scope coverage with include instead of exclude - #211
Merged
Conversation
`coverage.exclude` patterns are matched against absolute paths with picomatch's `contains` option, so every relative spelling of a `.claude` ignore also matches the project's own root when the repo is checked out under a `.claude/` path, as agent git worktrees in `.claude/worktrees/` are. That excluded every source file and let the thresholds pass against 0/0. All four relative forms were measured to fail open: .claude/** ./.claude/** /.claude/** **/.claude/** #206 fixed this by building an absolute pattern from `import.meta.url`. `coverage.include` is anchored to the project root instead, so scoping by include removes the failure mode structurally rather than working around it, and drops the `node:url` import, the derived root and five exclude entries along with it. `test.exclude` keeps its `.claude/**` entry: it is globbed relative to the project root and is load-bearing at the repo root, where dropping it collects the nested worktrees too (75 test files instead of 15). Coverage output is unchanged in both locations: 187/187 tests, 529/537 statements, no `.claude` paths in the report.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Coverage Report
File CoverageNo changed files found. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Scope v8 coverage with
coverage.include: ["src/**"]instead of enumeratingcoverage.excludeentries. Removes the derived project root, thenode:urlimport and five exclude entries.Why
coverage.excludepatterns are matched against absolute paths with picomatch'scontainsoption. A.claudeignore therefore also matches the project's own root whenever the repo is checked out under a.claude/path — which is exactly where Claude Code agent worktrees live (.claude/worktrees/). Every source file got excluded, and the 95%/90% thresholds passed against0/0while still exiting 0.#206 fixed this by building an absolute pattern from
import.meta.url. That works, but it works around the sharp edge rather than removing it.coverage.includeis anchored to the project root, so scoping by include has no such failure mode.I measured all four relative spellings inside a
.claude/worktrees/checkout — every one of them fails open:.claude/**(pre-#206)Statements : Unknown% ( 0/0 ), exit 0./.claude/**0/0, exit 0/.claude/**0/0, exit 0**/.claude/**0/0, exit 0include: ["src/**"]What is deliberately left alone
test.excludekeeps.claude/**. It is globbed by tinyglobby relative to the project root and is load-bearing at the repo root: dropping it collects the nested agent worktrees too, 75 test files instead of 15. I initially removed it based on a bad experiment — a CLI--excludeappends to the config value rather than replacing it, so both arms of the A/B still had the config entry active. The comment now records this so nobody repeats it.biome.jsonis unchanged."!.claude"is anchored to the config's directory and is load-bearing (removing it takes the check from 107 to 109 files and picks up.claude/). Worth noting the biome half never failed open: with the old"!**/.claude"it reportedChecked 0 files/No files were processed in the specified pathsand exited 1. Only vitest was silently green.Reviewer note — this is a semantic change, not only a cleanup
Setting
includemeans coverage now counts every file undersrc/, not just the ones a test imported. Numbers are byte-identical today because allsrcfiles are currently reached by tests, but from here on a new untestedsrc/*.mtswill pull the percentages down and can fail the 95% gate on an unrelated PR. That is stricter and seemed right for a repo that gates on coverage — happy to revert to the absolute-pattern form from #206 if you would rather not have that.Verification
Config file physically placed in each location and run, not simulated with CLI overrides.
.claude/worktrees/.claudepaths in reportnpm run lintNegative controls:
.claude/src/probe.mtsand.claude/src/probe.test.ts(paths that containsrc/, so they would leak ifincludewere contains-matched) stay out of both collection and the coverage report. No.d.tsrows entered the report.Unrelated, not addressed here: plain
npm testis flaky on a loaded machine (defaulttestTimeout5s /hookTimeout10s vs.generate.test.tsrunning@hey-api/openapi-tsinbeforeAll); the failing test names differ per run.--fileParallelism=falseis reliably green.🤖 Generated with Claude Code