fleetd #528: pin drain_gate_refusal's call site, not just the predicate
drain_gate_refusal composes the correct abort message and is well tested, but the main flow built its own `die "$(drain_gate_refusal ...)"` call — nothing proved that call site was ever consulted. Mutating it to a flat `die "aborted -- nothing changed"` left the whole suite green, silently reinstating the exact defect #517 was filed to fix. Same shape as #521/#526's should_swap/swap_if_built: the decision and the die() now live together in refuse_drain_gate, which the main flow calls unconditionally. drain_gate_refusal stays separate and separately tested for the message logic; four new behavioural tests stub die() to prove refuse_drain_gate calls it correctly for all four cases, and a fifth source-text test pins the main flow's call site itself (the only thing that can catch deleting the call, since sourcing stops before the main flow runs).
This commit is contained in:
@@ -519,6 +519,32 @@ drain_gate_refusal() {
|
||||
fi
|
||||
}
|
||||
|
||||
# fleetd #528 — drain_gate_refusal above is well tested (four cases, all direct), but nothing made
|
||||
# the MAIN FLOW's abort actually consult it. Before this, the main flow read
|
||||
# `die "$(drain_gate_refusal "$DO_BUILD" "$JAR_STAGED")"` directly, and mutating that one line to a
|
||||
# flat `die "aborted — nothing changed"` left the whole suite at exit 0 with zero FAIL lines and
|
||||
# byte-identical output to a clean run — every one of drain_gate_refusal's own tests still passed,
|
||||
# because they call the predicate directly and never touch this call site. That silently reinstated
|
||||
# the exact defect #517 was filed to fix. Same shape as #521/#526's should_swap/swap_if_built: a
|
||||
# predicate alone is not enough, because a test proving the predicate is right cannot also prove the
|
||||
# main flow consults it. So the decision (drain_gate_refusal) and the action (die) now live together
|
||||
# in ONE function, and the main flow calls it unconditionally instead of building the die() call
|
||||
# itself — there is no guard left in the main flow to remove, invert, or bypass independently of this
|
||||
# function. drain_gate_refusal stays separate and separately tested because the message-selection
|
||||
# logic is worth naming and testing on its own; refuse_drain_gate is the only thing that ever dies.
|
||||
#
|
||||
# What the behavioural tests above still cannot pin on their own: deleting the call to this function
|
||||
# from the main flow altogether — they call refuse_drain_gate directly, never through the main flow,
|
||||
# because sourcing stops before the main flow ever runs (see the SOURCED guard below). That gap is
|
||||
# closed the same way swap_if_built's is: test_refuse_drain_gate_call_site_present greps this script
|
||||
# for the real invocation, the same shape test_swap_ordered_after_wait_and_before_start already uses
|
||||
# for the swap call. Deliberately NOT written out here as a literal quoted string, so this comment
|
||||
# itself can never become a second match for that test's needle.
|
||||
refuse_drain_gate() {
|
||||
local do_build="$1" staged_path="$2"
|
||||
die "$(drain_gate_refusal "$do_build" "$staged_path")"
|
||||
}
|
||||
|
||||
# CB-600: sourceable for testing. When this file is SOURCED (not executed) it stops here — nothing
|
||||
# below runs — so a test harness can `source` it to call check_log_path_matches_plist (or the
|
||||
# other pure helpers above) against a throwaway plist fixture without ever reaching the mutating
|
||||
@@ -677,10 +703,11 @@ if [ -n "$OLD_PID" ] && [ "$ASSUME_YES" = 0 ]; then
|
||||
echo
|
||||
read -r -p " Fleet drained? type yes to restart: " reply
|
||||
if [ "$reply" != "yes" ]; then
|
||||
# fleetd #493 / #517: "nothing changed" would be a lie once a build has run and staged a jar —
|
||||
# see drain_gate_refusal above for the full decision and why each of its four cases reads the
|
||||
# way it does.
|
||||
die "$(drain_gate_refusal "$DO_BUILD" "$JAR_STAGED")"
|
||||
# fleetd #493 / #517 / #528: "nothing changed" would be a lie once a build has run and staged a
|
||||
# jar — see drain_gate_refusal above for the full decision and why each of its four cases reads
|
||||
# the way it does. refuse_drain_gate composes that message AND calls die itself, so this guard
|
||||
# has nothing left of its own to get wrong beyond whether it calls refuse_drain_gate at all.
|
||||
refuse_drain_gate "$DO_BUILD" "$JAR_STAGED"
|
||||
fi
|
||||
fi
|
||||
|
||||
|
||||
@@ -521,6 +521,106 @@ test_drain_gate_refusal_no_build_staged_absent() {
|
||||
assert_equals "aborted — nothing changed" "$result" "no-build+staged-absent refusal wording"
|
||||
}
|
||||
|
||||
# fleetd #528 — the four tests above pin drain_gate_refusal(), and that is ALL they pin: they call
|
||||
# the predicate directly and never touch the main flow's call site. That was measured to be not
|
||||
# enough, the same way test_should_swap_true_when_build_ran/test_should_swap_false_when_build_skipped
|
||||
# were not enough for #521: with the main flow reading `die "$(drain_gate_refusal "$DO_BUILD"
|
||||
# "$JAR_STAGED")"`, replacing that whole line with a flat `die "aborted — nothing changed"` left this
|
||||
# suite at exit 0 with zero FAIL lines and byte-identical output to a clean run. Nothing above could
|
||||
# tell the difference, because none of it calls anything at or above the call site itself.
|
||||
#
|
||||
# So these four call refuse_drain_gate() — the function the main flow actually calls, holding the
|
||||
# composed message and the die() together — with die() stubbed to RECORD whether it was called and
|
||||
# with what message, instead of exiting the process. That fails if refuse_drain_gate stops consulting
|
||||
# drain_gate_refusal, mangles what it passes it, or simply never calls die.
|
||||
#
|
||||
# What none of these four can catch: deleting the `refuse_drain_gate "$DO_BUILD" "$JAR_STAGED"` line
|
||||
# from the main flow altogether — see the comment above refuse_drain_gate in redeploy-fleetd.sh for
|
||||
# why no test in this file can do better than that (sourcing stops before the main flow runs).
|
||||
DIED_CALLED=0
|
||||
DIED_MESSAGE=""
|
||||
stub_die_recorder() {
|
||||
DIED_CALLED=0
|
||||
DIED_MESSAGE=""
|
||||
die() { DIED_CALLED=1; DIED_MESSAGE="$*"; }
|
||||
}
|
||||
|
||||
test_refuse_drain_gate_build_ran_staged_present() {
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
local dir staged
|
||||
dir="$TMP/refuse-drain-build-staged"; mkdir -p "$dir"
|
||||
staged="$dir/fleetd-new.jar"
|
||||
printf 'staged jar bytes' > "$staged"
|
||||
stub_die_recorder
|
||||
refuse_drain_gate 1 "$staged"
|
||||
[ "$DIED_CALLED" = 1 ] \
|
||||
|| fail "refuse_drain_gate build-ran+staged-present must call die, and did not"
|
||||
printf '%s' "$DIED_MESSAGE" | grep -qF "$staged" \
|
||||
|| fail "refuse_drain_gate build-ran+staged-present die message does not name the staged jar"
|
||||
printf '%s' "$DIED_MESSAGE" | grep -qF 'Rerun WITHOUT --no-build' \
|
||||
|| fail "refuse_drain_gate build-ran+staged-present die message is missing the rerun instruction"
|
||||
if printf '%s' "$DIED_MESSAGE" | grep -qF 'nothing changed'; then
|
||||
fail "refuse_drain_gate build-ran+staged-present must not claim nothing changed — the jar already moved"
|
||||
fi
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
}
|
||||
|
||||
test_refuse_drain_gate_build_ran_staged_absent() {
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
local dir
|
||||
dir="$TMP/refuse-drain-build-no-staged"; mkdir -p "$dir"
|
||||
stub_die_recorder
|
||||
refuse_drain_gate 1 "$dir/fleetd-new.jar"
|
||||
[ "$DIED_CALLED" = 1 ] \
|
||||
|| fail "refuse_drain_gate build-ran+staged-absent must call die, and did not"
|
||||
assert_equals "aborted — nothing changed" "$DIED_MESSAGE" "refuse_drain_gate build-ran+staged-absent die message"
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
}
|
||||
|
||||
test_refuse_drain_gate_no_build_staged_present() {
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
local dir staged
|
||||
dir="$TMP/refuse-drain-no-build-staged"; mkdir -p "$dir"
|
||||
staged="$dir/fleetd-new.jar"
|
||||
printf 'leftover staged jar bytes' > "$staged"
|
||||
stub_die_recorder
|
||||
refuse_drain_gate 0 "$staged"
|
||||
[ "$DIED_CALLED" = 1 ] \
|
||||
|| fail "refuse_drain_gate no-build+staged-present must call die, and did not"
|
||||
assert_equals "aborted — nothing changed" "$DIED_MESSAGE" "refuse_drain_gate no-build+staged-present die message"
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
}
|
||||
|
||||
test_refuse_drain_gate_no_build_staged_absent() {
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
local dir
|
||||
dir="$TMP/refuse-drain-no-build-no-staged"; mkdir -p "$dir"
|
||||
stub_die_recorder
|
||||
refuse_drain_gate 0 "$dir/fleetd-new.jar"
|
||||
[ "$DIED_CALLED" = 1 ] \
|
||||
|| fail "refuse_drain_gate no-build+staged-absent must call die, and did not"
|
||||
assert_equals "aborted — nothing changed" "$DIED_MESSAGE" "refuse_drain_gate no-build+staged-absent die message"
|
||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||
}
|
||||
|
||||
# fleetd #528 — closes the one gap the four behavioural tests above cannot: they call
|
||||
# refuse_drain_gate directly, and sourcing stops before the main flow ever runs (the SOURCED guard),
|
||||
# so none of them can prove the main flow still CALLS refuse_drain_gate at all. Same shape as
|
||||
# test_swap_ordered_after_wait_and_before_start: a source-text grep for the real call site. This is
|
||||
# what actually kills the item-1 mutation from the ticket — replacing the main flow's call with a
|
||||
# flat `die "aborted — nothing changed"` removes this exact needle, where none of the behavioural
|
||||
# tests above would even notice.
|
||||
#
|
||||
# The grep ends `|| true`: this file runs under `set -euo pipefail`, so an ABSENT needle would fail
|
||||
# the assignment and `set -e` would kill the whole suite before the `[ -n ... ] || fail` guard below
|
||||
# ever ran — the exact dead-check shape fleetd #528 also flags as a sweep finding (see the PR body).
|
||||
test_refuse_drain_gate_call_site_present() {
|
||||
local src="$ROOT/scripts/redeploy-fleetd.sh" call_line
|
||||
call_line="$(grep -Fn 'refuse_drain_gate "$DO_BUILD" "$JAR_STAGED"' "$src" | head -1 | cut -d: -f1 || true)"
|
||||
[ -n "$call_line" ] \
|
||||
|| fail "could not find the main flow's refuse_drain_gate call site in redeploy-fleetd.sh"
|
||||
}
|
||||
|
||||
test_no_errors() {
|
||||
cat > "$TMP/no-errors.log" <<'LOG'
|
||||
2026-09-05 12:00:00 INFO fleetd listening
|
||||
@@ -753,6 +853,11 @@ test_drain_gate_refusal_build_ran_staged_present
|
||||
test_drain_gate_refusal_build_ran_staged_absent
|
||||
test_drain_gate_refusal_no_build_staged_present
|
||||
test_drain_gate_refusal_no_build_staged_absent
|
||||
test_refuse_drain_gate_build_ran_staged_present
|
||||
test_refuse_drain_gate_build_ran_staged_absent
|
||||
test_refuse_drain_gate_no_build_staged_present
|
||||
test_refuse_drain_gate_no_build_staged_absent
|
||||
test_refuse_drain_gate_call_site_present
|
||||
test_no_errors
|
||||
test_recovery_patterns_match_source
|
||||
test_attributed_recovered_connection_error
|
||||
|
||||
Reference in New Issue
Block a user