Skip to content

Commit b7d0adc

Browse files
myasnikovdaniilmyasnikovdaniil
committed
test(hack): run the unit bats suite under bats(1)
hack/cozytest.sh serves two populations that want opposite things from a runner: 66 hermetic unit files covering hack/*.sh, the workflows and the packages tree, and 3 live-cluster imperative suites. This moves the unit half to real bats and leaves the live half where it is; the hack/e2e-%.bats filter already in BATS_UNIT_FILES is what keeps them apart. The unit files need no edits beyond the `load test_helper` the layer below added. They pass under vanilla bats unchanged because their authors wrote to the intersection of both runners -- no run, no $status, no skip, repo-root-relative paths. What this buys, none of which cozytest offers: --filter-status failed to rerun only the previous failures, JUnit so CI annotates the failing test instead of requiring a log read, -j, and deletion of a bespoke awk parser whose rewrite rules void helper exit codes, corrupt heredoc fixtures on a column-0 brace, and collide test names after sanitization. WHY THIS COULD NOT LAND FIRST. Under bats an EXIT handler inside a test body replaces the one the binary installs for its own bookkeeping, and a test that then fails prints no TAP line at all -- not `not ok`, nothing. The suite would have gone green and stayed green while real failures vanished. The 67 test-level handlers are gone as of the commits below this one, and that this closed the hole is measured rather than assumed: under Bats 1.14.0 all 66 files report a plan equal to their declared @test count and execute all of it -- 1161 declared, 1161 planned, 1161 executed, 0 failures, and no `bats warning: Executed N instead of expected M` from any file. An exit code would have answered whether anything failed and never whether anything ran. The same 66 files under hack/cozytest.sh execute 1116 of the 1161. The gap is one pre-existing failure, hack/ghcr-mirror_test.bats, byte-identical to origin/main here apart from its load line: it fails at its 20th test under cozytest, which ends that file, and passes 65 of 65 under bats. Not caused by this change and not addressed by it. Timing, measured on this tree on 8 cores: 367s of work, 151s wall under `bats -j`, against 312s serial under cozytest. The wall time is one file rather than the runner -- hack/run-kubernetes-node-join_test.bats spends 138s of it in real sleeps -- which is worth knowing before reading the parallel speedup as the ceiling. Two consequences worth naming, neither covered by the issue: bats is a new dependency. cozytest is pure bash+awk, so no runner image or contributor machine has ever needed anything else. The checks job installs it from a pinned tag rather than the distro package, whose version varies by base image, and the pin is the version the suite was verified against. The make target says so plainly when bats is absent. GNU parallel stays optional -- BATS_JOBS resolves to 1 without it, since `bats -j` exits non-zero rather than degrading. The pre-commit lane is the second consumer and does NOT get bats. That job lints changed YAML and Markdown on a runner with no bats, and the new hook's path filter covers .github/workflows/, so a workflow edit would drag the whole shell suite into a lint lane and duplicate the checks job that already runs `make unit-tests`. It is skipped there by id, keeping the hook a local-developer gate -- one that costs 151s, which the hook's own comment says rather than implies. The tested shell changes. cozytest is #!/bin/sh, which is dash on the CI runner, and bats runs bash. The POSIX scripts that tests source rather than execute therefore lose a runtime dash check they had by accident, while the shell=bash files under e2e-chainsaw/_lib gain correct fidelity for the same reason. The static replacement is the shellcheck gate in item 5 of #3453, which does not exist yet. Refs: #3453 Assisted-By: LLM Signed-off-by: Myasnikov Daniil <[email protected]>
1 parent d63f216 commit b7d0adc

4 files changed

Lines changed: 78 additions & 7 deletions

File tree

‎.github/workflows/pre-commit.yml‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,14 @@ jobs:
5757
curl -sSL https://github.com/cozystack/cozyvalues-gen/releases/download/v1.6.0/cozyvalues-gen-linux-amd64.tar.gz | tar -xzvf- -C /usr/local/bin/ cozyvalues-gen
5858
5959
- name: Run pre-commit hooks
60+
# bats-unit-tests is a local-developer gate, skipped here. This job
61+
# lints changed YAML/Markdown, and the hook's path filter covers
62+
# .github/workflows/, so a workflow edit would otherwise drag the whole
63+
# shell unit suite into a lint lane -- on a runner with no bats, and
64+
# duplicating the `checks` job in pull-requests.yaml that already runs
65+
# `make unit-tests`.
66+
env:
67+
SKIP: bats-unit-tests
6068
run: |
6169
git fetch origin main || git fetch origin master
6270
base_commit=$(git rev-parse --verify origin/main || git rev-parse --verify origin/master || echo "")

‎.github/workflows/pull-requests.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -311,11 +311,34 @@ jobs:
311311
# Ephemeral runners don't carry the helm-unittest plugin the persistent
312312
# self-hosted runner accreted. Install if absent (idempotent).
313313
- name: Set up test toolchain
314+
env:
315+
BATS_VERSION: v1.14.0
314316
run: |
317+
set -euo pipefail
315318
helm plugin list 2>/dev/null | grep -q unittest \
316319
|| helm plugin install https://github.com/helm-unittest/helm-unittest
320+
# The hack/*.bats unit suite moved from hack/cozytest.sh (pure bash,
321+
# no dependency) to bats(1) in #3453, so the runner image now needs
322+
# bats. Installed from a pinned tag rather than the distro package,
323+
# whose version varies by base image, and pinned to the version the
324+
# suite was verified against rather than to whatever is newest. GNU
325+
# parallel is optional -- the Makefile falls back to a serial run
326+
# when it is absent.
327+
if ! command -v bats >/dev/null 2>&1; then
328+
git clone --depth 1 --branch "$BATS_VERSION" \
329+
https://github.com/bats-core/bats-core.git /tmp/bats-core
330+
sudo /tmp/bats-core/install.sh /usr/local
331+
fi
332+
bats --version
317333
- name: Run unit tests
318334
run: make unit-tests
335+
- name: Upload unit test report
336+
if: always()
337+
uses: actions/upload-artifact@ea165f8d65b6e75b540449e92b4886f43607fa02 # v4
338+
with:
339+
name: bats-unit-report
340+
path: _out/test-reports/
341+
if-no-files-found: ignore
319342
- name: Run controller Go tests
320343
run: make test-controllers
321344

‎.pre-commit-config.yaml‎

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,29 @@ repos:
2626
'
2727
language: system
2828
files: ^.*$
29+
# The hack/*.bats unit suite, hermetic and parallel-safe (#3453). Measured
30+
# on this tree: 367s of work, 151s wall on 8 cores under bats -j, against
31+
# 312s serial under the runner it replaces. Not cheap for a commit hook,
32+
# and honestly so -- but the wall time is one file, not the runner:
33+
# hack/run-kubernetes-node-join_test.bats spends 138s of it in real sleeps,
34+
# so the floor moves when that file does rather than when the machine gets
35+
# bigger. `SKIP=bats-unit-tests git commit` is the escape hatch; the CI
36+
# `checks` job runs the suite either way.
37+
#
38+
# Requires bats-core on PATH; the make target says so explicitly when it is
39+
# missing.
40+
#
41+
# The path filter is the union of what the suite reads, not just the
42+
# scripts under test: several tests assert cross-tree consistency
43+
# (image-pin-consistency over packages/, downstream-trigger-map over
44+
# workflows, md-no-hardwrap over .claude/hooks/), so narrowing this to
45+
# hack/ would let exactly the regressions they exist to catch through.
46+
- id: bats-unit-tests
47+
name: Run hack/*.bats unit tests
48+
entry: make bats-unit-tests
49+
language: system
50+
files: ^(hack/|Makefile$|\.github/workflows/|packages/|\.claude/hooks/)
51+
pass_filenames: false
2952
# Lints changed GitHub Actions workflows for unpinned actions and excessive token
3053
# permissions (audit scope is set in .github/zizmor.yml). Runs offline so it needs
3154
# no GitHub token locally. The same gate runs tree-wide in .github/workflows/zizmor.yml.

‎Makefile‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -149,9 +149,16 @@ test-controllers:
149149
test-check-readiness:
150150
go test ./test/check-readiness/ -count=1
151151

152-
# Discover every hack/*.bats file that is NOT an e2e test and run it
153-
# through cozytest.sh. Drop a new *.bats file in hack/ and it is picked
154-
# up automatically on the next `make unit-tests` run.
152+
# Discover every hack/*.bats file that is NOT an e2e test and run it under
153+
# bats(1). Drop a new *.bats file in hack/ and it is picked up automatically
154+
# on the next `make unit-tests` run.
155+
#
156+
# These are hermetic unit tests of hack/*.sh, and they run under real bats
157+
# rather than hack/cozytest.sh (#3453). The live-cluster suite
158+
# (hack/e2e-*.bats) stays on cozytest, whose streaming trace and snapshot
159+
# behaviour earn their place over a 15-minute test; bats buffers a test's
160+
# output until it completes. Filtering by the e2e- prefix is what keeps the
161+
# two runners apart.
155162
#
156163
# Caveat: $(wildcard ...) returns space-separated names, so a filename
157164
# containing a literal space would split into multiple tokens here. All
@@ -167,15 +174,25 @@ BATS_UNIT_FILES := $(filter-out hack/e2e-%.bats,$(wildcard hack/*.bats))
167174
print-bats-unit-files:
168175
@printf '%s\n' $(BATS_UNIT_FILES)
169176

177+
# `bats -j` needs GNU parallel and exits non-zero rather than degrading when
178+
# it is missing, so resolve the job count to 1 unless parallel is present.
179+
BATS_JOBS ?= $(shell command -v parallel >/dev/null 2>&1 && nproc 2>/dev/null || echo 1)
180+
181+
# JUnit XML so CI annotates the failing test instead of requiring a log read.
182+
# _out is gitignored.
183+
BATS_REPORT_DIR ?= _out/test-reports
184+
170185
bats-unit-tests:
171186
@if [ -z "$(BATS_UNIT_FILES)" ]; then \
172187
echo "ERROR: no hack/*.bats unit test files found"; \
173188
exit 1; \
174189
fi
175-
@for f in $(BATS_UNIT_FILES); do \
176-
echo "--- running $$f ---"; \
177-
hack/cozytest.sh "$$f" || exit 1; \
178-
done
190+
@command -v bats >/dev/null 2>&1 || { \
191+
echo "ERROR: bats not found. Install bats-core >= 1.5 — https://bats-core.readthedocs.io"; \
192+
exit 1; \
193+
}
194+
@mkdir -p $(BATS_REPORT_DIR)
195+
bats -j $(BATS_JOBS) --report-formatter junit -o $(BATS_REPORT_DIR) $(BATS_UNIT_FILES)
179196

180197
# Operator-facing host preflight check. Warns about a standalone
181198
# containerd.service or docker.service running alongside the embedded

0 commit comments

Comments
 (0)