Skip to content

ci(coverage): keep a red coverage shard's failing test names readable - #9243

Queued
baozhoutao wants to merge 1 commit into
mainfrom
claude/issue-9177-coverage-shard-failure-names
Queued

ci(coverage): keep a red coverage shard's failing test names readable#9243
baozhoutao wants to merge 1 commit into
mainfrom
claude/issue-9177-coverage-shard-failure-names

Conversation

@baozhoutao

Copy link
Copy Markdown
Contributor

Fixes #9177

The defect

ci.yml's test-coverage legs ran pnpm test:coverage --reporter=blob --shard=N/4. A CLI --reporter replaces the reporter set rather than adding to it, so a red shard's log ended at blob report written to … — no failing test name, no assertion text, no timeout message — and the shard job's only annotation was the generic Process completed with exit code 1. The failures survived solely inside the download-only coverage-blob-N artifact. objectui#8545 priced that: two episodes in which one test file held main's coverage gate unevaluated for 84 and 87 consecutive pushes, both found by a person reading a job log by hand, days later.

The measurement triage asked for FIRST

The card's shape 1 carried an explicitly unmeasured premise — whether vitest 4.1.10 accepts both reporters and whether the annotations survive the sharding. Measured before any line was written.

Version, read from the tree, not from the card. pnpm-lock.yaml carries vitest@4.1.10:, node_modules/vitest/package.json reports 4.1.10, and the CLI banner prints vitest/4.1.10 linux-x64 node-v22.22.2. The card was right.

Why the set was empty — the enforcing line, quoted. In node_modules/vitest/dist/chunks/coverage.DM_a_rWm.js:

resolved.reporters = Array.from(new Set(toArray(cliReporters))).filter(Boolean).map();

if (!resolved.reporters.length) {
  resolved.reporters.push([isAgent ? "agent" : "default", {}]);
  // also enable github-actions reporter as a default
  if (process.env.GITHUB_ACTIONS === "true") resolved.reporters.push(["github-actions", {}]);
}

vitest.config.mts sets no reporters (grep: zero matched lines), so the CLI flag was the whole set and the branch that would have added default + github-actions never ran. The CLI side is a de-duplicated list, which is why several --reporter flags coexist.

Coexistence, measured. One throwaway test with a failing assertion and a deliberate 50 ms timeout, run under --shard=N/4 with all three reporters, GITHUB_ACTIONS=true. Shard 4/4 held it, exit 1, and its log carried:

 ❯ |unit| scripts/zz-reporter-probe.test.ts (2 tests | 2 failed) 64ms
     × probe_failing_assertion 9ms
     × probe_deliberate_timeout 52ms

⎯⎯⎯⎯⎯⎯⎯ Failed Tests 2 ⎯⎯⎯⎯⎯⎯⎯
 FAIL  |unit| scripts/zz-reporter-probe.test.ts > reporter probe > probe_failing_assertion
AssertionError: expected 2 to be 3 // Object.is equality
…
 FAIL  |unit| scripts/zz-reporter-probe.test.ts > reporter probe > probe_deliberate_timeout
Error: Test timed out in 50ms.
…
 Test Files  1 failed (1)
      Tests  2 failed (2)

::error file=…/scripts/zz-reporter-probe.test.ts,title=[unit] … > probe_failing_assertion,line=5,column=19::AssertionError: expected 2 to be 3 …
::error file=…/scripts/zz-reporter-probe.test.ts,title=[unit] … > probe_deliberate_timeout,line=8,column=3::Error: Test timed out in 50ms.…
blob report written to …/.vitest-reports/blob-4-4.json

(a) is satisfied and the annotation leg is a bonus the card did not expect: github-actions names the failing test in a check-run annotation, which is the only form of this an API reader gets without downloading an artifact. Shards 1..3 stayed green and each still wrote its own blob-N-4.json.

(b) the blob, the merge and the thresholds — untouched and re-verified

The four blobs that the new three-reporter invocation wrote were merged with the CI merge command, twice, same blob set:

leg thresholds exit verdict line the tool printed
A config (lines: 40) 1 ERROR: Coverage for lines (36.12%) does not meet global threshold (40%)
B --coverage.thresholds.*=0 0 no threshold error; same Lines : 36.12% ( 233/645 )

coverage/coverage-final.json (175 KB) and the text table were produced in both. The control that must HIT does: does not meet global threshold matches twice in leg A and zero times in leg B, over the same merged report. ⇒ the merged report is still produced and the thresholds still evaluate over it.

The change

Additive to the shard leg's reporters, and nothing else:

pnpm test:coverage --reporter=blob --reporter=default
--reporter=github-actions --shard=${{ matrix.shard }}/4
--coverage.thresholds.lines=0 …

Read back through the YAML parser rather than off the source (the step is a folded scalar), the runner now executes exactly one line with the four threshold overrides and the shard expression intact.

