From 23dc85e8ed629581c065311adc529daa24f57086 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marcos=20M=C3=A9ndez?= Date: Mon, 28 Sep 2026 07:38:56 +0000 Subject: [PATCH 1/2] test: a negated command only asserts in final position `! cmd` on its own line asserts nothing in a bats body unless it happens to be the last command of that body: bash does not apply errexit to a negated command, so the verdict is the exit status of the final command and every earlier `! cmd` is inert. shellcheck names the class SC2314 and grades the two cases apart, error for the inert ones and note for the rest: shellcheck -f gcc --shell=bash tests/*.bats | grep SC2314 `tests/postfix-local.bats` has three bare negations. One is inert, line 96, `! logged smtpd_banner` in "stops at the first failing postconf": that test would pass on a script that carried on past the failing postconf and wrote the banner anyway. The other two, lines 66 and 97, are last in their bodies and do assert. All three become `run !`, the form that asserts wherever it stands, including the two that already did: a line whose meaning depends on its position is the trap itself. That form needs bats 1.5.0, declared at the top of the file as the other suites in the organization declare it. No assertion changed its verdict; the suite is green with the inert one live. Coverage unchanged: postfix-local 100.00 percent, 17 of 17 lines, threshold 100. COVERAGE.md records the rule, and corrects a statement made on fix/keel-apt-identity that all three negations were inert. --- COVERAGE.md | 10 ++++++++++ tests/postfix-local.bats | 8 +++++--- 2 files changed, 15 insertions(+), 3 deletions(-) diff --git a/COVERAGE.md b/COVERAGE.md index 0895e226..5da74239 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -12,6 +12,16 @@ Pull request #2 merged on 2026-09-26 (merge commit 5a0a381) and brought down, and is only ever raised. The sections that follow record the state before the merge. +A refutation is written `run ! cmd`, never a bare `! cmd`: bash does not +apply errexit to a negated command, so a bare one is inert unless it happens +to be the last command of the test body, where it alone decides the verdict. +shellcheck names the class SC2314 and grades the two cases apart, error for +the inert ones and note for the rest. `tests/postfix-local.bats` had three +bare negations: line 96 was inert and lines 66 and 97 did assert, because +they were last. All three are `run !` now, so none of them depends on its +position, and the check runs for every repository in the reusable +`test-shell` workflow. + ## Baseline before the merge: 0 percent, nothing measured This repository has no test suite and no coverage tool wired up, so nothing diff --git a/tests/postfix-local.bats b/tests/postfix-local.bats index 1c2cb065..39dcdb14 100644 --- a/tests/postfix-local.bats +++ b/tests/postfix-local.bats @@ -5,6 +5,8 @@ # tests/stubs that record their arguments in STUB_LOG; the script never # reaches the real postfix or systemd. +bats_require_minimum_version 1.5.0 + setup() { TESTS_DIR="$(cd "$(dirname "$BATS_TEST_FILENAME")" && pwd)" SCRIPT="$TESTS_DIR/../conf/turnkey.d/postfix-local" @@ -63,7 +65,7 @@ LISTEN 0 100 [2001:db8::25]:25 [::]:* users:((\"master\",pi run "$SCRIPT" [ "$status" -eq 1 ] [[ "$output" == *"Port 25 is already in use"* ]] - ! logged postconf + run ! logged postconf } @test "configures postfix when port 25 is free" { @@ -93,6 +95,6 @@ EXPECTED run "$SCRIPT" [ "$status" -eq 1 ] logged "postconf -e myhostname=mail.example.com" - ! logged smtpd_banner - ! logged postmulti + run ! logged smtpd_banner + run ! logged postmulti } From f858258f7bdef81edac6441f91e3c1133426864f Mon Sep 17 00:00:00 2001 From: navigator Date: Tue, 29 Sep 2026 03:55:28 +0000 Subject: [PATCH 2/2] docs: one paragraph on refutations in COVERAGE.md, not two #8 landed on 19.x with its own paragraph on bare negations, which said the inert one in tests/postfix-local.bats was left for the pull request that owns that file. This is that pull request, and the merge kept both paragraphs. The first one now ends with what this branch did, and the second is gone. --- COVERAGE.md | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/COVERAGE.md b/COVERAGE.md index 8940ba1a..48dbaf36 100644 --- a/COVERAGE.md +++ b/COVERAGE.md @@ -53,15 +53,7 @@ not the last command of its test passes whatever happens; as the last command it does decide the test, because bats takes the last status as the verdict. `run !` asserts wherever it stands, which is why it is the convention. shellcheck grades the two cases differently: SC2314 is an error -for the inert one and a note otherwise. Of the three bare negations in -`tests/postfix-local.bats`, only line 96 is inert; lines 66 and 97 are last -in their tests. It is left for the pull request that owns that file. - -A refutation is written `run ! cmd`, never a bare `! cmd`: bash does not -apply errexit to a negated command, so a bare one is inert unless it happens -to be the last command of the test body, where it alone decides the verdict. -shellcheck names the class SC2314 and grades the two cases apart, error for -the inert ones and note for the rest. `tests/postfix-local.bats` had three +for the inert one and a note otherwise. `tests/postfix-local.bats` had three bare negations: line 96 was inert and lines 66 and 97 did assert, because they were last. All three are `run !` now, so none of them depends on its position, and the check runs for every repository in the reusable