test(tests): make negated BATS assertions able to fail - #3790

What this PR does

A hundred and twelve assertions across twelve BATS suites begin with !. A hundred and seven of them pass no matter what they find. POSIX exempts a command whose return value is inverted with ! from set -e, so the failing status is dropped where it is produced, and hack/cozytest.sh closes each test function with return 0, so it never resurfaces as the test's own status either. The other five are followed by || { ... exit 1; }, which reads the inverted status and does work; they are rewritten too, because telling the two apart needs the tail of every statement parsed, and a guard that guesses wrong there is green on a real one.

Switching to the bats binary does not fix the hundred and seven. Its ERR trap carries the identical exemption: bash -c 'set -eE; trap "echo TRAP" ERR; ! true; echo REACHED' prints REACHED, runs no trap and exits zero. The only negated assertion bats reports is one standing last in the test body, where the inverted status happens to be what the function returns. Two of the hundred and seven stood last; the rest did not.

What was lost is specific rather than diffuse. ! is how you spell "this must not appear", and an absence is what no other assertion in a test covers. The affected suites assert that a migration ran nothing destructive, that no image was copied, that a flag was not passed, that a backup cleanup never flushed the master, that an object does not exist. On that half of their contract they held for any input.

Each site is now written as if cmd; then echo "FAIL: ..."; false; fi, the form already used in the files where this had been noticed one at a time, and each carries a message naming what must not have happened. hack/bats-no-negated-assert.bats scans every hack/**/*.bats and refuses the old form, so a new one cannot be added silently. Inverted conditions (if !, while !, until !) are untouched, because set -e is not what decides a condition. [ ! -f x ] is untouched for a different reason and remains an ordinary failing assertion: the ! there is an operator of test, not negation of a command, so nothing exempts its status.

The guard pins its own premise instead of describing it, on both sides. One test asserts the runner still passes a bare negated assertion; a second, that neither set -e nor the ERR trap sees an inverted status; a third, that the prescribed if cmd; then ...; false; fi does fail, under both runners, and at the form rather than somewhere before it, since a run that died early would satisfy a bare status check and pin nothing.

Every rewrite here rests on the third test. If false inside a then body ever stopped propagating, the first test would still pass, because a negated assertion would still be vacuous, while every converted absence went silently green. A ban that prescribes a replacement has to test the replacement too.

Every claim the guard makes is pinned by removing the thing it claims. The recursion is exercised through the audit function against a fixture in a subdirectory, not through a second copy of its find expression: a copy keeps passing while the function loses its recursion, and an audit that returns empty reads as a clean tree. Recursion matters because BATS_UNIT_FILES is a non-recursive wildcard, so make unit-tests never runs a suite placed in a subdirectory. hack/e2e-apps/ held such suites until they moved to Chainsaw and is gone now, which is why the test pins against a fixture and not a real directory.

The rule's surface was checked in the other direction too. The shape appears in no hack/**/*.sh script as an assertion: the one hit, hack/testdata/install-wait/call-shapes.sh, is fixture text that hack/run-kubernetes-install-wait_test.bats parses, not a command anything runs. It appears in no Chainsaw script: step either, so .bats is the whole surface rather than the part that was convenient to guard.

