test(chart): errorPattern asserted nothing, so 7 refusal tests asserted nothing (backend#2606)

[LukasWodka]

Summary

failedTemplate + errorPattern asserts nothing in helm-unittest 0.5.2 — the plugin never reads the key. Seven such assertions across five suites were therefore exactly equivalent to a bare failedTemplate: {} while reading, in review, like a pinned refusal. This PR converts them, and gates the pattern so it cannot come back.

Closes tracebloc/backend#2606

The proof, re-run here before anything was changed

Probe on rbac_test.yaml's perDatasetPvcs case, errorPattern replaced with a string that appears nowhere:

279: errorPattern: "ZZZ_THIS_STRING_APPEARS_NOWHERE_ZZZ"
PASS RBAC	client/tests/rbac_test.yaml
Tests: 19 passed, 19 total

--strict is not a substitute, and that was checked rather than assumed. With errorPatternand an invented bogusKeyThatDoesNotExist both set on one assertion:

=== non-strict === Tests: 19 passed, 19 total
=== --strict === Tests: 19 passed, 19 total

The plugin validates no key names under failedTemplate at all. That is why the guard below allowlists errorMessage rather than blocklisting errorPattern — a typo fails just as silently.

Ticket table was one short: 7, not 6

grep -rn errorPattern client/tests/ found a seventh live assertion the ticket's table missed — secrets_test.yaml:326, the bootstrapDbReparent case, also a template fail. It carried the comment "Assert the stable prefix via errorPattern, not the full message", i.e. someone chose it deliberately, believing it worked. Converted here too. (Its full message names the credential's source indirectly, backend#947, and carries no literal value — so reproducing it in a public repo is safe for the same reason the template's own text is.)

Remedy 1 — the 2 template fails: errorMessage, mutation-proven

file	template message reworded	result
client/tests/rbac_test.yaml	perDatasetPvcs requires clusterScope → perDatasetPvcs needs something else entirely	1 failed, 18 passed, 19 total
client/tests/secrets_test.yaml	...bootstrapDbPassword is unset → ...something completely different	1 failed, 29 passed, 30 total

Both mutation anchors were asserted applied before the run (grep -c on the mutated string = 1), and both failure outputs printed the expected-vs-actual strings, so the mutation was not inert. After cp-restore, git diff --quiet on each template confirmed no residue and both suites returned 19 passed / 30 passed.

Remedy 2 — the 5 schema rejections: bare, and documented as bare

A values.schema.json rejection fails at chart load, before any template renders; the plugin reports an "errored" test and errorMessage compares against a render error that never happened. No message assertion can work. These become failedTemplate: {} with a comment stating why and pointing at errorMessage / the outside-the-plugin route, so nobody "strengthens" them back into vacuity.

They were probed in the other direction too — a bare assertion that passes even on a legal config would prove nothing. Each set: block was made legal and the assertion reddened:

file / case	legal value substituted	result
auto_upgrade_test.yaml — image.tag=latest	tag: "3.16.4"	1 failed, 21 passed, 22 total
image_refresh_test.yaml — ingestor.tag=latest	tag: "0.8"	1 failed, 35 passed, 36 total
image_refresh_test.yaml — image.tag=latest	tag: "1.30.5"	1 failed, 35 passed, 36 total
network_policy_test.yaml — empty clusterCidrs	["10.0.0.0/8"]	1 failed, 12 passed, 13 total
priority_class_pdb_test.yaml — empty name	"tracebloc-training"	1 failed, 18 passed, 19 total

Every anchor was asserted unique (assert s.count(a) == 1) before substitution, and every file was restored from a copy afterwards.

The guard

scripts/tests/helm-unittest-error-assertions.sh, added to the Makefile's DRIFT_GUARDS (now 22 entries). It runs in Source-of-truth drift — a required status check on develop and main. Deliberately not in helm-ci's unittest job: Helm unit tests is required on neither branch, so a guard there could only advise (backend#1729 rule 2). drift-checks.yaml has no paths: filter on pull_request, so it reports on every PR.

Not a grep, for a concrete reason.errorPattern is now named on purpose in several test-file comments — including the ones this PR added, warning people off it — so a text match on the token flags the warnings and not the violations. The guard parses the YAML and inspects the keys of a failedTemplate mapping, derived from */tests/*_test.yaml so an ingestor/tests suite added later is covered without editing the script.

Mutation-proven, both directions of the rule:

### MUTATION 1: reintroduce errorPattern
[FAIL] client/tests/network_policy_test.yaml: .tests[2].asserts[0].failedTemplate.errorPattern
guard exit: 1
### MUTATION 2: typo key errorMesage
[FAIL] client/tests/rbac_test.yaml: .tests[18].asserts[0].failedTemplate.errorMesage
guard exit: 1
### RESTORED
[OK] 19 failedTemplate assertion(s) across 34 suite file(s); every key present is in ['errorMessage']
guard exit: 0

Both offender paths name the exact assertion index, so the mutation demonstrably applied rather than the guard failing for some other reason.

All three fail-closed paths exercised (backend#1729 rule 3), in a throwaway tree:

### A: zero suites → [ERROR] found ZERO helm-unittest suites ... refusing. exit 1
### B: unparseable suite → [ERROR] ... could not be parsed ... 'cannot tell' is a finding. exit 1
### C: suites, 0 failedTemplate → [ERROR] walked 1 suite file(s) and reached ZERO ... exit 1

Test counts — before / after

before: Test Suites: 34 passed, 34 total Tests: 581 passed, 581 total
after: Test Suites: 34 passed, 34 total Tests: 581 passed, 581 total

Unchanged by design: no test was added or removed, seven assertions changed from asserting nothing to asserting something (or to saying honestly that they cannot).

Other checks run

• make check (parse + shellcheck + drift + helm-lint + helm-vocab): green. all 58 shell scripts parse; shellcheck: 64 file(s), severity=error clean — the new script is picked up by the derived file set. chart-env-vocabulary: all 50 checks passed.
• make drift: all 22 guards green.
• scripts/check-style.sh: green after the .cursor/BUGBOT.md edit.
• bats scripts/tests/bats-hygiene.bats check-style.bats chart-version-guard.bats: 72 ok, 0 not ok.
• Not verified locally, stated plainly: the fullmake bats suite. It hung on this Mac for 20+ minutes inside install-client-helm.bats (host proxy -> values.yaml carries split proxy keys) and was killed. That file is untouched by this PR — no shell library, installer, or template changed — but I did not confirm the hang is pre-existing, so Unit tests in CI is the evidence for that tier, not a local run.
• No Chart.yaml bump: scripts/chart-version-guard.sh:79 records tests/** as deliberately not chart content, and no template changed in the final diff.

Docs kept true

• .cursor/BUGBOT.md gains a fourth shape of vacuous test alongside the existing three: an assertion key the test runner never reads — the fixture reaches the code, the mutation is live, the spelling is right, and the runner ignores the key.
• The Makefile's drift comment described the list as "duplicated-declaration guards". That was already loose (the R8 manifest and cross-OS facts checks are house rules, not duplicated values) and this guard makes it looser, so the comment now states the real membership rule: a guard belongs there because it must block.

Test plan

• helm unittest ./client — 581/581, before and after
• Both errorMessage conversions mutation-proven red, then green on restore
• All 5 bare assertions proven to redden on a legal config
• Guard mutation-proven in both directions plus three fail-closed paths
• make check / make drift green

---

Note

Low Risk
Test and CI guard changes only; no chart templates or runtime behavior. Risk is limited to false positives in the new YAML key scanner if suite structure changes.

Overview
Helm-unittest 0.5.2 ignores errorPattern under failedTemplate, so seven refusal tests were green on any failure while looking pinned in review. This PR fixes those assertions and blocks the pattern from returning.

Template fail cases in rbac_test.yaml and secrets_test.yaml now use errorMessage with the exact template text. Schema-at-load rejections (five cases across auto-upgrade, image-refresh, network policy, priority class) stay failedTemplate: {} with comments explaining why message matching cannot work inside the plugin.

A new required drift guard, scripts/tests/helm-unittest-error-assertions.sh, parses */tests/*_test.yaml and allowlists only errorMessage under failedTemplate (not grep, because warning comments mention errorPattern). It is wired into MakefileDRIFT_GUARDS so Source-of-truth drift enforces it on develop/main.

.cursor/BUGBOT.md documents a fourth vacuous-test shape: assertion keys the runner never reads.

^{Reviewed by Cursor Bugbot for commit a61186c. Bugbot is set up for automated code reviews on this repo. Configure here.}

[LukasWodka]

CI note for the audit trail, not a silent dismissal.

Seal-check egress-enforcement (k3d) failed on the first run of this PR:

Error: INSTALLATION FAILED: execution error at (client/templates/resource-monitor-daemonset.yaml:69:10):
resourceMonitor is enabled but the metrics.k8s.io/v1beta1 API is not registered.

That is the metrics-server APIService not being registered yet in the freshly-created k3d cluster — a race in the job's own cluster bring-up, not something this diff can reach. This PR touches only helm-unittest suite files (never read by helm install), two Makefile comments plus one DRIFT_GUARDS entry, .cursor/BUGBOT.md, and a new guard script. No template, values file, chart default, or installer library changed.

Checked rather than asserted: the same job is success on the last 7 PR runs and on the latest develop push, and origin/develop moved only by #858 (PowerShell installer) since this branch was cut. Re-ran the failed job on the same commit — success. Rollup is now 47 SUCCESS / 4 skipped / 0 failed.

If it flakes the same way again on an unrelated PR it is worth its own ticket (wait on the metrics.k8s.io/v1beta1 APIService in e2e-seal-check.sh before installing); one occurrence is not enough to call it.

[saadqbal]

Seven assertions that read in review as pinned refusals and were equivalent to failedTemplate: {} is the worst kind of test bug, because the diff that introduces it looks more rigorous than the honest version. And secrets_test.yaml:326 carrying the comment "Assert the stable prefix via errorPattern, not the full message" is the detail that makes the case: someone chose it deliberately, believing it worked, and every reviewer since read the comment as evidence.

Three things I checked rather than took:

The allowlist-not-blocklist choice is load-bearing, and the reason is measured.errorPatternplus an invented bogusKeyThatDoesNotExist, under --strict, still passes 19/19 — so the plugin validates no key names under failedTemplate at all, and a typo fails exactly as silently as the key you happened to notice. Blocklisting errorPattern would have closed one instance of an open class.

The guard explains why it isn't a grep, which is the trap I'd have expected it to fall into: errorPattern is named on purpose in several test-file comments — including the ones explaining this very bug — so a grep would flag its own documentation. Choosing a parser over a pattern for that stated reason is the right call.

It states its own blind spot — the case where the template errors rather than fails needs a layer that can see stderr, and the comment says so instead of implying full coverage.

Finding the seventh occurrence by grep when the ticket's table said six is the difference between fixing the instances and fixing the class.

The .cursor/BUGBOT.md entry is the most valuable part of this PR and I want to be explicit about why. "Spelling is not evidence that anything reads it" is a genuinely new shape in that taxonomy — the fixture reaches the code, the mutation is live, the assertion is spelled right, and the runner ignores the key. And generalising it to "flag any new assertion key on a plugin/tool whose honoured set was not checked" makes it reusable beyond helm-unittest. I've twice this week said a recurring finding deserved a rule in that file and not written one; doing it in the same PR as the fix is the standard.

Green, no threads. 👍