fix(tests): fail when a chart runs no Helm unit test suites - #3632

What this PR does

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. 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.

That is measurable in both directions rather than argued. Move packages/system/velero/tests/velero_test.yaml aside and run the script as it exists on main today:

Running tests in packages/system/velero
helm unittest .

### Chart [ cozy-velero ] .


Charts:      1 passed, 1 total
Test Suites: 0 passed, 0 total
Tests:       0 passed, 0 total

The run ends with All Helm unit tests passed. and exit 0. The chart passed, and the count of things it checked was zero. With this PR the same tree exits 1 and names the package and the reason. Neither --strict nor an explicit non-matching -f glob changes this on its own; all three forms exit 0, checked against plugin 1.1.1.

So the script now reads the captured output and fails the package when the zero-suite line appears.

Matching that line names a cause and a remedy 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 that make finds nothing to do for, print no marker of any kind and exit 0, so no additional match reaches them; enumerating absences leaves a fresh hole every time a new shape turns up. So the script also requires the line a real run always emits, a Test Suites: count of at least one. That turns the question round: every way of asserting nothing exits 0, and "what did this run assert" is the question that has an answer. All 76 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 same bargain the zero-suite check already strikes.

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 ever firing. The check keys on what make reports rather than on the path existing, 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 it accepts either opening quote; anchoring on the trailing quote alone would also match a sub-make reporting some other target whose name ends in test.

That second check is guarding a live gap rather than a hypothetical one. hack/package.mk line 2 reads .PHONY=help show diff apply delete update image, which is a variable assignment and not a target declaration, so it makes nothing phony. Of the packages the script visits that define a test rule, 18 declare it phony in their own Makefile; the rest are unprotected if a path of that name ever appears. No such path exists today. Fixing the shared include is #3353 and is not this PR.

One deliberate non-change, stated so it is not rediscovered as an oversight. Running the suite now captures output instead of streaming it, because a pipeline's exit status in POSIX sh reports the last command rather than make. Streaming reads better and loses the status, so it stays buffered.

The run is pinned to LC_ALL=C, since both checks match English wording and a localized make would disarm one of them silently, which is the failure mode the whole change exists to remove.

The guard is all-or-nothing, which is worth stating plainly. A chart that loses four of its five suites still reports Test Suites: 1 passed and passes; only total disappearance is caught. Tying the expected count to what each chart actually has would be a per-chart number to maintain, and a number maintained in one place while the suites move in another is the failure this repo has been paying for elsewhere, so the cheap check that catches the total loss is the one worth having.

One behaviour worth stating because the message does not: a suite in which every test carries skip: reports Test Suites: 0 passed, 1 skipped, 1 total and exits 0, and the positive-evidence check refuses it. That refusal is intended, since a wholly skipped suite asserted nothing, but the message talks about suites expected under tests/, which in that case are present and skipped on purpose. No package is in that state today. Partial skips are unaffected: 2 passed, 1 skipped, 3 total satisfies the check.

Three known gaps in what the change ships, none of them a wrong statement. The positive-evidence message explains its cause but stops short of naming a remedy, where the other two messages both end in an action; the remedy for the legitimate case, renaming the target or giving it an opt-out, currently lives only in the code comment. That same message enumerates two ways a run reports nothing, and the skipped-suite case above is a third, so it reads as a diagnosis where it is really a list of the common causes. And no document in the tree states the convention this tightens: docs/agents/overview.md still describes the script accurately as running over every package that defines a test target, but the contract is now that such a package must also report at least one passing suite. All three are improvements to make on the next touch of these files rather than reasons to hold the change.

One adjacent gap stays open, and it is worth naming rather than leaving someone to assume otherwise. This catches a chart whose suite files vanished; it does not catch a package that loses its test: rule along with them. Nothing ties the presence of tests/*_test.yaml to the presence of a test target, so a change that removes both is skipped in silence, and the script only complains when no package in the tree has a test target at all. The test named a package with no test target is skipped, not failed pins that permissiveness deliberately, because the many packages that legitimately define no test rule would otherwise turn every run red. Closing it needs an instrument keyed on the suite files rather than on the Makefile target, which is a separate change.

hack/helm-unit-tests.bats adds eight tests, picked up automatically by the hack/*.bats glob in make unit-tests. Three pin the new refusals, and each matches its own message so none can stay green on a refusal that came from another. Two pin what must not be refused: 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, which is what stops the refusals being satisfied by rejecting everything. Two pin behaviour that was already there, namely that a failing package is still reported by name and that a package with no test rule is skipped rather than failed. That last one builds a tree carrying a second package that does run a suite: with only the target-less package present, the script takes an early exit above the failure summary and the assertion would hold whatever the script had done. The fixtures stub make output instead of invoking helm, so they need no plugin installed.

Relates to #3453.

Screenshots

Downstream repositories

• No downstream repository is affected by this change
• cozystack/website - follow-up:
• cozystack/terraform-provider-cozystack - follow-up:
• cozystack/ansible-cozystack - follow-up:
• cozystack/ccp - follow-up:
• cozystack/talm - follow-up:
• cozystack/cozyhr - follow-up:
• cozystack/cozy-proxy - follow-up:
• cozystack/cozystack-telemetry-server - follow-up:
• cozystack/external-apps-example - follow-up:
• cozystack/examples - follow-up:

Release note

fix(tests): `make helm-unit-tests` now fails a chart that runs no Helm unit test suites, instead of reporting success for a chart whose suite files were deleted, moved, or renamed out of the `tests/*_test.yaml` glob

Summary by CodeRabbit

• Bug Fixes

  • Improved Helm test validation to detect skipped test recipes, zero discovered suites, and runs without evidence of a passing suite.
  • Helm test failures now provide clearer diagnostics, including affected directories and failed commands.
  • Test output is consistently captured and replayed for easier troubleshooting.

• Tests

  • Added comprehensive coverage for successful, failed, skipped, empty, and invalid Helm test scenarios.
  • Expanded coverage across package test targets, including phony and unrelated sub-make behavior.

• Documentation

  • Clarified requirements for package test targets and suite reporting.