The scan is lexical, so its header names the statement boundaries it owns rather than implying it understands shell: ; & && || { (, the ) closing a case pattern, and the words then do else, each measured against the runner before being added. A keyword may legally be preceded only by whitespace or by ;, since every other separator in front of then, do or else is a shell syntax error, checked with sh -n rather than recalled. The scan therefore admits exactly those two, and that part of the class is closed rather than patched one spelling at a time. All three keywords are vacuous after either spelling, so missing one is a silent miss, the single direction this scan must not err in, given that its whole design trades noise for safety. For the same reason it takes !(cmd) with no space before the subshell, which is vacuous on dash and bash alike, and reads through a time or time -p prefix, which is vacuous under bats and under cozytest wherever /bin/sh is bash.

Five over-counts remain and are deliberate, each named in the header: ( ! cmd ) does propagate its inverted status and is reported anyway; so would a ! opening a statement after a redirection &; x=$(true || ! false) is reported although the || reads the inverted status; a condition continued by a trailing && instead of a backslash is reported, because the strip only recognises a condition whose end it can see; and time ! cmd is reported although on a dash /bin/sh, which has no time keyword, it runs an external time that fails. Each costs one rewritten line, where the opposite error costs a silent pass. They are enumerated rather than engineered away on purpose, because a removed error leaves no trace while a listed one tells the next reader what they are looking at. The scan also errs quiet in at least two places, named here rather than in the header. A file cut off midway through a line continuation loses its last buffered statement, but that input is not a runnable test, since sh -n rejects it with unexpected end of file. And time -- ! cmd, which bash accepts, is vacuous under bats and not peeled by the scan; nothing in the tree uses it, and I stopped adding spellings there rather than chase every prefix bash allows. The test naming the shapes claims only what its fixture holds. A lexical scan over shell cannot promise it has enumerated the language, and a name that promised it would be false whether or not a gap existed today.

The branch name is narrower than the subject and I am leaving it as it is rather than churn it. This is not a defect in hack/cozytest.sh that moving to the bats binary would fix. It is a POSIX rule both runners inherit, and moving to bats would close two of the hundred and seven sites.

One suite went red for real once its assertions could fail. seaweedfs-guard-parity forbids four mutable discriminators by grepping each chart, and both charts carry a template comment block arguing why those signals were rejected, so the grep matched the argument and would have reported a chart for documenting the rule it follows. The charts are correct; the assertion was not. It now strips {{/* ... */}} before matching. Two details there are load-bearing and neither is obvious. The strip removes the comment span, not the line that opens it: dropping the line is shorter and silently loses a discriminator sharing a line with a comment, which is a chart classifying on a forbidden signal and a test reporting that it does not. And the strip reports when the file ended inside a comment, and the checks refuse to read its output when it did, because every {{/* is read as an opener, so one inside a quoted string (these charts carry long fail (printf ...) messages) would swallow the rest of the file and leave the checks reading nothing.

One commit widens six comments that explained this trap as applying to a "negated pipeline". It is not about pipelines, and the narrower wording invites the conclusion that the other shapes are safe. Comment text only; each of those files already uses the working form.

The last commit makes the one absence check in hack/e2e-install-cozystack.bats that already used the if form tell a missing object from a failed read. It asserted that the version configmap was gone on kubectl get's exit status alone, and that status is 1 both for a NotFound and for an api-server the test cannot reach.

Verification

Every unit suite this branch touches was run through hack/cozytest.sh on the current base and is green. migration-seaweedfs-db-adopt passes 15 of 15 on macOS only when its temp directory sits under $HOME. Its container cannot write into a bind mount under /var/folders, and /usr/bin/mktemp -d ignores TMPDIR, so the run needs GNU mktemp first on PATH as well. Measured on Linux before the rebase, it passed 15 of 15 as is.

The six sites that reached main after this branch was first written, in redis-backup-strategy and proxmox-e2e-fixture_test, were each shown to be able to fail: with the condition inverted, the suite goes red at that assertion and prints its own FAIL: message. The nine sites in migration-seaweedfs-db-adopt carry a two-sided check as well: with the pattern swapped for one the same test proves is present, the rewritten form fails the suite and the original ! form passes it.

The guard itself is green under cozytest with bash and with dash as /bin/sh, under the bats binary, and with mawk, gawk and BSD awk. Each branch of its separator class and of the time and !(cmd) handling was removed in turn, and each removal turns the shapes test red.

e2e-install-cozystack.bats is an e2e suite and runs only in the E2E workflow, so its one changed absence check is not exercised locally.

Screenshots

Not a UI change.

Downstream repositories

Walked the trigger map against the diff file by file. The change touches eighteen existing BATS files, adds one, and adds a convention to docs/agents/e2e-testing.md. It does not touch hack/e2e-prepare-cluster.bats, which is the node contract ansible-cozystack and talm restate; it moves and renames nothing under hack/ and changes no make target's behaviour, which is what ccp gates on; and it touches no package, schema, enum, default, CRD, label, annotation or metric.

• 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:
• cozystack/community - follow-up:

Release note

NONE

Closes #3565