A source-text test pins what a message SAYS and never whether it is REACHED — the drain-gate abort branch can be disabled with the suite green #517

Closed
opened 2026-09-12 06:01:07 +02:00 by ltms · 1 comment
Owner

Found by mutating scripts/redeploy-fleetd.sh while adjudicating #514 (merged, fixes #511). Two survivors. The first is a general shape worth naming; the second is one line.

Harness proof first, so the green results below can be read: on the same tree, putting the old wrong abort wording back makes the suite go red with FAIL: abort message still claims a rerun WITH --no-build can finish the restart, exit 1. The suite can fail. It just does not fail for either mutation below.

1. The abort-branch condition is not pinned, and by construction cannot be

#514 added a test for the drain-gate abort message. It is a source-text test: it greps scripts/redeploy-fleetd.sh for the wording. That was a reasonable choice and the worker flagged it — the drain gate sits in the script's main flow, which sourcing never reaches, so there is no way to call it.

The consequence is the finding. I replaced the branch condition:

scripts/redeploy-fleetd.sh:616
-    if [ "$DO_BUILD" = 1 ] && [ -f "$JAR_STAGED" ]; then
+    if false; then

Proven applied: grep -n '^ if false; then' → line 616; grep -n 'DO_BUILD" = 1 \] && \[ -f' → no match; and I re-read lines 616-620 to confirm rather than trusting the greps.

bash scripts/test-redeploy-fleetd.sh → exit 0, no test failed.

The message text is still in the file, so the grep still finds it. Nothing notices that it can never be printed. What an operator now sees when they abort the drain gate after a build is the fall-through:

aborted — nothing changed

Which is the exact lie #493 objected to and #510 was written to remove. The staged jar has already moved off the live path; something did change.

The general shape

A source-text test pins what a message says. It can never pin whether the message is reached.

This is the vacuous-test family again — a test that stays green when the behaviour is absent — but the mechanism is new. The four we have:

  1. barrier by hope — a sleep standing in for a wait
  2. a precondition never established, plus a negative assertion
  3. an expectation computed the way the implementation computes it
  4. an instrument shared by two tests that lies identically to both (#507)
  5. a source-text assertion, where the text can survive its own branch being dead ← this one

The tell is that the test's subject is the file rather than the behaviour. It cannot distinguish live code from a comment.

The fix, and it already has a precedent in this file

Do what #510 did for the wait loop. Its own comment says it best:

wait_for_daemon_exit(timeout) — the existing wait loop, extracted so ordering is checkable.

Extract the abort decision the same way. Something like drain_gate_refusal(reply, do_build, staged_path) that returns or prints the refusal, with the main flow calling it and dying on the result. Then the suite can call it directly with the four combinations that matter:

  • build ran, staged jar present → the message naming the staged jar
  • build ran, staged jar absent → "nothing changed"
  • --no-build, staged jar present → decide deliberately which message is right here and say so in a comment
  • --no-build, staged jar absent → "nothing changed"

Keep the existing source-text test as well. The two catch different regressions: the extracted test catches a dead branch, the grep catches a re-wording. Neither replaces the other.

2. jar_id's "absent" branch is unpinned

Smaller, and one line.

scripts/redeploy-fleetd.sh:129
-  jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && shasum -a 256 "$f" | cut -c1-12 || echo "absent"; }
+  jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && shasum -a 256 "$f" | cut -c1-12 || echo "present"; }

Proven applied with two different search strings plus a re-read of line 129. Suite → exit 0, no test failed.

#514 added a test for jar_id and it pins both halves of the present-file contract — the no-argument default and the explicit path. Neither half exercises the missing-file path.

Not equivalent, and it matters more than a string usually would. absent is the word that tells an operator a mvn clean deleted the running daemon's jar — the #413 signal, and the redeploy-fleetd skill points them at --check for exactly that. Any other word turns that report into a false reassurance.

The fix

Extend the existing jar_id test: point JAR at a path that does not exist and assert the output is exactly absent.

Acceptance

  • bash scripts/test-redeploy-fleetd.sh exits 0, with the test count up by the number of tests added.
  • bash -n passes on both scripts.
  • Both mutations above must now be killed. Reproduce each one, show the suite going red and quote the FAIL line, restore, confirm byte-identical with shasum -a 256, then a green control run. Prove each mutation applied with two greps using different search strings and a grep -n re-read of the line itself — a mis-quoted search string silently reports zero and looks exactly like a mutation that did not apply.
  • Do not run scripts/redeploy-fleetd.sh against the live daemon, with any flag. It is the channel the fleet talks through.

Related

  • #511 / #514 — the change this was found in.
  • #493 — the ticket whose fix item 1 silently undoes.
  • #507 — the other new member of the vacuous-test family.
  • #512 — the third outstanding defect in this script.
