Conversation
…fall (#3318) frontend-next ran vitest and Playwright in quality.yml with nothing collecting coverage, so there was no baseline and no way to tell a change that deleted tested behavior from one that did not. Add @vitest/coverage-v8 and a `test:coverage` script (v8, lines and branches, over src/, published as an artifact), then hold the number with a gate. Measured, on the image's own runtime (node 22, node:22-alpine@sha256:c610fcd): npm run test:coverage Test Files 24 passed (24) Tests 215 passed (215) All files | % Stmts | % Branch | % Funcs | % Lines All files | 10.16 | 4.77 | 4.01 | 10.95 10.95% lines, 4.77% branches, over 7359 lines and 7250 branches. Low because src/routes/ is 23,450 lines of route module with almost no unit tests behind it -- recorded as measured rather than trimmed to look better. The one exclusion in the coverage config is the generated src/routeTree.gen.ts, and it costs coverage rather than buying it: including it reports 11.63%. That was measured, not assumed, and the config comment says so. scripts/check-frontend-next-coverage.py reads the summary that run just produced and compares it to the committed coverage-baseline.json, so the gate cannot be satisfied by a constant. Two conditions on the same measurement: - the percentage may not fall more than tolerancePoints (0.5) below the baseline, and - the covered line and branch counts may not fall at all. The second is what makes the first worth having, and that is a measured claim rather than a preference. Over 7359 lines one new untested seven-line module moved the line percentage 10.95% -> 10.94%, which any percentage tolerance passes -- so the percentage alone would not have noticed a real regression. A 400-line untested module moves it to 10.38% and the gate goes red with all 215 tests still green, which is the gap this issue describes. The same script fails when a *.test.ts exists that vitest's own `include:` globs would never collect: a test file nothing runs is a file, not a check, and it would otherwise sit inside the number while contributing nothing. The baseline moves only by editing it in a commit. There is no --update flag, no tolerance on the command line, and the CI step passes no arguments at all, so the measured path and the threshold are both committed files. A missing or unreadable summary exits 2 rather than passing. tests/docs/test_3318_frontend_coverage_ratchet.py (39 tests) pins the gate itself: the decision logic against fixtures, including the seven-line case that the percentage tolerance cannot see; the glob translation, including the `**`-matches-zero-directories case fnmatch gets wrong; the end-to-end exit codes through the real command line; that the CLI cannot grow a flag that widens the gate; that the baseline's own percentages follow from its own counts; and the CI wiring, the untouched `npm test` step, and that no new action was introduced unpinned. The three new steps go into the existing frontend-next/-cloud pair, which already has the node runtime #3331 derives from the image and a `npm ci`, so nothing re-installs a toolchain. The report upload reuses the pinned upload-artifact SHA the job already uses, with if-no-files-found: error so "the report was published" is a fact the step can fail on. No existing test was weakened, skipped or deleted, no file was excluded to raise the number, no threshold was lowered, and openapi.json is untouched.
Dependency ReviewThe following issues were found:
OpenSSF ScorecardScorecard details
Scanned Files
|
Xore
enabled auto-merge (squash)
September 27, 2026 22:52
Xore
disabled auto-merge
September 27, 2026 23:22
Owner
Author
|
Closing as superseded. All of this PR's content landed via #3417 (squash It shared branch Keeping it open would only re-introduce the CONFLICTING state. |
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
Closes the measurement half of #3318 and adds the ratchet.
frontend-nextran vitest and Playwright in
quality.ymlwith nothing collecting coverage,so there was no baseline and nothing that could tell a change which deleted
tested behavior from one which did not.
@vitest/coverage-v8+npm run test:coverage: v8 line and branchcoverage over
src/only,all: trueso an untested file reports 0%rather than nothing, reporters
text/json-summary/html, written tocoverage/(gitignored, published as a CI artifact).scripts/check-frontend-next-coverage.py: reads the summary that run justproduced and compares it to the committed
coverage-baseline.json, so thegate cannot be satisfied by a hardcoded constant. It also fails on a
*.test.tsthat vitest's owninclude:globs would never collect.frontend-next/frontend-next-cloudpair:measure, gate, publish. Nothing re-installs a toolchain — the node runtime
is the one ci: frontend-next is tested on Node 24 but the image runs Node 22 — test on the runtime major #3331 derives from the image, and
npm ciis already there.The measured baseline
Run on the image's own runtime —
node:22-alpine@sha256:c610fcdfb1d5, node22.23.2 / npm 10.9.8, the
FROMline #3331 exists to keep CI on — with thelockfile installed by
npm ci:10.95% lines (806 of 7359) and 4.77% branches (346 of 7250).
That is low, and the reason is structural rather than mysterious:
src/routes/is 23,450 lines of route module with almost no unit testsbehind it. It is recorded as measured. Nothing was excluded to raise it —
the one exclusion in the config is the generated
src/routeTree.gen.ts, andthat one costs coverage rather than buying it: including it reports
11.63% lines. Measured, not assumed; the config comment says so.
The
All filesline above is reproduced by a cleannpm cifrom thecommitted lockfile, and the runner was verified bit-identical across three
runs of the same tree (v8 coverage of an unchanged tree is deterministic), so
the tolerances are not absorbing measurement noise.
The ratchet, and how it was shown failing
Two conditions on the same measurement, because a percentage over 7,359 lines
cannot see a small regression:
tolerancePoints(0.5) below thebaseline — the issue's own ask;
Condition 2 is what makes condition 1 worth having, and that is a measured
claim, not a preference:
coverage lines dropped,coverage branches droppedcovered lines regressed,covered branches regressed*.test.tsoutside the include globsEach RED is the real script against real inputs; the only thing that varies
is which measured/baseline file it reads, or the presence of one temporary
probe file that was removed immediately after. The 400-line probe was a
generated throwaway
src/lib/zzRatchetProbe.ts, deleted before the commit —no existing test file was touched to produce any of those runs.
The gate reads the measured value, not a constant:
--measuredis how theRED runs above were produced, and the CI step passes no arguments at all, so
in CI both the measured path and the threshold are committed files.
Locking the gate
tests/docs/test_3318_frontend_coverage_ratchet.py(39 tests, in theexisting
tests/docs/pytest row — no new CI wiring needed) asserts:and the per-metric reporting that stops branches-for-lines trades hiding
inside an average;
**-glob translation, including the zero-directory casefnmatchgetswrong — the guard's one catastrophic failure mode is matching too little;
missing or unreadable summary is
2and never a pass;--tolerance/--updateflag, so the committedbaseline is the only place strictness can change;
tolerances stay small (
0 < tolerancePoints <= 1,toleranceCounts == 0);if-no-files-found: error, the two jobs' steps still identical, the plainnpm test/ typecheck / build / route-tree steps still present, and no newaction introduced unpinned.
Deliberately not done
npm test,typecheck,buildand the generated-route-tree diff allstill run in both twins.
exclusion is generated code, and it lowers the figure.
pinned (
upload-artifact@ea165f8…v4.6.2), and zizmor + actionlint bothrun clean on the changed workflow.
openapi.jsonuntouched — nothing here reaches the Rust crate.A local
npm installunder npm 11 rewrote unrelatedioredisdedupeentries; that churn was reverted and the lockfile regenerated inside
node:22-alpine, so the diff is 171 pure insertions and the Dependabot lockfile bumps break npm ci for every following PR — check it in the Dependabot PR #1816"lockfile installs under the image's npm" step keeps meaning something.
route module at ~0% is the real finding here; raising it is follow-up work,
not something to smuggle into a measurement change.
tolerancePointsvalue is 0.5, not 0. A v8 run is deterministic,so zero would work mechanically, but a PR adding one ordinary untested
function would then fail CI for it. The pressure valve the issue asks for
is the explicit baseline commit, and it is the only one.
Refs #3318