Repository navigation
testing: 68 test-level EXIT traps in 8 bats files can silence a failing test's TAP line #3623
Description
Summary
68 test-level EXIT traps remain across 8 hack/*.bats files, and each one can make a failing test disappear from the output entirely. A test-level EXIT trap replaces the trap the bats binary installs for its own bookkeeping, so a test that fails prints no TAP line at all, not even not ok. The run ends with # bats warning: Executed N instead of expected M tests, and anyone reading the tail for not ok sees a green suite. Verified experimentally under Bats 1.14.0, and hack/promote-retag_test.bats was the live precedent: six of its eleven tests printed nothing across five passes until conversion surfaced them.
#3622 lands the ratchet that makes this debt visible and exact: every affected file now declares # EXIT-TRAP DEBT: N in its header, and hack/bats-no-exit-trap.bats fails on both growth and undeclared shrinkage. This issue tracks paying the debt down.
The ledger
| file | traps |
|---|---|
hack/select-e2e_test.bats |
15 |
hack/build-matrix_test.bats |
14 |
hack/multus-install-cni-plugins.bats |
12 |
hack/overlay-main-images_test.bats |
11 |
hack/release-changelog-behaviour.bats |
9 |
hack/nightly-mirror_test.bats |
5 |
hack/capture-dataplane.bats |
1 |
hack/release-changelog-contract.bats |
1 |
hack/e2e-test-openapi.bats also declares 1, and that one is not debt: its trap sits inside an explicit subshell (killing a backgrounded kubectl proxy), which does not replace the bats-level trap, so a failure there still prints its not ok. It stays declared because the ratchet is exact in both directions.
How to pay it down
When touching any of the eight files, convert its traps to end-of-body cleanup and lower the file's # EXIT-TRAP DEBT: count in the same change; the guard fails until the number matches. Both runners set -e, so on a failing test the end-of-body cleanup is unreachable and the scratch directory survives for inspection, which is what a failed test wants. A mass one-sweep conversion was deliberately not done: each of these files is owned by other in-flight work, and the exact per-file count makes incremental paydown safe. Until a file reaches zero, do not trust its green run alone: reconcile the 1..N plan against the ok count.
Two boundaries the instrument does not cover
The guard scans .bats files only. hack/e2e-chainsaw/_lib/run-kubernetes.sh installs two handlers that reach test bodies through sourcing, and neither is safe by design: the one at line 572 is benign only because its function is declared with ( ... ) (subshell), so flipping those delimiters to braces reinstates the defect in every calling test with the guard still green; the one at line 830 is a brace function that no @test currently calls. Widening the scan to .sh was rejected on purpose, since a trap is correct in a script and wrong only in a test body, so these two lines are the part a human keeps in mind.
The documented rule in docs/agents/e2e-testing.md bans EXIT and RETURN traps, but every mechanical guard this repo has had, the deleted inventory and the new ratchet alike, matches only EXIT/0. hack/ holds zero RETURN traps today, so nothing is broken, but half of the documented rule has no executor. Closing that means widening the pattern or narrowing the doc's wording.
Activity
test(tests): preserve Bats failure reports during fixture cleanup (co…