Conversation
|
|
fa9b3fe to
b396012
Compare
Rebase with requested adjustmentsBranch Changes applied
StatsActions performed
CI statusCI will be checked asynchronously. Automated by Kōan |
|
Really nice work on this — a coverage gate that can only go up is a genuinely good idea, and the implementation is solid. 👏 Deferring it for now rather than merging, for a couple of reasons specific to this moment rather than the quality of the PR:
This is a "not yet," not a "no." Will revisit when we are ready to adopt a coverage policy. Thanks for the thorough implementation and the documentation in CONTRIBUTING! |
Rebase with requested adjustmentsBranch Changes applied
StatsActions performed
CI statusCI will be checked asynchronously. Automated by Kōan |
751407c to
f7881d0
Compare
|
@BabyKoan review |
PR Review — ci: track coverage and block coverage regressions on PRsClean coverage gate with minor CI hygiene issues; not quite merge-ready. The PR does what it promises: V8 coverage over What needs attention:
🟡 Important
1. Coverage job missing npm cache
|
Add a dedicated coverage job to CI that measures V8 coverage on both the PR head and its base branch, then fails if statements or branches decrease. This enforces 'test coverage never decreases on a PR' by direct comparison rather than a static threshold. - vitest.config.ts: configure v8 coverage over the whole src tree with text + json reporters. - package.json: add 'coverage' script (vitest run --coverage). - .github/scripts/compare-coverage.mjs: weighted aggregate of statements and branches from coverage-final.json; emits GitHub error annotations on regression. Functions skipped (v8 per-function counts are unreliable). - .github/workflows/ci.yml: coverage job checks out base + head, runs coverage on both with matching src scope, uploads head report artifact. - .gitignore: ignore coverage/ output. - CONTRIBUTING.md: document the coverage script and CI guard.
Rebase with requested adjustmentsBranch Changes applied
StatsActions performed
CI statusCI will be checked asynchronously. Automated by Kōan |
f7881d0 to
a9a20a6
Compare
What
Track test coverage in CI and block any PR that decreases it.
Why
Coverage was measured ad-hoc locally (
npx vitest run --coverage) but never enforced. Without a guard, untested code can land and silently erode coverage over time. This adds a CI gate so a PR that drops coverage cannot merge.How
A dedicated
coveragejob runs on every PR (.github/workflows/ci.yml):src/**/*.tsscope (apples-to-apples).compare-coverage.mjscomputes weighted statements and branches coverage for each side and fails if head < base.Enforcement is by direct base-vs-head comparison, not a static threshold — there is no number to chase, coverage may only go up. Functions are intentionally excluded because the v8 provider reports unreliable per-function hit counts; statements and branches are stable.
Supporting changes:
vitest.config.ts— v8 coverage over the wholesrctree,text+jsonreporters (json feeds the comparator).package.json—npm run coveragescript..github/scripts/compare-coverage.mjs— aggregatescoverage-final.json, emits::error::annotations on regression..gitignore— ignorecoverage/.CONTRIBUTING.md— document the script and the CI guard.Testing
npm run typecheck && npm test && npm run lint— all green (682 tests).npm run coverage— exits 0; statements 79.13%, branches 68.59%.compare-coverage.mjsagainst identical reports → exit 0 ("No coverage regression").compare-coverage.mjsagainst a report with 300 statements zeroed → exit 1 with::error::statements decreased 4.16%, as designed.The existing
checkjob (typecheck/test/build on Node 20 + 22) is unchanged.Quality Report
Changes: 6 files changed, 138 insertions(+), 1 deletion(-)
Code scan: 4 issue(s) found
.github/scripts/compare-coverage.mjs:60— console.log statement.github/scripts/compare-coverage.mjs:61— console.log statement.github/scripts/compare-coverage.mjs:62— console.log statement.github/scripts/compare-coverage.mjs:81— console.log statementTests: passed (0 test)
Branch hygiene: clean
Generated by Kōan