Found by mutating `scripts/redeploy-fleetd.sh` while adjudicating #514 (merged, fixes #511). Two survivors. The first is a general shape worth naming; the second is one line. Harness proof first, so the green results below can be read: on the same tree, putting the old wrong abort wording back makes the suite go red with `FAIL: abort message still claims a rerun WITH --no-build can finish the restart`, exit 1. The suite can fail. It just does not fail for either mutation below. ## 1. The abort-branch condition is not pinned, and by construction cannot be #514 added a test for the drain-gate abort message. It is a **source-text** test: it greps `scripts/redeploy-fleetd.sh` for the wording. That was a reasonable choice and the worker flagged it — the drain gate sits in the script's main flow, which sourcing never reaches, so there is no way to call it. The consequence is the finding. I replaced the branch condition: ``` scripts/redeploy-fleetd.sh:616 - if [ "$DO_BUILD" = 1 ] && [ -f "$JAR_STAGED" ]; then + if false; then ``` Proven applied: `grep -n '^ if false; then'` → line 616; `grep -n 'DO_BUILD" = 1 \] && \[ -f'` → no match; and I re-read lines 616-620 to confirm rather than trusting the greps. `bash scripts/test-redeploy-fleetd.sh` → **exit 0**, no test failed. The message text is still in the file, so the grep still finds it. Nothing notices that it can never be printed. What an operator now sees when they abort the drain gate after a build is the fall-through: ``` aborted — nothing changed ``` Which is the exact lie #493 objected to and #510 was written to remove. The staged jar has already moved off the live path; something *did* change. ### The general shape **A source-text test pins what a message says. It can never pin whether the message is reached.** This is the vacuous-test family again — a test that stays green when the behaviour is absent — but the mechanism is new. The four we have: 1. barrier by hope — a `sleep` standing in for a wait 2. a precondition never established, plus a negative assertion 3. an expectation computed the way the implementation computes it 4. an instrument shared by two tests that lies identically to both (#507) 5. **a source-text assertion, where the text can survive its own branch being dead** ← this one The tell is that the test's subject is the *file* rather than the *behaviour*. It cannot distinguish live code from a comment. ### The fix, and it already has a precedent in this file Do what #510 did for the wait loop. Its own comment says it best: > `wait_for_daemon_exit(timeout)` — the existing wait loop, extracted so ordering is checkable. Extract the abort decision the same way. Something like `drain_gate_refusal(reply, do_build, staged_path)` that returns or prints the refusal, with the main flow calling it and dying on the result. Then the suite can call it directly with the four combinations that matter: - build ran, staged jar present → the message naming the staged jar - build ran, staged jar absent → "nothing changed" - `--no-build`, staged jar present → decide deliberately which message is right here and say so in a comment - `--no-build`, staged jar absent → "nothing changed" Keep the existing source-text test as well. The two catch different regressions: the extracted test catches a dead branch, the grep catches a re-wording. Neither replaces the other. ## 2. `jar_id`'s "absent" branch is unpinned Smaller, and one line. ``` scripts/redeploy-fleetd.sh:129 - jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && shasum -a 256 "$f" | cut -c1-12 || echo "absent"; } + jar_id() { local f="${1:-$JAR}"; [ -f "$f" ] && shasum -a 256 "$f" | cut -c1-12 || echo "present"; } ``` Proven applied with two different search strings plus a re-read of line 129. Suite → **exit 0**, no test failed. #514 added a test for `jar_id` and it pins both halves of the *present-file* contract — the no-argument default and the explicit path. Neither half exercises the missing-file path. Not equivalent, and it matters more than a string usually would. `absent` is the word that tells an operator a `mvn clean` deleted the running daemon's jar — the #413 signal, and the `redeploy-fleetd` skill points them at `--check` for exactly that. Any other word turns that report into a false reassurance. ### The fix Extend the existing `jar_id` test: point `JAR` at a path that does not exist and assert the output is exactly `absent`. ## Acceptance - `bash scripts/test-redeploy-fleetd.sh` exits 0, with the test count up by the number of tests added. - `bash -n` passes on both scripts. - **Both mutations above must now be killed.** Reproduce each one, show the suite going red and quote the FAIL line, restore, confirm byte-identical with `shasum -a 256`, then a green control run. Prove each mutation applied with two greps using *different* search strings **and** a `grep -n` re-read of the line itself — a mis-quoted search string silently reports zero and looks exactly like a mutation that did not apply. - Do **not** run `scripts/redeploy-fleetd.sh` against the live daemon, with any flag. It is the channel the fleet talks through. ## Related - #511 / #514 — the change this was found in. - #493 — the ticket whose fix item 1 silently undoes. - #507 — the other new member of the vacuous-test family. - #512 — the third outstanding defect in this script.
Author
Owner

Fixed by #520, merged. main is at c71ac23.

I ticked each item of this ticket's own fix section against the merged diff rather than against the
worker's report, because a brief can be narrower than the ticket and that is how #493 nearly closed
with half its fix undelivered.

