Skip to content

ci: assert every test vitest counted actually ran (#3825)#3852

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
os-zhuang merged 1 commit into from
Jul 28, 2026
Merged

ci: assert every test vitest counted actually ran (#3825) #3852

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
51 changes: 51 additions & 0 deletions .changeset/ci-test-completeness-guard.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
---
---

ci: assert every test vitest counted actually ran (#3825)

A vitest worker can die at the process level — a native module segfault, OOM, an
abort inside a binding. There is no JS error to catch, so the cases that worker
owned never run and the summary reports only what survived:

```
Test Files 1 passed (40)
Tests 21 passed (401)
```

That leads with "passed". It is **380 tests short**. #3812 hit exactly this shape
(17 cases silently skipped, reported as `22 passed (23)`) and it was found by a
human reading the log closely — which is not a control.

**To be precise about the risk:** the run does exit non-zero, so the gate goes
red. The failure mode is not a false green, it is **a red that reads like a
pass** — someone triaging sees "passed" and a plausible file count and concludes
one file flaked, rather than that a fifth of the suite never executed. This turns
that into a specific, quantified error naming the package and the shortfall. It
also covers the genuinely dangerous variant, where a crash lands somewhere that
does not propagate a non-zero exit at all.

`scripts/check-test-completeness.mjs` reads a saved `turbo run test` log and
asserts, per package, that the tallies (`passed | skipped | failed`) sum to the
declared total. Reading the log rather than wrapping vitest means no change to
the 60+ per-package vitest configs.

Wired into `ci.yml`'s Test Core (both the PR and push steps) and both Dogfood
shards, each as a separate `if: always()` step so it runs **when the suite
failed** — that is when it earns its keep. A red suite plus a green completeness
check means real test failures; a red suite plus a red completeness check means a
worker died.

Two details that are load-bearing rather than incidental:

- The test steps now `tee` their output, and `set -o pipefail` goes with it.
GitHub runs these with `bash -e`, which does **not** set pipefail, so
`turbo … | tee` would report *tee's* exit status and a failing suite would go
green. Verified both ways: with pipefail the step exits 7, without it exits 0.
- Zero summaries in the log is a **pass with an explicit note**, not a silent
one — `turbo run test --affected` legitimately runs nothing when a PR touches
no package.

Validated against the real logs from the #3830 Node 20/22 comparison rather than
synthetic fixtures: the Node 22 log passes (`68 packages, 16678 declared and all
16678 accounted for`), and the Node 20 log fails, naming `@objectstack/driver-sql`
and its 380 missing tests.
44 changes: 41 additions & 3 deletions .github/workflows/ci.yml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -141,11 +141,18 @@ jobs:
# in parallel, and it dominated this job's critical path. The exclusion
# subtracts from the affected set (verified: turbo unions inclusive
# filters, then applies `!` negations to the result).
# `set -o pipefail` is load-bearing: GitHub runs these with `bash -e`, which
# does NOT set it, so `turbo … | tee` would report TEE's status and a
# failing suite would go green. That is the same class of bug the tee is
# here to catch, so it must not be introduced by the catching.
- name: Run affected tests (PR)
if: github.event_name == 'pull_request'
env:
TURBO_SCM_BASE: ${{ github.event.pull_request.base.sha }}
run: pnpm turbo run test --affected --filter=!@objectstack/dogfood --concurrency=4
run: |
set -o pipefail
pnpm turbo run test --affected --filter=!@objectstack/dogfood --concurrency=4 \
2>&1 | tee "$RUNNER_TEMP/test-core.log"

# Push to main: full run. Spec's suite runs here plain (uninstrumented);
# the coverage-instrumented pass moved to the nightly Spec Coverage
Expand All@@ -155,7 +162,23 @@ jobs:
# Dogfood job runs it.
- name: Run all tests (push)
if: github.event_name == 'push'
run: pnpm turbo run test --filter=!@objectstack/dogfood --concurrency=4
run: |
set -o pipefail
pnpm turbo run test --filter=!@objectstack/dogfood --concurrency=4 \
2>&1 | tee "$RUNNER_TEMP/test-core.log"

# Runs even when the suite failed — that is when it earns its keep. A red
# suite plus a GREEN completeness check means real test failures; a red
# suite plus a RED completeness check means a worker died and the cases it
# owned never ran, which reads almost identically in the log (#3812).
- name: Test completeness guard
if: always()
run: |
if [ ! -f "$RUNNER_TEMP/test-core.log" ]; then
echo "No test log — the test step did not get far enough to produce one."
exit 0
fi
node scripts/check-test-completeness.mjs "$RUNNER_TEMP/test-core.log"

# Seed the shared Turbo cache from main only (see the restore step
# above). always(): keep the seed fresh even when a test fails, matching
Expand DownExpand Up@@ -236,7 +259,22 @@ jobs:
# package's `vitest run` and are hashed into the turbo task, so each
# shard caches independently.
- name: Boot example apps and exercise real user flows
run: pnpm turbo run test --filter=@objectstack/dogfood -- --shard=${{ matrix.shard }}/2
run: |
set -o pipefail
pnpm turbo run test --filter=@objectstack/dogfood -- --shard=${{ matrix.shard }}/2 \
2>&1 | tee "$RUNNER_TEMP/dogfood.log"

# Dogfood boots real apps in-process, so a native/OOM abort is likelier
# here than in the unit suites — and a shard that dies silently looks like
# a shard that had less work.
- name: Test completeness guard
if: always()
run: |
if [ ! -f "$RUNNER_TEMP/dogfood.log" ]; then
echo "No test log — the test step did not get far enough to produce one."
exit 0
fi
node scripts/check-test-completeness.mjs "$RUNNER_TEMP/dogfood.log"

# Replaces the former auto-verify dogfood tests: runs the published
# `objectstack verify` engine over each example app through the CLI —
Expand Down
118 changes: 118 additions & 0 deletions scripts/check-test-completeness.mjs
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,118 @@
#!/usr/bin/env node
// Copyright (c) 2025 ObjectStack. Licensed under the Apache-2.0 license.
//
// check-test-completeness -- every test vitest COUNTED must actually have RUN.
//
// A vitest worker can die at the process level -- native module segfault, OOM,
// an abort inside a binding. There is no JS error to catch, so the cases that
// worker owned never run, and the summary reports what survived:
//
// Test Files 1 passed (40)
// Tests 21 passed (401)
//
// That line leads with "passed". It is 380 tests short. #3812 hit exactly this
// (17 cases silently skipped, reported as "22 passed (23)") and it was caught
// by a human reading the log closely, which is not a control.
//
// The run does exit non-zero, so the gate goes red -- the failure mode is not a
// false green, it is a red that READS like a pass. Someone triaging sees
// "passed" and a plausible file count and concludes one file flaked. This turns
// that into a specific, quantified error naming the package and the shortfall.
// It also catches the genuinely dangerous variant, where a crash lands somewhere
// that does not propagate a non-zero exit at all.
//
// node scripts/check-test-completeness.mjs <turbo-test-log>
//
// Reads a saved `turbo run test` log rather than wrapping vitest, so it needs no
// change to the 60+ per-package vitest configs. In CI the test step tees its
// output here. NOTE the tee: `cmd | tee f` reports TEE's exit status, so the
// workflow sets `set -o pipefail` -- without it a failing test suite would look
// green because tee succeeded.

import { readFileSync } from 'node:fs';

const logPath = process.argv[2];
if (!logPath) {
console.error('check-test-completeness: usage: check-test-completeness.mjs <turbo-test-log>');
process.exit(1);
}

let raw;
try {
raw = readFileSync(logPath, 'utf8');
} catch (err) {
console.error(`check-test-completeness: cannot read ${logPath} -- ${err.message}`);
process.exit(1);
}

// Strip ANSI first. vitest colours its summary, and the escape bytes sit
// between the number and its label, so every naive column-based parse of a raw
// log silently reads the wrong field.
const text = raw.replace(/\x1B\[[0-9;]*m/g, '');

// `@objectstack/cli:test: Tests 381 passed | 3 skipped (384)`
// ^ turbo prefix (absent when vitest runs directly) ^ tallies ^ declared
const SUMMARY = /^(?:(\S+?):test:)?\s*(Test Files|Tests)\s+(.+?)\s+\((\d+)\)\s*$/;

const rows = [];
for (const line of text.split('\n')) {
const m = line.match(SUMMARY);
if (!m) continue;
const [, pkg, kind, tallies, declared] = m;

// `381 passed | 3 skipped` -> 384. Every bucket counts as "accounted for";
// a skipped test is a decision, an absent one is a hole.
const counted = [...tallies.matchAll(/(\d+)\s+[a-z]+/g)].reduce((sum, t) => sum + Number(t[1]), 0);

rows.push({
pkg: pkg ?? '(vitest)',
kind,
counted,
declared: Number(declared),
line: line.trim(),
});
}

if (rows.length === 0) {
// Legitimate: `turbo run test --affected` runs nothing when a PR touches no
// package. Say so out loud rather than reporting a vacuous pass.
console.log(
'check-test-completeness: no vitest summaries in the log -- nothing to verify ' +
'(expected when --affected selects no packages).',
);
process.exit(0);
}

const holes = rows.filter((r) => r.counted !== r.declared);

if (holes.length === 0) {
const tests = rows.filter((r) => r.kind === 'Tests');
const total = tests.reduce((sum, r) => sum + r.declared, 0);
console.log(
`check-test-completeness: OK (${tests.length} package(s), ` +
`${total} test(s) declared and all ${total} accounted for).`,
);
process.exit(0);
}

const plural = holes.length === 1 ? 'summary reports' : 'summaries report';
console.error(`check-test-completeness: ${holes.length} ${plural} fewer results than it counted\n`);
for (const h of holes) {
const missing = h.declared - h.counted;
console.error(` • ${h.pkg} -- ${h.kind}: ${h.counted} of ${h.declared} accounted for, ${missing} missing`);
console.error(` ${h.line}`);
}
console.error(`
vitest counted these and then did not report an outcome for all of them. The
usual cause is a worker dying at the process level -- a native module segfault,
OOM, or an abort inside a binding -- which produces no JS error, so the cases
that worker owned never ran and the summary still leads with "passed".

This is not a flake; re-running does not make those tests have run. Reproduce on
the runtime CI uses (see .nvmrc) and look for "Worker exited unexpectedly" or a
non-zero signal exit above the summary.

Precedent: #3812, where a test imported a native better-sqlite3 whose engines
required a newer Node than CI ran. It reported "22 passed (23)" while 17 cases
silently did not run.`);
process.exit(1);
Loading