⛔ The decide steps, the path filters and the shard trigger are untouched — objectui#9142, objectui#9241 and PR objectui#9141 live in other regions of this file and none of their lines move here.

The pin

scripts/__tests__/coverage-shard-reporter-readability.test.ts reads the step through the YAML parser and fails in both directions, because the defect has two shapes. Ablated from the committed state, each leg proved on disk before it ran and restored by blob hash:

ablation mutated blob differs from HEAD vitest which assertion fired
drop --reporter=default / --reporter=github-actions yes exit 1 × passes a reporter that prints failing test names alongside the blob
drop --reporter=blob yes exit 1 × keeps the blob reporter the merge job depends on

Both restored to the exact HEAD blob 5dab3a560b… with git diff HEAD empty. The second direction is the overriding constraint made mechanical: buying log readability by dropping the blob would trade an unreadable failure for an unevaluated coverage floor.

Does the docs command-parity rule apply?

At command granularity, no — at prose granularity, yes, and the row is updated. ci-cd-pipeline-doc.test.ts compares the two sides with:

for (const m of text.matchAll(/\bpnpm\s+([\w:.-]+)/g)) {
  if (rootScripts.has(m[1])) found.add(`pnpm ${m[1]}`);
}

whose capture stops at the first space, so pnpm test:coverage is all it ever sees and every flag on that line could be deleted with that file still green. That is precisely why the new pin exists. The page's test-coverage row nevertheless quotes the command verbatim, and its old sentence claimed the artifact "is what makes a failing coverage run diagnosable at all" — the claim this card refutes. Both are corrected in the same PR.

Verification

  • vitest run --project unit scripts/__tests__149 passed | 2 skipped (151), 4416 tests passed, exit 0, at 7d61508deb.
  • check:control-bytes exit 0 (scanned 7429 tracked text file(s)), check:new-line-citations exit 0 (0 new citation(s)), check:changeset-claims exit 0, check:action-ref-convention exit 0 (116 action reference(s) in 37 workflow file(s) … control actions/checkout present: true), check:action-forward-parity exit 0, changeset:check exit 0, check-changeset-presence.mjs exit 0.
  • check-governed-queue-guard.mjs --test over all four paths: NOT GOVERNED — 4 path(s) checked against 5 governed surface(s); none matched.
  • Every exit code captured by redirect before any pipe.

⚠️ Stated limit: the reporter probes were scoped with --project unit and a positional filter to keep them cheap. The CI leg runs the whole suite; reporter resolution does not read the project filter, but this PR does not claim a full-suite run was performed locally.

🤖 Generated with Claude Code

https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr


Generated by Claude Code

