a2n-seattle / a2n-seattle/rms-app

Enforce non-decreasing test coverage in CI (coverage ratchet)

Aperta
#297 1 commento 0 reazioni 0 assegnatari Vedi su GitHub
idea priority-low
Lingua principale
TypeScript
Stelle
1
Fork
1
Merge medio
27m
PR unite (30g)
4

Descrizione

# Why?

**As a** maintainer of rms-app's backend

**I want** CI to fail a PR if test coverage decreases relative to `master`

**So that** the testing policy in root `CLAUDE.md` ("every change must include a corresponding test change") is actually enforced by a hard gate, not just convention — currently `backend-ci.yml`'s coverage check (`artiomtr/jest-coverage-report-action`, `threshold: 80`) is a flat floor. A PR can stay flat or even dip in coverage (as long as it stays ≥80%) and still pass, so coverage can quietly erode over many small PRs even while every individual one is "compliant."

# What?

Add a coverage-ratchet step to `.github/workflows/backend-ci.yml` (or a new workflow) that:

1. Enables a machine-readable coverage report — add `"json-summary"` to `coverageReporters` in `package.json`'s `jest` config (currently only `text`/`text-summary`), so `coverage/coverage-summary.json` gets written when `collectCoverage: true` runs.
2. Runs the unit test suite twice in the same CI job: once against `origin/master` (or the PR's merge-base) to get baseline `coverage-summary.json`, and once against the PR head (already happening via `npm run test:unit`) to get the PR's `coverage-summary.json`.
3. Compares the `total.lines.pct` (and/or `statements`/`branches`/`functions`, TBD during implementation) between the two, and fails the job if the PR's overall coverage percentage is lower than master's. This is a *repo-total* ratchet, not a per-file one — don't block on individual file regressions if the overall number still improves or holds steady, to avoid blocking legitimate refactors that shift lines between files.
4. Keep the existing `artiomtr/jest-coverage-report-action` step for its PR-comment annotations (useful visibility) — this is an additive hard gate, not a replacement.
5. Update root `CLAUDE.md`'s testing-policy section to document the new gate so it's discoverable.

# Additional context

Investigated during a prior session: `artiomtr/jest-coverage-report-action` supports a `base-coverage-file` input for *displaying* a base-vs-head diff in its PR comment, but its actual pass/fail gate (`threshold`) is still a flat floor — it does not fail the check purely because coverage regressed. So this needs a small custom comparison script/step, not just an action config change.

Also note the coverage-collection path currently only covers Gen 1 output (`collectCoverageFrom: ["./amplify/ts-output/src/**/*.js"]`, `roots: ["./amplify/ts-output"]`) — worth double-checking this still points at the right compiled output once the Gen 1 → Gen 2 migration (#292) lands, since `amplify/ts-output` is Gen-1-build-specific.

**Sequencing**: hold off starting implementation until the Gen 1 → Gen 2 migration (#292 and its subtasks #293/#294/#295) completes, since this touches `backend-ci.yml` and the build/test paths that migration is actively changing.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.