Skip to content

Commit bf7cc13

Browse files
os-zhuangclaude
andauthored
fix(ci): make the test-completeness guard see a package that reported nothing (#10205)
* fix(ci): make the test-completeness guard see a package that reported nothing The guard builds its rows by regex over vitest summary lines PRESENT in the log, so a package emitting no summary at all contributes no row: it is neither counted nor missed, it is invisible. Its green therefore means "every package that reported was internally consistent", never "every package on the shard reported" -- and ci.yml's own note read that green as "so these are real test failures", which is how one triage went to a wrong hypothesis and stayed there. Feed the guard the shard's scheduled package list (--scheduled, ci.yml's $RUNNER_TEMP/shard-packages.txt) plus the turbo ls document it was sharded from (--package-list, for each package's directory) and a scheduled-but-silent package becomes a named red. Two measured false-red sources shape the rules rather than a naive scheduled-minus-reported join: - 5 of 77 packages declare no `test` script and 16 more run `vitest run --passWithNoTests`, which prints no summary with no test files, so a package is expected to report only if it has both a test script and at least one test file; - turbo stops scheduling on the first failure, so an ordinary red suite leaves later packages unrun and silent through no fault of their own (measured: one failing task in a 4-task run printed `Tasks: 1 successful, 4 total`). So a missing summary is red only when turbo named the package in its `Failed:` roster (the case this fixes) or when the run completed with every task successful (a suite that went green having reported nothing). A package the run never reached is a note, never red. Behaviour without the new flags is unchanged, so the dogfood job's invocation is untouched. The self-test runs on every invocation rather than from a lint step, which keeps it inside this change's file surface and is the one placement it cannot rot in. Part of #10032 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt * fix(ci): attribute vitest summaries by turbo's group header, not only its line prefix The first version of this guard read package attribution only from the `<pkg>:test:` line prefix. Measured on its own PR (Test Core (1/3), run 32376757655): `@objectstack/spec` reported `Test Files 415 passed (415)` and `Tests 11045 passed (11045)`, and the guard said it had reported nothing. Why: turbo uses STREAM log order locally, which prefixes every line, but switches to GROUPED order in GitHub Actions, which emits ::group::@objectstack/spec:test <- GitHub renders this ##[group] Test Files 415 passed (415) ::endgroup:: and no per-line prefix at all. Grouped is therefore the ONLY shape CI ever writes, so the prefix-only join attributed nothing there and would have reddened every multi-package shard, not just the single-task one that happened to be affected here. Verified on turbo 2.10.10 in both orders. The group header is turbo's own statement of whose output follows, so it is now the primary attribution, with the line prefix still winning when present. Only a `:test` group attributes: `:build` groups and GitHub's own `Run <script>` step groups lend nothing. A summary that neither spelling can attribute is a backstop for a third log shape, and it refuses to guess: one remaining candidate is unambiguous, more than one is not graded at all and says so out loud. Attributing a stray summary to the wrong package would mark a silent package as having reported, which is worse than the gap. Fixture coverage missed this because every local leg used stream-order logs. The self-test now pins both orders, including the exact three lines from the failing CI job, plus the grouped multi-package shape and the group-leak, build-group and step-group cases. Verified: the #10032 true positive still goes red in BOTH orders while origin/main stays green on both; the real CI log that produced the false positive is green; and behaviour without the flags is byte-identical to origin/main on all five fixtures, so the Dogfood invocation is untouched. Part of #10032 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01DdCnBGcHeufjrq7drTD3wt --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 108fae9 commit bf7cc13

2 files changed

Lines changed: 699 additions & 68 deletions

File tree

.github/workflows/ci.yml

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -421,18 +421,46 @@ jobs:
421421
--report-dir "$RUNNER_TEMP/stall-reports" -- \
422422
pnpm turbo run test $FILTERS --concurrency=4
423423
424-
# Runs even when the suite failed — that is when it earns its keep. A red
425-
# suite plus a GREEN completeness check means real test failures; a red
426-
# suite plus a RED completeness check means a worker died and the cases it
427-
# owned never ran, which reads almost identically in the log (#3812).
424+
# Runs even when the suite failed — that is when it earns its keep. It
425+
# answers TWO questions about a red suite, and needs both to be able to
426+
# say anything at all about a green one.
427+
#
428+
# 1. Was every test vitest COUNTED actually run (#3812)? A worker dying
429+
# at the process level leaves a summary that still leads with
430+
# "passed" while falling short of its own declared count — a red
431+
# that READS like a pass.
432+
# 2. Did every package scheduled on this shard report AT ALL (#10032)?
433+
# Question 1 is answered from summary lines PRESENT in the log, so a
434+
# package that printed nothing contributes no row: neither counted
435+
# nor missed, simply invisible. That is why this step is handed the
436+
# shard's scheduled package list (and the `turbo ls` document it was
437+
# sharded from, which resolves each name to a directory) instead of
438+
# grading the log against itself.
439+
#
440+
# ⛔ THE OLD NOTE HERE WAS WRONG AND COST A REAL TRIAGE. It read "a red
441+
# suite plus a GREEN completeness check means real test failures". That
442+
# inference does not hold for case 2, and #10032 is the bill: Test Core
443+
# (2/3) failed naming @objectstack/example-showcase#test while this guard
444+
# printed OK, because the failing package was the one that printed
445+
# nothing — its complete 5083-line job log mentions that package twice,
446+
# both times in the error summary, with no vitest output anywhere. The
447+
# dichotomy is true again only because green now covers both questions.
448+
#
449+
# ⚠ The guard does NOT charge a silent package to this shard when turbo
450+
# simply never reached it — turbo stops scheduling on the first failure,
451+
# so an ordinary red suite leaves later packages unrun. Those are printed
452+
# as notes. See the script header for the two exemptions and the two
453+
# rules that decide red.
428454
- name: Test completeness guard
429455
if: always()
430456
run: |
431457
if [ ! -f "$RUNNER_TEMP/test-core.log" ]; then
432458
echo "No test log — the test step did not get far enough to produce one."
433459
exit 0
434460
fi
435-
node scripts/check-test-completeness.mjs "$RUNNER_TEMP/test-core.log"
461+
node scripts/check-test-completeness.mjs "$RUNNER_TEMP/test-core.log" \
462+
--scheduled "$RUNNER_TEMP/shard-packages.txt" \
463+
--package-list "$RUNNER_TEMP/turbo-ls.json"
436464
437465
# A stall's full diagnostic reports (JS stacks, libuv handles, heap
438466
# summary per process) outlive the in-log digest — keep them so a #4250

0 commit comments

Comments
 (0)