[Tracking] Shell test architecture: vanilla BATS for unit tests, assert library, testable suite shell #3453

Summary

The repo has one shell test runner, hack/cozytest.sh, serving two different jobs: hermetic unit tests of hack/*.sh and live-cluster imperative tests. It reads a hand-rolled subset of the BATS grammar while every test file carries a #!/usr/bin/env bats shebang, so the format advertises semantics the runner does not implement, and the divergences fail silently rather than loudly. Now that Chainsaw owns the app suites (#2979), the live-cluster population has shrunk while the unit population has grown, and the two want opposite things from a runner. This issue proposes splitting them by execution context, moving the unit suite to real BATS (which measurably costs nothing), keeping cozytest for the live suite where its trace and snapshot behaviour genuinely earn their place, and fixing the swallowed-exit-status class in the shell that already runs against live clusters.

Status as of 2026-10-01

Checked against main at bc400dff1.

Two items moved. Item 1a is done: #3848 merged on 2026-09-30 and closed #3623, all eight unit files that declared test-level EXIT handlers (67 of them) are converted. 4 traps are left in two files, capk-provider_test.bats (3) and e2e-test-openapi.bats (1), both still declared in the # EXIT-TRAP DEBT ratchet. Item 7 is done too, #4019 merged on 2026-09-25 and deleted the two monitoring OIDC suites.

Item 1, the runner switch, is #3849 (stacked on #3848, open, CHANGES_REQUESTED). #3497 and #3498 were closed unmerged on 2026-08-16, and #3849 is the flattened result the previous status asked for. make bats-unit-tests on main is still cozytest.sh, but it runs as one make target per file since e49633208 (2026-09-02), so make -j works without bats, and #4633 (2026-10-01) makes it keep going after a failing file. The suite grew from 60 files / 1025 tests to 149 files / 2045 tests.

Items 2, 3, 5 and 6 are untouched and the surface keeps growing: run-kubernetes.sh went from 2893 to 6272 lines and || true in _lib/*.sh from 79 to 93. Item 4 is partial, _lib/etcd-cleanup.sh still has no test. skip is still undefined in cozytest.sh, no assert library exists and no lane runs shellcheck. Open PRs on the same defect class that this body didn't list: #3933 (the interim skip shim), #3790 (negated ! assertions), #3929 (negated containsDocument without any: true) and #4027 (parse every /bin/sh surface with dash).

Measurements

Re-measured against main on 2026-08-13 except where noted.

The earlier load-bearing measurement was 32 of 32 unit files passing under vanilla BATS with no source edits, because their authors wrote to the intersection of both runners (${BATS_TEST_FILENAME:-$0}, repo-root-relative paths, no run/$status/skip). #3849 is the re-measurement over the current suite now, so quote its CI and not the 32/32 number.

Three execution contexts, three verdicts

1. Hermetic unit tests of hack/*.sh → vanilla BATS

Switch bats-unit-tests to invoke bats -j over the existing BATS_UNIT_FILES glob. What this buys, none of which cozytest can offer:

• --filter-status failed, rerun only the previous run's failures, which resolves the fail-fast-versus-edit-loop tension instead of trading it off.
• -F junit / --report-formatter junit, CI annotates the failing test rather than requiring a log read. The Chainsaw suite already uploads chainsaw-report.xml, the shell suite currently emits nothing machine-readable.
• -j, --filter-tags, and bats-assert-style diffing.
• Deletion of the bespoke parser, and with it a whole class of silent failure modes for 60 files at once (see below).

Two strictness regressions to guard, one of them found after this issue was filed.

The first is set -u: cozytest runs test bodies under set -eu -x and BATS does not set -u, so a test that today aborts on an unset variable would silently pass. Verified fix: set -u inside setup() does propagate into the test body, so a shared helper restores parity. hack/test_helper.bash in #3849 is that helper and it is not on main yet. Every unit file has to load it or the migration ships without -u on part of the suite. The [mm:ss] per-line stamps are also lost, which does not matter for tests that finish in milliseconds.

The second was the EXIT-trap population, a prerequisite for the flip. Per docs/agents/e2e-testing.md, verified against Bats 1.14.0, a test body that installs an EXIT trap replaces the one the bats binary installs for its own bookkeeping, and a test that then fails prints no TAP line at all, while hack/cozytest.sh reports the same failure correctly. #3848 cleared the eight files that declared 67 handlers between them. hack/bats-no-exit-trap.bats keeps the ban and the per-file # EXIT-TRAP DEBT ratchet, so the flip no longer waits on this.

On the pre-commit question: 26s makes a pre-commit gate viable, though that figure was taken over 32 files and needs re-running. The durable wins are --filter-status, JUnit and parser deletion, not wall-clock.

2. Live-cluster imperative tests → keep cozytest as the runner, stop growing it

The population here is 4 files (the 2 orphans are gone, see item 7), and the right change is subtractive.

Two of cozytest's properties are genuinely right for a 15-minute live test and are not available from BATS:

• Live streaming trace with elapsed stamps. BATS buffers a test's output until it completes (verified, including under -x), so a long install step is a black box while it runs.
• The EXIT-trap hook that snapshots before cleanup, perishable previous-instance container logs first, then crust-gather, then cozy_cleanup. hack/cozytest-capture-gate.bats (fix(hack): arm cozytest cluster captures for e2e suites only #3645) pins the e2e- prefix that arms it.

These 4 files are already imperative scripts, install then assert, and they already have no per-test isolation: cozytest shares file scope and aborts at the first failure, which is exactly plain-script semantics. So convert each to a plain .sh sourcing the assertion library as it is next touched, and reduce cozytest to what it is actually good at, a traced snapshotting script runner with no @test parser. That also makes these files unit-testable under context 1, which is the ask in context 3 applied to context 2.

The two monitoring OIDC suites that had no invocation site are covered by item 7 below, they were deleted in #4019.

3. Shell embedded in Chainsaw suites → extract, then unit-test

Already largely done at the library level: 7 of the 8 _lib scripts have unit tests (ghcr-mirror.sh was removed in 186547d83, run-proxmox.sh is new). _lib/etcd-cleanup.sh still has none, and _lib/run-kubernetes.sh has grown to 6272 lines against 17 run-kubernetes-*_test.bats files, so its coverage fraction is not established by that count.

The real gap is the 92 inline script: blocks, which are untestable by construction and have grown from 44. Proposed rule: a script: body over roughly 10 lines must be extracted to _lib/<name>.sh taking arguments, with a hack/<name>_test.bats beside it, and inline stays for one-liners. 39 blocks are over that cap today. Enforceable by counting lines under content: | per block. gateway is the largest single win: 22 blocks, 18 of them over the cap, the largest at 231 lines.

The cross-cutting defect: swallowed exit status

The concrete failure that motivates this, a kubectl exec error inside a wait loop going unreported, is a shell problem rather than a runner problem. set -e does not fire in if/while conditions, in && chains, in subshells whose status is discarded, in pipelines without pipefail, or in a loop body whose last command happens to succeed. Changing runners does not address any of those.

The instances live in the shell that already runs against live clusters: the 7455 lines under hack/e2e-chainsaw/_lib/ and the 92 inline script: blocks. Measured surface: 93 || true in _lib/*.sh, 72 of them in run-kubernetes.sh, 57 more in the suite YAMLs, and 8 hand-rolled until/while … kubectl loops. That is where the audit-and-fix work belongs, and the shared helpers below should be written there as those call sites need them rather than up front as a library with no consumer. The unit suite does not need them, its files assert with plain shell today and pass, and no bats-assert vendoring is proposed.

Shared wait/assert helpers, shaped by their call sites

hack/e2e-chainsaw/_lib/ already hosts shared helpers of this kind (remediation-guard.sh), so they belong beside them, and move only if a second execution context turns out to need them.

• assert_ok <cmd…> captures rc and stdout/stderr, prints both on failure, returns non-zero. Never || true.
• retry_until <timeout> <cmd…> as the only sanctioned wait loop. A hand-rolled until … do sleep 2; done discards every intermediate failure, which is precisely the reported bug. On exhaustion this prints the last rc and the last output.
• assert_eq / assert_contains / assert_json_eq, each printing both sides on mismatch.
• POSIX-only, with no reliance on pipefail. cozytest.sh is #!/bin/sh and sources a .bats into its own process, so a sourced library's top-level code runs in dash on the Ubuntu runners, where set -o pipefail is not POSIX and dash 0.5.12 rejects it outright (set: Illegal option -o pipefail, exit 2), aborting the file before the first test reports. It does not reproduce on Arch (/bin/sh is bash) or Debian trixie (dash 0.5.13 added pipefail), so it lands green locally and red in CI, and every BATS-sourced library in the tree is deliberately pipefail-free for this reason. Each helper inspects and returns the status it cares about explicitly instead. Item 1 relaxes this for the unit suite only. The live-cluster path in item 6 keeps the constraint.
• A lint to keep it true: shellcheck, plus a gate rejecting || true and 2>/dev/null on assertion-bearing calls and bare until … kubectl loops outside retry_until. Neither shellcheck nor such a gate exists in .pre-commit-config.yaml or under .github/workflows/ today.

Silent failure modes the parser split removes

Each of these was reproduced against hack/cozytest.sh, and each is still live on main in hack/cozytest.sh.

• The parser voids helper function exit codes, which is the most consequential of these and the correctness argument for item 1 rather than a stylistic one. The awk /^}$/ branch emits return 0 before the closing brace, for every bare column-0 }, including one that closes an ordinary helper defined at file scope, not only a @test block. A helper that ends by falling off its own end therefore returns 0 whatever its body exited with: a three-line subject_fails() { false } returns 0 under cozytest.sh and 1 under vanilla BATS. The dangerous direction is a helper wrapping the subject under test, where run_migration() { docker run … } reports success while the subject exits 1, so every assertion that the subject succeeded passes vacuously. Inline code in a @test body is unaffected, so this bites precisely when factoring a helper out, which is the refactor that looks safest. The present workaround is to capture and return by hand (_rc=0; cmd || _rc=$?; return "$_rc"), which 5 unit files now do, or to return the status via stdout, as hack/md-no-hardwrap.bats does. A second instance was hack/ghcr-mirror_test.bats, which defined a timeout() stub whose voided status was fatal under bash and invisible under dash. The file was deleted in 186547d83. timeout() stubs remain in run-kubernetes-block-io_test.bats, run-kubernetes-cpu-throttle_test.bats and run-kubernetes-join-timing_test.bats, and I haven't checked them for the same bash/dash split. Real BATS does not rewrite function bodies, so item 1 removes the failure mode outright for every unit file.
• skip is undefined, so it resolves to command not found, exits 127 under set -e, and fails the test plus aborts the file, inverting the intent of a graceful-degradation branch. Live today in hack/e2e-install-cozystack.bats, dormant only because CI always has the ValidatingAdmissionPolicy API. It is the only skip call in any .bats file under hack/.
• ! cmd is vacuous under set -e, since errexit is suppressed for a negated pipeline, so an absence assertion written that way asserts nothing. 24 of the 60 unit files (counted on 2026-08-13) carry a comment reasoning about a vacuous assertion and use if cmd; then …; false; fi instead. Real BATS does not change this, it is a shell property rather than a parser one, but the workaround's rationale currently reads as cozytest-specific and will need rewording with the runner.
• A column-0 } inside a heredoc silently corrupts fixture data. The awk transform rewrites the line wherever it appears, so a JSON fixture is written to disk with return 0 injected into it. The probe only failed because the fixture was piped through jq. A test that greps it, or feeds it to a tolerant consumer, passes while asserting against corrupted input. Correctness currently rests on unenforced comments such as "no line below is a bare column-0 }".
• Titles collide after sanitization. "foo bar" and "foo-bar" both become test_foo_bar, the second definition wins, and one test silently disappears.
• A malformed @test line kills the whole file with a bare command not found. No test in it runs, including the well-formed ones, and nothing counts or reports tests.

The same "asserts nothing" class exists outside the shell suite and is worth fixing alongside it, because the reasoning is identical. helm-unittest exits 0 when it finds no suite files at all, and #3632 closed that by failing on Test Suites: 0 passed, 0 total and requiring positive evidence of a suite that ran (hack/helm-unit-tests.sh:87-93 and :109-120). Still open: a negated containsDocument without any: true negates "EVERY rendered document matches", which any multi-document render satisfies trivially. The rule is documented at packages/core/platform/tests/bundles_isp_hosted_dependency_closure_test.yaml:117-124 and seven assertions still break it (#3929 is open for them), at bundles_proxy_protocol_test.yaml:17,149,166, bundles_proxy_protocol_lookup_test.yaml:55, bundles_proxy_protocol_lookup_nil_test.yaml:27 and bundles_velero_default_test.yaml:36,41.

Plan

Each item is independently mergeable, and each is either self-contained or consumed by the one after it, so nothing is built ahead of a caller.

• 1a. Clear the test-level EXIT handlers from the 8 unit files that declared them. Done in test(tests): preserve Bats failure reports during fixture cleanup #3848 (merged 2026-09-30, closed testing: 68 test-level EXIT traps in 8 bats files can silence a failing test's TAP line #3623). hack/bats-no-exit-trap.bats keeps the ban and the ratchet, 4 traps remain in two files (see the status).
• 1. Switch bats-unit-tests to bats -j, add the shared set -u setup() helper so strictness is kept across every unit file, emit a JUnit report, and wire the suite into pre-commit. This is test(tests): run unit suites with Bats and retain POSIX coverage #3849 (open, CHANGES_REQUESTED), stacked on test(tests): preserve Bats failure reports during fixture cleanup #3848, the flattened replacement for test(hack): run the unit bats suite under bats(1) #3497 and test(hack): restore set -u and refresh the cozytest-era headers #3498 (closed unmerged 2026-08-16). Until it lands make bats-unit-tests stays on cozytest, run as one make target per file.
• 2. Audit _lib/*.sh and the inline script: blocks for discarded exit status and fix the instances found: 93 || true in _lib, 57 in the suite YAMLs, 8 hand-rolled until … kubectl loops. The assert_ok / retry_until helpers get written here, shaped by the call sites that need them. Not started.
• 3. Extract gateway's inline script: blocks into _lib with unit tests, on the helpers from item 2. 22 blocks now, 18 over the 10-line cap, the largest at 231 lines. Not started.
• 4. Unit test for _lib/etcd-cleanup.sh, and wider _lib/run-kubernetes.sh coverage. Partly done: 17 run-kubernetes-*_test.bats files exist, etcd-cleanup.sh still has no test.
• 5. Lint gate, once the helpers exist to redirect violators at: reject || true and 2>/dev/null on assertion-bearing calls, hand-rolled until … kubectl loops, BATS-only idioms in the live-cluster files, and inline script: bodies over the line cap. Add shellcheck, which no lane runs today. Not started.
• 6. Convert the 4 live-cluster files to plain .sh sourcing the helpers, and reduce cozytest.sh to a traced snapshotting script runner. Last: highest cost, smallest population.
• 7. Give hack/e2e-apps/monitoring-oidc-system.bats and hack/e2e-apps/monitoring-oidc-customconfig.bats a lane. Closed by test(e2e): remove unused monitoring suites and audit BATS runners #4019 by DELETION rather than a port. The premise here ("deleting these files would drop the only coverage of the feature") does not hold: packages/system/monitoring/tests/oidc_test.yaml already covers the same surface with 37 cases against their 11, and more of it, because everything those files inspect is rendered by the chart. Three assertions were theirs alone - the aggregated-API admission (the generated ApplicationDefinition schema, guarded by codegen-drift and rd-presets-check), grafana-operator injecting an env var into a Deployment it owns (not reachable by any change here), and that no KeycloakRealmGroup is provisioned, which was a real gap and is now a unit case. hack/e2e-apps/ is empty and removed; the four gates matching that path shape pass without it. See the comment below for the detail.

Interim, independent of the sequence above: add a skip() shim to cozytest.sh so the existing call site in hack/e2e-install-cozystack.bats degrades as intended rather than exiting 127. #3933 is open for it, CHANGES_REQUESTED.

Out of scope here but the same defect class, tracked so it is not lost: the seven negated containsDocument assertions listed above need any: true or they assert nothing.

Related

• [Tracking] Remaining CI / build / release / E2E automation work #3278, umbrella CI/build/release/E2E tracker. This is a testing-infrastructure item under it.
• test(e2e): migrate the E2E app suite from BATS to Kyverno Chainsaw #2979, the BATS-to-Chainsaw app suite migration that shrank the live-cluster population and created the imbalance this issue addresses. Closed 2026-08-14. Its 2 orphaned BATS files became item 7 and were deleted in test(e2e): remove unused monitoring suites and audit BATS runners #4019.
• e2e: move per-test resource cleanup into bats teardown() #2583 proposes moving hack/e2e-apps/*.bats cleanup into a BATS teardown(). Both premises have since lapsed: cozytest has no teardown() (the hook is cozy_cleanup, as the two remaining files' own headers state at monitoring-oidc-system.bats:22-25), and the app suite was superseded by the Chainsaw suites, which own their own cleanup. Closed as obsolete. Note this closes the cleanup-mechanism question only, the coverage question those same two files raise is item 7 and stays open.
• test(e2e): fail when a bats file is run by no runner #3778 was closed unmerged on 2026-09-25. The runner audit went in with test(e2e): remove unused monitoring suites and audit BATS runners #4019 as hack/bats-runner-coverage.bats, so a bats file that no runner runs fails the build.
• test(tests): let each bats file declare the EXIT traps it still holds #3622, the EXIT-trap ban and the per-file # EXIT-TRAP DEBT ratchet, which is what makes item 1a countable.
• fix(hack): arm cozytest cluster captures for e2e suites only #3645 pins the e2e- prefix that arms cozytest's capture legs, so a unit runner no longer walks into them.
• fix(tests): fail when a chart runs no Helm unit test suites #3632 closes the helm-unittest fail-open on absent suites, the same "asserts nothing" class as the parser defects above.
• test(tests): preserve Bats failure reports during fixture cleanup #3848, merged 2026-09-30, closed testing: 68 test-level EXIT traps in 8 bats files can silence a failing test's TAP line #3623 and cleared item 1a.
• test(tests): run unit suites with Bats and retain POSIX coverage #3849, the runner switch (item 1), open.
• fix(tests): support skipped tests in cozytest #3933, skip support in cozytest (cozytest.sh implements no skip: a skipping test hard-fails at exit 127 #3827). test: cozytest.sh matches test names by substring, and a pattern that selects nothing exits 0 #3717 (cozytest matches test names by substring) and test: cozytest.sh stops a bats file at its first failing test #4620 (cozytest stops a file at the first failing test) are the other two runner defects open.
• test(tests): make negated BATS assertions able to fail #3790, negated ! assertions that cannot fail (Assertions written as ! command cannot fail a test #3565). fix(platform tests): make negated containsDocument assertions load-bearing with any: true #3929, negated containsDocument without any: true (testing: negated containsDocument without any: true passes on any multi-document render #3864).