Files
cozystack/hack/helm-unit-tests.sh
Aleksei Sviridkin 94c41d100f fix(hack): fail when a chart runs no Helm unit test suites
helm-unittest is fail-closed on a suite it cannot parse and on one
declaring `tests: []`, and fail-open on suites that are not there
at all: it prints "Test Suites: 0 passed, 0 total" and exits 0.
Measured on plugin 1.1.1 against a throwaway chart, all three
forms.

hack/helm-unit-tests.sh judges each package by that exit code
alone, so a chart whose suites were deleted, moved, or renamed past
the tests/*_test.yaml glob reports success having asserted nothing.
Moving packages/system/velero/tests aside reproduces it: the script
prints "All Helm unit tests passed" and exits 0.

Check the captured output for the zero-suite line and fail the
package when it appears. Neither `--strict` nor an explicit
non-matching `-f` glob does this on its own; both exit 0.

Matching that line names the cause where it applies, but the set of
ways to run nothing is not closed: a recipe that never invokes
helm-unittest, and a `test:` rule make finds nothing to do for,
both print no marker at all, so no further match reaches them. So
also require the line a real run always emits, a `Test Suites:` count
of at least one. Every way of asserting nothing exits 0, and asking
what the run asserted is the question that has an answer. All 77
packages the script visits emit that line today. The tradeoff is
that a `test` target which legitimately runs no helm-unittest would
need renaming or an opt-out, which is the bargain the zero-suite
check already struck. That is a contract contributors have to know
about, so docs/agents/overview.md states it where it describes the
sweep.

Because the check matches helm-unittest's own summary wording, a
plugin upgrade that reworded it would fail every package at once
while blaming each package's Makefile. The error text says so, so
the reader of a tree-wide red has the real cause in front of them
rather than 77 copies of the wrong one.

The discovery gate has the same shape from the other side.
`make -C dir -n test` succeeds against a file-backed target, so a
path named `test` beside a package Makefile whose rule is not phony
would make both the gate and the run exit 0 without the recipe
firing. Of the packages the script visits that define a `test` rule,
18 declare it phony, and no such path exists today. Match what make
reports rather than looking for the path, because a package that
declares the target phony runs its recipe whatever sits next to it,
and refusing that would be a false failure. The
quoting around the target name differs between make 3.x and 4.x, so
the match accepts either opening quote; the trailing quote alone
would also match a sub-make reporting another target whose name
ends in "test".

Running the suite now captures output rather than streaming it,
because a pipeline's exit status in POSIX sh reports the last
command rather than make. The capture file is allocated once and
removed by the EXIT trap that already owns the failure list, and the
run is pinned to LC_ALL=C: all three checks match English wording,
one on make's and two on helm-unittest's, so a localized make would
disarm the first of them silently, which is the failure mode this
guard exists to remove. The locale reaches every package's recipe,
not only make.

Extend hack/helm-unit-tests.bats with eight tests. Three pin the new
refusals; two pin what they must not refuse, a phony target with a
colliding path and a sub-make reporting an unrelated target up to
date; one pins that a chart which does run suites still passes; two
pin behaviour that was already there, namely that a failing package
is still reported by name and a package with no `test` rule is
skipped rather than failed. That last one carries a second package
that does run a suite, because with only the target-less package
present the script takes its early exit above the failure summary
and the assertion holds whatever the script did. The fixtures stub
make output instead of invoking helm, so they need no plugin
installed.

Both tests that assert a package is named in the failure summary
match the summary's "  - " prefix rather than the bare path. The
script announces "Running tests in $dir" before it runs anything, so
the bare path is present in the output whatever the summary does;
matching it asserted nothing, and the summary could stop naming
packages entirely with the suite still green.

The failing-package fixture reports a passing suite count alongside
its non-zero exit, so that the exit status is the only thing that
can decide the outcome. A fixture that fails without printing the
marker is refused by the positive-evidence check first, which leaves
the exit-status branch unpinned. That shape occurs in the tree: a
`test` target that runs helm-unittest and then a second step, a
render-parity script or a vendored-version guard or go tests, emits
the marker and still fails.

The two tests already in that file cover a package reached under
packages/tests, and their fixture printed a marker and nothing else.
That is the shape the positive-evidence check refuses, so the
fixture now also emits the line a real run emits; without it both
would fail for a reason unrelated to what they assert.

Assisted-By: Claude <noreply@anthropic.com>
Signed-off-by: Aleksei Sviridkin <f@lex.la>
2026-08-12 18:45:22 +03:00

151 lines
7.3 KiB
Bash
Executable File

#!/bin/sh
set -eu
# Script to run unit tests for all Helm charts.
# It iterates through directories in packages/apps, packages/core,
# packages/extra, packages/system, packages/library and packages/tests and runs
# the 'test' Makefile target if it exists. Keep this list in step with the loop
# below: packages/core carries suites of its own, so dropping it to match a
# stale comment would silently stop running them.
#
# packages/tests is on the list because every check below runs INSIDE a
# directory the loop already reached — the Makefile test, the `make -n test`
# probe, the per-package suite checks, the failure collection. A package that
# never enters the loop takes all of them with it and leaves only tests_found,
# which counts suites across the whole run rather than per package, so it stays
# satisfied by the other directories. That is what happened here, silently:
# packages/tests/cozy-lib-tests ran only via a delegating 'test' target in
# packages/library/cozy-lib/Makefile, and deleting that line dropped the
# library's entire coverage while this sweep stayed green and still reported
# success. The delegation can stay for `make -C packages/library/cozy-lib test`
# ergonomics; it is no longer the only thing running the suite. The cost of
# keeping it is that a full sweep executes cozy-lib-tests twice, once through
# each route — 0.1s to 0.3s over six runs — so the duplicate pair of
# `Running tests in ...` lines in the log is expected rather than a bug.
FAILED_DIRS_FILE="$(mktemp)"
OUTPUT_FILE="$(mktemp)"
trap 'rm -f "$FAILED_DIRS_FILE" "$OUTPUT_FILE"' EXIT
tests_found=0
check_and_run_test() {
dir="$1"
makefile="$dir/Makefile"
if [ ! -f "$makefile" ]; then
return 0
fi
if make -C "$dir" -n test >/dev/null 2>&1; then
echo "Running tests in $dir"
tests_found=$((tests_found + 1))
# LC_ALL=C because all three checks below match English wording, one on
# make's and two on helm-unittest's, and a localized make would disarm
# the first of them silently, which is the failure mode this whole guard
# exists to remove. It reaches every package's recipe, not just make, so
# the recipes run under a fixed locale too; that is the determinism-safe
# direction and the tree is green under it.
#
# Capturing to a file rather than streaming is deliberate: the checks
# below have to read the whole output, and a pipeline's exit status in
# POSIX sh reports the last command rather than make. The cost is that a
# package killed mid-run prints nothing, where a stream would have shown
# the partial run, and that make's stderr is folded into stdout.
rc=0
LC_ALL=C make -C "$dir" test > "$OUTPUT_FILE" 2>&1 || rc=$?
cat "$OUTPUT_FILE"
if [ "$rc" -ne 0 ]; then
printf '%s\n' "$dir" >> "$FAILED_DIRS_FILE"
return 1
fi
# The discovery gate above succeeds against a file-backed target too, so
# it cannot tell a real recipe from one make will skip. Match what make
# actually reports rather than looking for a colliding path, because a
# package that declares the target phony runs its recipe regardless of
# what sits next to the Makefile. The quoting around the target name
# differs between make 3.x and 4.x, so accept either opening quote;
# anchoring on the trailing quote alone would also match a sub-make
# reporting some other target whose name ends in "test".
if grep -qE "[\`']test' is up to date" "$OUTPUT_FILE"; then
echo "ERROR: a 'test' target was reported up to date while testing" >&2
echo " $dir, so its recipe was skipped. The target is shadowed by" >&2
echo " a file or directory of that name, here or in a sub-make" >&2
echo " this package invokes. Remove the path, or declare the" >&2
echo " target phony in the Makefile that defines it." >&2
printf '%s\n' "$dir" >> "$FAILED_DIRS_FILE"
return 1
fi
# helm-unittest is fail-closed on a suite it cannot parse and on one
# declaring `tests: []`, but fail-open on suites that are absent: it
# reports zero of them and exits 0. Charts whose suites were deleted,
# moved, or renamed past the tests/*_test.yaml glob land here.
if grep -qE 'Test Suites:[[:space:]]+0 passed,[[:space:]]+0 total' "$OUTPUT_FILE"; then
echo "ERROR: $dir ran no test suites. Its Makefile declares a 'test'" >&2
echo " target, so suite files are expected under tests/ matching" >&2
echo " *_test.yaml; helm-unittest exits 0 when it finds none." >&2
printf '%s\n' "$dir" >> "$FAILED_DIRS_FILE"
return 1
fi
# Then require positive evidence, because the two checks above match
# shapes of running nothing and there is no reason to believe that list
# is complete. A recipe that never invokes helm-unittest, and a `test:`
# rule make finds nothing to do for, both print no marker at all, so no
# further match could catch them. Asking instead for the line a real run
# always emits turns the question round: every way of asserting nothing
# exits 0, so the run has to say what it asserted. Kept after the two
# specific checks, which name a cause and a remedy where they apply,
# rather than replacing them.
#
# The tradeoff is that a `test` target which legitimately runs no
# helm-unittest would have to be renamed or given an opt-out. That is
# the same bargain the zero-suite check already strikes, and no package
# is in that position today.
if ! grep -qE 'Test Suites:[[:space:]]+[1-9][0-9]* passed' "$OUTPUT_FILE"; then
echo "ERROR: $dir reported no test suite that ran. Its Makefile" >&2
echo " declares a 'test' target, so its recipe is expected to" >&2
echo " run helm-unittest over suites under tests/ matching" >&2
echo " *_test.yaml. A recipe that runs something else, and a" >&2
echo " rule make had nothing to do for, both exit 0 in silence." >&2
echo " If every package fails this way at once, suspect the" >&2
echo " summary wording instead: this matches helm-unittest's" >&2
echo " own, and a plugin upgrade can change it." >&2
printf '%s\n' "$dir" >> "$FAILED_DIRS_FILE"
return 1
fi
fi
return 0
}
for package_dir in packages/apps packages/core packages/extra packages/system packages/library packages/tests; do
if [ ! -d "$package_dir" ]; then
echo "Warning: Directory $package_dir does not exist, skipping..." >&2
continue
fi
for dir in "$package_dir"/*; do
[ -d "$dir" ] || continue
check_and_run_test "$dir" || true
done
done
if [ "$tests_found" -eq 0 ]; then
echo "No directories with 'test' Makefile targets found."
exit 0
fi
if [ -s "$FAILED_DIRS_FILE" ]; then
echo "ERROR: Tests failed in the following directories:" >&2
while IFS= read -r dir; do
echo " - $dir" >&2
done < "$FAILED_DIRS_FILE"
exit 1
fi
echo "All Helm unit tests passed."