A bare `!` in a bats body asserts nothing unless it is the last command: 57 dead assertions in 8 repositories #19

New issue

Closed

opened 2026-09-28 05:24:35 -03:00 by marcos-mendez · 2 comments

marcos-mendez commented

2026-09-28 05:24:35 -03:00

(Migrated from github.com)

A bats test body has no assertions of its own: its verdict is the exit status of the last command it ran. Bash does not apply errexit to a negated command, so ! cmd on its own line decides the test when it is the final command of the body and is inert everywhere else. The line looks like a refutation either way, and which one it is depends only on where it sits.

Measured 2026-09-28 over every default branch in the organization that carries a bats file (19 of them):

shellcheck -f gcc --shell=bash <file>.bats | grep SC2314

57 inert negations in 8 repositories, beside 45 that are in final position and do assert. shellcheck grades the two apart, error: for the inert ones and note: for the rest, which is what makes the sweep exact.

Repository	File	Inert	Also converted (final position)
inithooks	tests/test-ipconfig.bats	17	6
inithooks	tests/test-init-fence.bats	3	2
inithooks	tests/test-tagid.bats	2	0
inithooks	tests/test-run.bats	0	1
keel-wordpress	tests/boot-test.bats	10	6
keel-wordpress	tests/hook.bats	0	1
.github	tests/boot-test-nodes.bats	9	3
keel-nodebb	tests/nodebb.bats	4	3
keel-nodebb	tests/nginx.bats	2	3
keel-nodebb	tests/hook.bats	0	2
unit-redis	tests/conf.bats	3	2
unit-redis	tests/unit.bats	1	2
unit-redis	tests/hook.bats	0	2
keel-mariadb	tests/boot-test.bats	3	1
keel-mariadb	tests/hook.bats	0	1
apt	tests/build-package.bats	2	1
common	tests/postfix-local.bats	1	2

Clean, confirmed rather than assumed: keel-postgresql, keel-transition, unit-mariadb, unit-postgresql, keel-core, keel, keel-redis, keel-lamp, keel-lapp, keel-apache-php, tkldev. keel-redis and keel-lamp were already written run ! throughout, which is where the form comes from.

The worst of them

inithooks tests/test-ipconfig.bats, "ip6_syntax rejects what is not an IPv6 address": thirteen rejection cases, twelve inert, only the last deciding anything. An IPv6 validator's rejection set, in an IPv6-first distribution, in the repository whose hook writes the address.

Closely followed by keel-wordpress, where five of the six cases of "is_global_ipv6 refuses link local, loopback, multicast and IPv4" never ran, and unit-redis, where the refutation that the bind fragment never says localhost — the one put there by the localhost entry in docs/traps.md — was itself inert.

What was found by turning them on

Every predicate turned out to answer as its test believed, so nothing shipped broken. That is luck, not evidence.

One newly live assertion failed: keel-nodebb tests/nodebb.bats, "proxy_conf without an address trusts nobody", asserted ! grep -q set_real_ip_from unanchored against a file whose own comment names the directive. It could never have passed, for any input, and nothing had ever run it. Anchored to ^set_real_ip_from, which is the property meant.

The fix and the gate

run ! cmd, with bats_require_minimum_version 1.5.0, which asserts wherever it stands. Every negation is converted, not only the inert ones: a file that mixes the two forms leaves a reader to work out which half is real.

The gate is one step in test-shell.yml of the .github repository, Negations that assert (SC2314), rather than a copy in each repository: every repository with a bats suite already calls that workflow at @main, and "copying a test library instead of sharing it" is already in docs/traps.md as a mistake this project made.

Merge order matters. The .github pull request should merge last. It is @main, so the moment it lands every repository that still has an inert negation goes red.

The pull requests

One per repository, each closing an issue in that repository. This issue is only the coordination.