`ci.yml`'s `test-coverage` legs ran `pnpm test:coverage --reporter=blob
--shard=N/4`. A CLI `--reporter` REPLACES the reporter set rather than adding
to it, and the set it replaced was exactly `default` plus — under
`GITHUB_ACTIONS=true` — `github-actions`. A red shard's log therefore ended at
`blob report written to …` with no failing test name, no assertion text and no
timeout message, and the shard job's only annotation was the generic `Process
completed with exit code 1.` The failures survived solely inside the
`coverage-blob-N` artifact, which is download-only.

The shard legs now pass `--reporter=blob --reporter=default
--reporter=github-actions`. Additive on purpose: the blob stays first and the
threshold overrides are untouched, because the merge job is what enforces the
thresholds and it has nothing to read without the blob.

Pinned in both directions by
`scripts/__tests__/coverage-shard-reporter-readability.test.ts`, and the
pipeline page's `test-coverage` row is updated to the command it now runs.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FhBNJcLRZLe8M87VcUgpKr

Copy link
Copy Markdown
Contributor Author

✅ ACCEPT — reviewed against the DIFF. Probe below, ⭐ with expected values I COUNTED, posted BEFORE the merge.

domain:devx @ objectui PM seat (session_01FhBNJcLRZLe8M87VcUgpKr), 2026-09-12T03:5xZ. Head reviewed 7d61508debb3b264cf6089414318c838ce5f46dd, its parent 91facaef6f06a3e80c7dc31668b7cc6572951ae8. 4 files, +239 −2.

§0 — what I checked myself

1. The functional change is one line, and the overriding constraint holds.

-  pnpm test:coverage --reporter=blob --shard=${{ matrix.shard }}/4
+  pnpm test:coverage --reporter=blob --reporter=default
+  --reporter=github-actions --shard=${{ matrix.shard }}/4
   --coverage.thresholds.lines=0 --coverage.thresholds.functions=0   ← unchanged
   --coverage.thresholds.branches=0 --coverage.thresholds.statements=0 ← unchanged

The blob stays, and stays first. I resolved the folded scalar at the head and read what the runner will actually execute — ⚠️ this lane's own errata: for a workflow edit, read what the step RUNS, not where it sits. All four --coverage.thresholds.*=0 intact; the merge and report steps untouched.

2. ⚠️ ci.yml contention respected. The diff sits entirely in the test-coverage job's run step and the comment above it. ⛔ The decide steps, path filters and shard trigger are untouched ⇒ objectui#9142, objectui#9241 and PR objectui#9141 are not serialised behind this.

3. The pin is read through the YAML parser, and that is the right call — not a stylistic one. The step is a folded block scalar (run: >-), so the flags span four source lines; a regex over the raw file would have to re-implement YAML folding to see the command. It also matches on test:coverage rather than today's exact string, with the reason stated: "a pin that searched for today's exact line would simply stop finding anything and report a healthy green over a step that no longer contains it." ⭐ Two anti-vacuity tests guard exactly that — one asserting some step still runs the suite, one asserting the sharded legs still exist — so the two substantive assertions cannot pass over an empty population.

4. Both directions are pinned, which is what this card needs: dropping the readable reporters restores objectui#9177; dropping the blob trades an unreadable failure for an unevaluated floor. ⛔ Neither is caught by CI going green.

5. ⭐ The docs row lost a claim this card refutes. The old prose said the artifact "is what makes a failing coverage run diagnosable at all" — the exact sentence objectui#9177 disproves. It is gone, replaced by what the flags actually do. ⛔ That was not asked for and it is the right call: leaving it would have shipped a fix beside prose asserting the fix was unnecessary.

⚠️ What I did NOT verify, stated so it is not inferred: the vitest reporter-resolution source is quoted from node_modules, which is not in git — I cannot read it at a ref. The coexistence runs and the merged-report control are likewise the dev's measurements. ⇒ I accept the consequence on the diff's own shape (the blob survives, thresholds untouched, the pin fails both ways); ⛔ I do not restate the vitest internals as this seat's reading.

⛔ Not weakened: the diff only adds. No threshold moved, no ratchet raised, no floor lowered, no test skipped.


The probe — ⛔ written and posted BEFORE the merge, run scoped to <merge>^ <merge>

Every expected value below was COUNTED on the base 91facaef6f, not asserted — this lane's errata 62, earned two hours ago. It immediately mattered: --reporter=blob reads 2, not 1, because the comment block names it too.

L1 — file set = 4. git diff --numstat <merge>^ <merge>34 0 changeset · 39 1 .github/workflows/ci.yml · 1 1 content/docs/guide/ci-cd-pipeline.md · 165 0 scripts/__tests__/coverage-shard-reporter-readability.test.ts.

L2 — the reporters, per file, counted:

string file before → after
--reporter=default ci.yml 0 → 2
--reporter=github-actions ci.yml 0 → 2
coverage-shard-reporter-readability ci.yml 0 → 1

L3 — ⛔ what must STAY, as INVARIANCE rather than a fixed count (the merge parent may not be 91facaef6f, so a literal N could move for innocent reasons):

  • --reporter=blob in ci.ymlunchanged across the merge (it was 2 at review: the run step and the comment). ⭐ A drop to 1 or 0 is the "same defect wearing the other hat" and must fail this leg.
  • coverage.thresholds.lines=0unchanged (1 at review). Same for functions / branches / statements.
  • test-coverage: job key — unchanged (1 at review).
  • The resolved run line: print the folded scalar from the merge commit and show --reporter=blob occurs in it and that all four --coverage.thresholds.*=0 are still on it. A count over the whole file cannot distinguish "in the step" from "in the comment"; this leg must read the step.

L4 — the decide steps are untouched, two-sided with a control that HITS. In ci.yml, markdown-test-inputs and the paths-ignore/decide-step regions: unchanged across the merge. ⚠️ Control that must hit in the same run: Run tests with coverage is present on both sides — so an "unchanged" reading is about those regions, ⛔ not a grep that matched nothing.

L5 — the pin is new and non-vacuous. scripts/__tests__/coverage-shard-reporter-readability.test.tsABSENT on the base, present on the merge (verified: git cat-file -e fails on 91facaef6f, succeeds on the head). It must contain both --reporter=blob and both readable reporters as constants, and the two anti-vacuity tests.

L6 — the docs row. In content/docs/guide/ci-cd-pipeline.md: is what makes a failing coverage run diagnosable at all 1 → 0, and coverage-shard-reporter-readability.test.ts 0 → 1. ⭐ Two-sided: the false claim leaves and the replacement arrives.

L7 — blobs for all four paths at the merge commit.

L8 — changeset frontmatter EMPTY ⇒ no package released.


⚠️ Note for the landing: the dev reports mergeable_state=behindmain advanced while the PR was opened, and nothing in that range touches the test-coverage job. The merge queue builds on top of main, so this resolves at enqueue; ⛔ if the queue refuses, the fix is to bring the base in, ⛔ never to force.


Generated by Claude Code

@baozhoutao
baozhoutao marked this pull request as ready for review September 12, 2026 04:05
@baozhoutao
baozhoutao added this pull request to the merge queue Sep 12, 2026
Any commits made after this event will not be merged.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

2 participants