Skip to content

Commit 1c2ddf2

Browse files
rustyconoverclaude
andcommitted
ci(integration): enforce a skip contract + executed-case floor
haybarn-unittest exits 0 whether one test skipped or every test did — a failed require/require-env is a SKIP, not an error (and the runner's skip_error_messages HTTP policy turns http errors into skips too) — so "All tests passed" is not evidence anything ran. The failure mode is documented in CLAUDE.md (the lost-HTTP -skip mode, 37048ef) but nothing enforced it; a dead shared worker or an empty stage would still read green. This is the same detector vgi-typescript has had from the start and vgi-go just gained. summarize_run, run after each unittest invocation: * Skip allowlist: every reason the runner prints must be in EXPECTED_SKIP_REASONS (a base set + http-lane additions, since the bad-enum / dedicated-crash / launcher tests RUN on launch but gate off over http). An unlisted reason — a whole-file `require httpfs: 227` — fails the lane. Reasons captured from run 30041947972. * Executed-case floor: the main suite's executed count must exceed a per-lane MIN_EXECUTED — launch/shm 255, http 248, ~16 below the 2026-07-24 measured 271 / 265 against Query-farm/vgi main. A collapse in this number is the signature of a silent skip; a floor, not an equality, so upstream growth is fine. * Empty-stage guard: a "No test cases matched" report fails instead of passing. run_unittest also folds in the fork-child fatal-signal scan (a SIGTERM handler a fork()ed child runs against the parent's counters prints a FAILED block the exit code never sees), matching the rust and typescript harnesses. The gzip_fallback step runs in a subshell so its env overrides don't leak and its single-test count doesn't perturb the main-suite floor. Verified against Query-farm/vgi main: launch 271, shm 271, http 265 executed — all above floors, no unexpected skips. Negative tests confirm both guards bite: an injected `require httpfs` skip fails as unexpected while allowlisted reasons pass, and an executed count below the floor errors. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 6f9498f commit 1c2ddf2

2 files changed

Lines changed: 174 additions & 13 deletions

File tree

ci/README.md

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,33 @@ artifact). The HTTP / bearer / dynamic-code /
7171
`schema_reconcile` tests skip via their `require-env` gates (we don't set those
7272
workers), exactly as in the reference harness.
7373

74+
## The skip contract + executed-case floor
75+
76+
`haybarn-unittest` exits 0 whether one test skipped or every test did — a failed
77+
`require` / `require-env` is a **skip**, not an error (compounded here by the
78+
runner's `skip_error_messages HTTP` policy, which also turns http errors into
79+
skips). So "All tests passed" on its own is not evidence anything ran: a dead
80+
shared worker, an empty stage, or a mis-wired env var all read as green while the
81+
suite quietly tested nothing. `run-integration.sh` closes that gap with two
82+
guards in `summarize_run`:
83+
84+
- **Skip allowlist.** Every skip reason the runner prints must appear in
85+
`EXPECTED_SKIP_REASONS` (a base list plus, on the http lane, the bad-enum /
86+
dedicated-crash / launcher reasons that gate off there but *run* on launch). An
87+
unlisted reason — the signature of a whole file silently gated off, like
88+
`require httpfs: 227` — fails the lane. A genuinely new, legitimate skip is
89+
added to the list *with the reason why*.
90+
- **Executed-case floor.** The main suite's executed-case count must stay above a
91+
per-lane `MIN_EXECUTED` (launch/shm 255, http 248 — floors ~16 below the
92+
measured 271 / 265, leaving room for upstream growth). A collapse in this
93+
number is the tell of a suite-wide silent skip. It is a floor, not an equality:
94+
**do not lower it to make a run pass** — find what stopped running. Ported from
95+
vgi-typescript, whose harness had this from the start.
96+
97+
`run_unittest` also scans for the `fatal error condition` block a fork()ed child
98+
prints against the parent's counters — a failure invisible to the exit code by
99+
construction.
100+
74101
## Run it locally
75102

76103
```bash

ci/run-integration.sh

Lines changed: 147 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,60 @@ case "$TRANSPORT" in
178178
echo "::error::unknown TRANSPORT=$TRANSPORT (expected launch|http)"; exit 1 ;;
179179
esac
180180

181+
# ---------------------------------------------------------------------------
182+
# Skip contract + executed-case floor.
183+
#
184+
# haybarn-unittest exits 0 whether one test skipped or every test did: a failed
185+
# `require` / `require-env` is a SKIP, not an error. So "All tests passed" is not
186+
# evidence anything ran — a dead shared worker, an empty stage, or a mis-wired
187+
# env var all read as green while the suite quietly tested nothing. Two guards
188+
# close that gap (summarize_run, below):
189+
#
190+
# * every skip reason must be named in EXPECTED_SKIP_REASONS — an unlisted
191+
# reason (e.g. a whole-suite `require httpfs`) fails the lane;
192+
# * the count of executed cases must stay above MIN_EXECUTED — the signature
193+
# of a silent collapse is this number falling off a cliff.
194+
#
195+
# The strings are the exact reasons the runner prints under "Skipped tests for
196+
# the following reasons:". These are fixtures / lanes this port deliberately does
197+
# not stand up; each is a legitimate, load-bearing skip.
198+
EXPECTED_SKIP_REASONS=(
199+
'require-env VGI_BAD_PROTOCOL_WORKER' # incompatible-protocol fixture, not wired here
200+
'require-env VGI_RULES_WORKER' # vgi-rust multibatch-repro fixture
201+
'require-env VGI_SCHEMA_RECONCILE_DB' # schema-reconcile sqlite fixture, not set here
202+
'require-env VGI_WORKER_SUPPORTS_DYNAMIC_CODE' # dynamic-code registration, not implemented
203+
'require-env VGI_HTTP_TRANSPORT' # http-identity tests; this port never sets the flag
204+
'require-env VGI_HTTP_DISABLE_ZSTD' # gzip-fallback; skips in the main run, runs in its own step
205+
'require-env VGI_HTTP_NO_COMPRESSION' # no-compression fixture server (Python-side)
206+
'require-env VGI_TEST_BEARER_TOKEN' # bearer-auth fixture server
207+
'require-env VGI_TEST_COMPANION_TARGET' # companion-catalog fixture (Python-side)
208+
'require-env VGI_TEST_ICEBERG' # iceberg fixture, not stood up here
209+
'require-env VGI_DOCKER_IMAGE' # containerised worker lane
210+
'require-env VGI_DOCKER_TCP_IMAGE' # containerised worker over TCP
211+
'require-env VGI_GITHUB_NETWORK_TESTS' # hits github.com; opt-in only
212+
)
213+
# Lane-specific additions — a skip expected on one lane is a red flag on another.
214+
# The launch lane wires the bad-enum / dedicated-crash / launcher workers, so
215+
# those tests RUN there; over http they gate off (shared server / no launcher).
216+
if [ "$TRANSPORT" = "http" ]; then
217+
EXPECTED_SKIP_REASONS+=(
218+
'require-env VGI_BAD_ENUM_WORKER'
219+
'require-env VGI_REQUIRE_LAUNCHER_TRANSPORT'
220+
'require-env VGI_TEST_DEDICATED_WORKER'
221+
)
222+
fi
223+
224+
# Floor on executed cases in the main suite invocation. Deliberately a floor, not
225+
# an equality — the upstream suite grows. Measured 2026-07-24 against
226+
# Query-farm/vgi main: the launch (and shm, which runs TRANSPORT=launch) lane
227+
# executes 271, the http lane 265. The floors sit ~16 below, leaving headroom for
228+
# churn while staying far above the handful a silent collapse would leave.
229+
if [ "$TRANSPORT" = "http" ]; then
230+
MIN_EXECUTED="${MIN_EXECUTED:-248}"
231+
else
232+
MIN_EXECUTED="${MIN_EXECUTED:-255}"
233+
fi
234+
181235
cd "$STAGE"
182236

183237
echo "Warming the extension cache (vgi from community, deps from core) ..."
@@ -203,26 +257,106 @@ EOF
203257
"$HAYBARN_UNITTEST" "test/_warm.test" >/dev/null 2>&1 || echo "::warning::extension warm step did not fully succeed"
204258
rm -f "$STAGE/test/_warm.test"
205259

206-
# Run the whole suite in ONE unittest invocation (as `make test_launcher`
207-
# does), streaming the runner's native sqllogictest report: a `[i/N] (..%):
208-
# test/...` progress line per file and the final
209-
# `All tests passed (.. N assertions in M test cases)` summary (a failure
210-
# prints the offending query + a `M test cases | K failed` summary). This keeps
211-
# the CI log showing that the tests actually ran — and how many assertions —
212-
# rather than a rolled-up count. Out-of-scope tests were already dropped at
213-
# staging, so the glob never matches them; any failed assertion exits non-zero
214-
# and fails the job (via `set -e`).
260+
# summarize_run <log> — parse the runner's console report and enforce the skip
261+
# contract + record the executed-case count. Returns non-zero on an unexpected
262+
# skip reason or a runner that matched no tests at all (the empty-stage
263+
# signature); a suite-assertion failure is caught by the exit code (below).
264+
#
265+
# total = the N in the last "[i/N] (..%):" progress line (cases staged)
266+
# skipped = the sum of the "Skipped tests for the following reasons:" block
267+
# executed = total - skipped
268+
MAIN_EXECUTED=0
269+
summarize_run() {
270+
local log="$1" total skipped executed rc=0 reason
271+
if grep -q 'No test cases matched\|No tests ran' "$log"; then
272+
echo "::error::the runner matched no test cases — the glob or the staging is wrong" \
273+
"(an empty stage still exits 0). transport=$TRANSPORT"
274+
return 1
275+
fi
276+
total="$(sed -n 's/^\[[0-9]*\/\([0-9]*\)\].*/\1/p' "$log" | tail -1)"
277+
[ -n "$total" ] || { echo "::error::could not parse a test-case total out of the report"; return 1; }
278+
skipped="$(awk '
279+
/^Skipped tests for the following reasons:/ { in_block = 1; next }
280+
in_block && /^[[:space:]]*$/ { in_block = 0; next }
281+
in_block && match($0, /: [0-9]+[[:space:]]*$/) { n += substr($0, RSTART + 2) }
282+
END { print n + 0 }' "$log")"
283+
executed=$(( total - skipped ))
284+
echo " executed: $executed / $total skipped: $skipped (transport=$TRANSPORT)"
285+
# Every skip reason must be on the allowlist; an unlisted one fails the lane.
286+
if [ "$skipped" -gt 0 ]; then
287+
while IFS= read -r reason; do
288+
[ -n "$reason" ] || continue
289+
if printf '%s\n' "${EXPECTED_SKIP_REASONS[@]}" | grep -qxF "$reason"; then
290+
continue
291+
fi
292+
echo "::error::unexpected skip reason '$reason'. Either a gate regressed and a" \
293+
"whole file silently stopped running, or this skip is legitimate — if so add" \
294+
"it to EXPECTED_SKIP_REASONS in ci/run-integration.sh with the reason why."
295+
rc=1
296+
done < <(awk '
297+
/^Skipped tests for the following reasons:/ { in_block = 1; next }
298+
in_block && /^[[:space:]]*$/ { in_block = 0; next }
299+
in_block && match($0, /: [0-9]+[[:space:]]*$/) { print substr($0, 1, RSTART - 1) }' "$log")
300+
fi
301+
MAIN_EXECUTED="$executed"
302+
return "$rc"
303+
}
304+
305+
# run_unittest <glob...> — run one haybarn-unittest invocation, streaming its
306+
# report, then fail on anything its exit code cannot express: an unexpected skip,
307+
# an empty stage, or a fatal-signal report a fork()ed child printed against the
308+
# parent's counters (invisible to the exit code by construction).
309+
run_unittest() {
310+
local log rc=0
311+
log="$(mktemp)"
312+
# `&& rc=0 || rc=${PIPESTATUS[0]}` keeps the suite's own exit code without
313+
# tripping errexit and without a trailing `|| true` (which, as a new simple
314+
# command, would overwrite PIPESTATUS with 0 — the accounting must still run
315+
# when the suite itself failed).
316+
"$HAYBARN_UNITTEST" "$@" 2>&1 | tee "$log" && rc=0 || rc="${PIPESTATUS[0]}"
317+
if grep -q 'due to a fatal error condition' "$log"; then
318+
echo "::error::a forked child ran the test harness's signal handler (see the" \
319+
"'fatal error condition' block above). The parent exited $rc and would" \
320+
"otherwise have passed. This is invisible to the exit code by construction."
321+
rc=1
322+
fi
323+
summarize_run "$log" || rc=1
324+
rm -f "$log"
325+
return "$rc"
326+
}
327+
328+
# Run the whole suite in ONE invocation (as `make test_launcher` does), streaming
329+
# the runner's native sqllogictest report: a `[i/N] (..%): test/...` progress
330+
# line per file and the final `All tests passed (.. N assertions in M test
331+
# cases)` summary. Out-of-scope tests were dropped at staging, so the glob never
332+
# matches them; any failed assertion, unexpected skip, or executed-case collapse
333+
# fails the job. run_unittest returns non-zero on failure → `set -e` aborts.
215334
echo "Running suite (single invocation — native sqllogictest report) ..."
216-
"$HAYBARN_UNITTEST" "test/sql/integration/*"
335+
run_unittest "test/sql/integration/*"
217336

218337
# gzip_fallback.test needs VGI_TEST_WORKER itself to be a zstd-disabled HTTP
219338
# worker (it attaches it directly and asserts the gzip codec fallback). That
220339
# conflicts with the main suite's launch: worker, so run it as a dedicated
221340
# single-test invocation against its own worker. Only on the launch lane (shm is
222-
# the same coverage; the http lane is local-only).
341+
# the same coverage; the http lane is local-only). The subshell keeps the env
342+
# overrides from leaking (a `VAR=v func` prefix persists in bash, unlike
343+
# `VAR=v some_binary`) and isolates the summarize_run accounting — the floor
344+
# below is checked against the MAIN suite, not this single-test run.
223345
if [ "$TRANSPORT" = "launch" ]; then
224346
echo "Running http/gzip_fallback.test (zstd-disabled http worker) ..."
225347
gz_port="$(VGI_HTTP_DISABLE_ZSTD=1 boot_http_worker "$VGI_WORKER_BIN")"
226-
VGI_TEST_WORKER="http://localhost:${gz_port}" VGI_HTTP_DISABLE_ZSTD=1 \
227-
"$HAYBARN_UNITTEST" "test/sql/integration/http/gzip_fallback.test"
348+
( export VGI_TEST_WORKER="http://localhost:${gz_port}" VGI_HTTP_DISABLE_ZSTD=1
349+
run_unittest "test/sql/integration/http/gzip_fallback.test" )
350+
fi
351+
352+
# Executed-case floor on the main suite — the collapse-detector. Reached only if
353+
# the runs above passed (a failure already aborted via set -e); MAIN_EXECUTED
354+
# holds the main invocation's count (the gzip subshell does not touch it).
355+
if [ "$MAIN_EXECUTED" -lt "$MIN_EXECUTED" ]; then
356+
echo "::error::only $MAIN_EXECUTED test cases executed on the $TRANSPORT lane," \
357+
"floor is MIN_EXECUTED=$MIN_EXECUTED. This is the signature of a suite-wide" \
358+
"silent skip (a failed require is a SKIP, not an error). Do NOT lower the floor" \
359+
"to make this pass — find what stopped running."
360+
exit 1
228361
fi
362+
echo "Executed $MAIN_EXECUTED main-suite test cases (floor $MIN_EXECUTED) on the $TRANSPORT lane."

0 commit comments

Comments
 (0)