A bats test body has no assertions of its own: its verdict is the exit status of the last command it ran. Bash does not apply errexit to a negated command, so `! cmd` on its own line decides the test when it is the final command of the body and is **inert everywhere else**. The line looks like a refutation either way, and which one it is depends only on where it sits. Measured 2026-09-28 over every default branch in the organization that carries a bats file (19 of them): shellcheck -f gcc --shell=bash <file>.bats | grep SC2314 **57 inert negations in 8 repositories**, beside 45 that are in final position and do assert. shellcheck grades the two apart, `error:` for the inert ones and `note:` for the rest, which is what makes the sweep exact. | Repository | File | Inert | Also converted (final position) | |---|---|---|---| | inithooks | tests/test-ipconfig.bats | 17 | 6 | | inithooks | tests/test-init-fence.bats | 3 | 2 | | inithooks | tests/test-tagid.bats | 2 | 0 | | inithooks | tests/test-run.bats | 0 | 1 | | keel-wordpress | tests/boot-test.bats | 10 | 6 | | keel-wordpress | tests/hook.bats | 0 | 1 | | .github | tests/boot-test-nodes.bats | 9 | 3 | | keel-nodebb | tests/nodebb.bats | 4 | 3 | | keel-nodebb | tests/nginx.bats | 2 | 3 | | keel-nodebb | tests/hook.bats | 0 | 2 | | unit-redis | tests/conf.bats | 3 | 2 | | unit-redis | tests/unit.bats | 1 | 2 | | unit-redis | tests/hook.bats | 0 | 2 | | keel-mariadb | tests/boot-test.bats | 3 | 1 | | keel-mariadb | tests/hook.bats | 0 | 1 | | apt | tests/build-package.bats | 2 | 1 | | common | tests/postfix-local.bats | 1 | 2 | Clean, confirmed rather than assumed: `keel-postgresql`, `keel-transition`, `unit-mariadb`, `unit-postgresql`, `keel-core`, `keel`, `keel-redis`, `keel-lamp`, `keel-lapp`, `keel-apache-php`, `tkldev`. `keel-redis` and `keel-lamp` were already written `run !` throughout, which is where the form comes from. ## The worst of them `inithooks` `tests/test-ipconfig.bats`, "ip6_syntax rejects what is not an IPv6 address": thirteen rejection cases, twelve inert, only the last deciding anything. An IPv6 validator's rejection set, in an IPv6-first distribution, in the repository whose hook writes the address. Closely followed by `keel-wordpress`, where five of the six cases of "is_global_ipv6 refuses link local, loopback, multicast and IPv4" never ran, and `unit-redis`, where the refutation that the bind fragment never says `localhost` — the one put there by the `localhost` entry in `docs/traps.md` — was itself inert. ## What was found by turning them on Every predicate turned out to answer as its test believed, so nothing shipped broken. That is luck, not evidence. One newly live assertion failed: `keel-nodebb` `tests/nodebb.bats`, "proxy_conf without an address trusts nobody", asserted `! grep -q set_real_ip_from` unanchored against a file whose own comment names the directive. It could never have passed, for any input, and nothing had ever run it. Anchored to `^set_real_ip_from`, which is the property meant. ## The fix and the gate `run ! cmd`, with `bats_require_minimum_version 1.5.0`, which asserts wherever it stands. Every negation is converted, not only the inert ones: a file that mixes the two forms leaves a reader to work out which half is real. The gate is one step in `test-shell.yml` of the `.github` repository, `Negations that assert (SC2314)`, rather than a copy in each repository: every repository with a bats suite already calls that workflow at `@main`, and "copying a test library instead of sharing it" is already in `docs/traps.md` as a mistake this project made. **Merge order matters.** The `.github` pull request should merge last. It is `@main`, so the moment it lands every repository that still has an inert negation goes red. ## The pull requests One per repository, each closing an issue in that repository. This issue is only the coordination.

marcos-mendez commented

2026-09-28 05:34:36 -03:00

(Migrated from github.com)

The pull requests, one per repository, each closing an issue there.

Repository	Issue	Pull request	Base	Inert fixed
inithooks	Keel-Linux/inithooks#15	Keel-Linux/inithooks#16	master	22
keel-wordpress	Keel-Linux/keel-wordpress#10	Keel-Linux/keel-wordpress#11	master	10
keel-nodebb	Keel-Linux/keel-nodebb#15	Keel-Linux/keel-nodebb#16	main	6
unit-redis	Keel-Linux/unit-redis#4	Keel-Linux/unit-redis#5	main	4
keel-mariadb	Keel-Linux/keel-mariadb#18	Keel-Linux/keel-mariadb#19	main	3
apt	Keel-Linux/apt#16	Keel-Linux/apt#17	main	2
common	Keel-Linux/common#9	Keel-Linux/common#10	19.x	1
.github	Keel-Linux/.github#13	Keel-Linux/.github#14	main	9, plus the gate
handbook	Keel-Linux/handbook#15	Keel-Linux/handbook#16	main	the `docs/traps.md` entry

Keel-Linux/.github#14 merges last. It is @main, so the moment it lands every repository that still has an inert negation goes red.

One newly live assertion failed, in keel-nodebb: ! grep -q set_real_ip_from, unanchored, against a file whose own comment names the directive. It could never have passed for any input. Anchored in Keel-Linux/keel-nodebb#16.

Two gaps left open, neither in scope here: keel-lamp and keel-lapp have bats suites and no workflows at all, so the gate will not reach them until they call it; and common's COVERAGE.md sentence about these three negations also exists on the unmerged Keel-Linux/common#8, where it must be dropped on rebase.

The pull requests, one per repository, each closing an issue there. | Repository | Issue | Pull request | Base | Inert fixed | |---|---|---|---|---| | inithooks | Keel-Linux/inithooks#15 | Keel-Linux/inithooks#16 | master | 22 | | keel-wordpress | Keel-Linux/keel-wordpress#10 | Keel-Linux/keel-wordpress#11 | master | 10 | | keel-nodebb | Keel-Linux/keel-nodebb#15 | Keel-Linux/keel-nodebb#16 | main | 6 | | unit-redis | Keel-Linux/unit-redis#4 | Keel-Linux/unit-redis#5 | main | 4 | | keel-mariadb | Keel-Linux/keel-mariadb#18 | Keel-Linux/keel-mariadb#19 | main | 3 | | apt | Keel-Linux/apt#16 | Keel-Linux/apt#17 | main | 2 | | common | Keel-Linux/common#9 | Keel-Linux/common#10 | 19.x | 1 | | .github | Keel-Linux/.github#13 | Keel-Linux/.github#14 | main | 9, plus the gate | | handbook | Keel-Linux/handbook#15 | Keel-Linux/handbook#16 | main | the `docs/traps.md` entry | **Keel-Linux/.github#14 merges last.** It is `@main`, so the moment it lands every repository that still has an inert negation goes red. One newly live assertion failed, in keel-nodebb: `! grep -q set_real_ip_from`, unanchored, against a file whose own comment names the directive. It could never have passed for any input. Anchored in Keel-Linux/keel-nodebb#16. Two gaps left open, neither in scope here: `keel-lamp` and `keel-lapp` have bats suites and no workflows at all, so the gate will not reach them until they call it; and `common`'s `COVERAGE.md` sentence about these three negations also exists on the unmerged Keel-Linux/common#8, where it must be dropped on rebase.

marcos-mendez commented

2026-09-29 01:45:34 -03:00

(Migrated from github.com)

Fixed in the nine repositories (57 negations) and kept out by the gate in Keel-Linux/.github#14.