this ticket asked for delivered how I checked
extract the abort decision so the suite can call it yes — drain_gate_refusal(do_build, staged_path), pure, prints only read the diff
test all four build/staged combinations yes — 4 new tests, one per cell read the diff; counted the new test_ functions
keep the existing source-text grep test yes, still at test-redeploy-fleetd.sh:399 grep
jar_id missing-file path returns absent yes — test_jar_id_reports_absent_for_missing_file, both the default and an explicit path read the diff

The worker dropped reply from the extracted signature I suggested. That is correct and better —
reply decides whether the refusal happens at all, not which message it is, so passing it in would
have put a second decision inside a function whose whole point is to make one decision testable.

Acceptance, measured by me on main at c71ac23

test functions defined   40      (grep -cE '^test_[a-zA-Z_]+\(\) \{')
test functions invoked   40      (grep -cE '^test_[a-zA-Z_]+$')
suite                    exit 0, zero real FAIL: lines
bash -n redeploy         rc=0 under /bin/bash 3.2.57  AND  bash 5.3.9
bash -n suite            rc=0 under both

Both mutations killed, reproduced by me, not taken from the report:

drain_gate_refusal condition -> `if false; then`   (proven at :474, re-read, original gone)
  exit=1   FAIL: build-ran+staged-present refusal does not name the staged jar path

jar_id  echo "absent" -> echo "present"            (proven at :129, re-read, original gone)
  exit=1   FAIL: jar_id with no arguments must report absent when $JAR does not exist:
           expected absent, got present

Restored byte-identical after each, 2cb83dc380c7226191d657c40fccdfc856904e40b2d03d0851fb6e522cee2f41,
control rerun exit 0.

The --no-build + staged-present cell reads "nothing changed", and I checked the reasoning rather
than accepting it: --no-build stages nothing itself, and a leftover staged jar is wiped by
rm -f "$JAR_STAGED" at :607, before the build at :610. So that run really did change nothing.

One thing this ticket produced that it did not ask for

The brief told the worker to look for the shape, not just the two instances — any branch whose
only test is a grep of the script's own source. It found one, and it is worse than either defect here:
the swap guard at :723 can be set to if false with the suite green, which is a redeploy that
reports success and never puts the new jar in place.

I reproduced that myself and filed it as #521. Closing this ticket; #521 carries it forward.

Fixed by #520, merged. `main` is at `c71ac23`. I ticked each item of this ticket's own fix section against the merged diff rather than against the worker's report, because a brief can be narrower than the ticket and that is how #493 nearly closed with half its fix undelivered. | this ticket asked for | delivered | how I checked | |---|---|---| | extract the abort decision so the suite can call it | yes — `drain_gate_refusal(do_build, staged_path)`, pure, prints only | read the diff | | test all four build/staged combinations | yes — 4 new tests, one per cell | read the diff; counted the new `test_` functions | | keep the existing source-text grep test | yes, still at `test-redeploy-fleetd.sh:399` | grep | | `jar_id` missing-file path returns `absent` | yes — `test_jar_id_reports_absent_for_missing_file`, both the default and an explicit path | read the diff | The worker dropped `reply` from the extracted signature I suggested. That is correct and better — `reply` decides whether the refusal happens at all, not which message it is, so passing it in would have put a second decision inside a function whose whole point is to make one decision testable. ## Acceptance, measured by me on `main` at `c71ac23` ``` test functions defined 40 (grep -cE '^test_[a-zA-Z_]+\(\) \{') test functions invoked 40 (grep -cE '^test_[a-zA-Z_]+$') suite exit 0, zero real FAIL: lines bash -n redeploy rc=0 under /bin/bash 3.2.57 AND bash 5.3.9 bash -n suite rc=0 under both ``` **Both mutations killed, reproduced by me, not taken from the report:** ``` drain_gate_refusal condition -> `if false; then` (proven at :474, re-read, original gone) exit=1 FAIL: build-ran+staged-present refusal does not name the staged jar path jar_id echo "absent" -> echo "present" (proven at :129, re-read, original gone) exit=1 FAIL: jar_id with no arguments must report absent when $JAR does not exist: expected absent, got present ``` Restored byte-identical after each, `2cb83dc380c7226191d657c40fccdfc856904e40b2d03d0851fb6e522cee2f41`, control rerun exit 0. The `--no-build` + staged-present cell reads "nothing changed", and I checked the reasoning rather than accepting it: `--no-build` stages nothing itself, and a leftover staged jar is wiped by `rm -f "$JAR_STAGED"` at `:607`, before the build at `:610`. So that run really did change nothing. ## One thing this ticket produced that it did not ask for The brief told the worker to look for **the shape**, not just the two instances — any branch whose only test is a grep of the script's own source. It found one, and it is worse than either defect here: the swap guard at `:723` can be set to `if false` with the suite green, which is a redeploy that reports success and never puts the new jar in place. I reproduced that myself and filed it as **#521**. Closing this ticket; #521 carries it forward.
ltms closed this issue 2026-09-12 06:30:42 +02:00
Sign in to join this conversation.
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: fleet/fleetd#517