Skip to content

testing: 68 test-level EXIT traps in 8 bats files can silence a failing test's TAP line #3623

Closed

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

  1. added
    kind/failing-testCategorizes issue or PR as related to a consistently or frequently failing test
    area/testingIssues or PRs related to testing (e2e, bats, unit tests)
    on Aug 7, 2026
  2. added
    kind/cleanupCategorizes issue or PR as related to cleanup of code, process, or technical debt
    triage/acceptedIndicates an issue is ready to be actively worked on
    priority/backlogGeneral backlog priority. Lower than priority/important-longterm
    and removed
    kind/failing-testCategorizes issue or PR as related to a consistently or frequently failing test
    triage/needs-triageIndicates an issue needs triage by a maintainer
    on Sep 23, 2026
  3. added a commit that references this issue on Oct 1, 2026
    9ac67d7
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area/testingIssues or PRs related to testing (e2e, bats, unit tests)kind/cleanupCategorizes issue or PR as related to cleanup of code, process, or technical debtpriority/backlogGeneral backlog priority. Lower than priority/important-longtermtriage/acceptedIndicates an issue is ready to be actively worked on

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions