From 3833d8e52b5429fa130ad00be195c917ee1fb652 Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 11:23:37 +0700 Subject: [PATCH] fleetd #517: pin the drain-gate abort branch and jar_id's absent case Two mutation-testing survivors in scripts/redeploy-fleetd.sh: a source-text test pins what a message SAYS but never whether the branch that prints it is REACHED. - Extract the drain-gate abort decision into drain_gate_refusal(do_build, staged_path), a pure function the suite can call directly for all four build/staged combinations. The existing source-text grep test is kept alongside it (it catches a re-wording; the new tests catch a dead branch). - Extend jar_id's test to cover the missing-file path (both the no-argument default and an explicit path), which the #511 test never exercised. --- scripts/redeploy-fleetd.sh | 43 +++++++++++++++----- scripts/test-redeploy-fleetd.sh | 71 +++++++++++++++++++++++++++++++++ 2 files changed, 104 insertions(+), 10 deletions(-) diff --git a/scripts/redeploy-fleetd.sh b/scripts/redeploy-fleetd.sh index a9067b9..36a423d 100755 --- a/scripts/redeploy-fleetd.sh +++ b/scripts/redeploy-fleetd.sh @@ -452,6 +452,35 @@ classify_amqp_connection_errors() { REDEPLOY_UNEXPLAINED_ERRORS=$((REDEPLOY_UNEXPLAINED_ERRORS + pending_inbox + pending_lead_mailbox)) } +# fleetd #517: extracted so the suite can call this decision directly, the same way #510 extracted +# wait_for_daemon_exit so its ordering became checkable. Before this, the only test of the drain-gate +# abort message was a grep of this script's own source for the wording — so mutating the `if` below +# to `if false` (making the branch unreachable) left every test green, because the wording was still +# sitting in the file. Pure: only decides which message applies and prints it, no side effects, so a +# test can call it directly with an in-memory staged path instead of driving the real drain-gate flow +# (which needs a live $OLD_PID and an interactive prompt neither test can supply). +# +# The four cases: +# build ran, staged jar present -> names the staged jar and how to finish or discard it +# build ran, staged jar absent -> "nothing changed" (nothing was staged this run either) +# --no-build, staged jar present -> ALSO "nothing changed", deliberately: --no-build itself builds +# and stages nothing (see require_no_build_jar above), so a staged jar found here is a leftover +# from an earlier, unrelated run. THIS run truly changed nothing, and the next DO_BUILD=1 run +# wipes that leftover before it builds (`rm -f "$JAR_STAGED"` in the build section above) — so +# there is nothing here for the operator to lose track of. +# --no-build, staged jar absent -> "nothing changed" +drain_gate_refusal() { + local do_build="$1" staged_path="$2" + if [ "$do_build" = 1 ] && [ -f "$staged_path" ]; then + printf 'aborted — the running daemon was NOT touched, but the freshly built jar is sitting at + %s, not yet swapped into %s. Rerun WITHOUT --no-build to finish the restart — + the freshly built jar is no longer at the live path that --no-build requires — or + remove %s by hand if you want to discard this build.' "$staged_path" "$JAR" "$staged_path" + else + printf 'aborted — nothing changed' + fi +} + # 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 @@ -610,16 +639,10 @@ 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: "nothing changed" would be a lie once a build has run — the freshly built jar - # already moved to $JAR_STAGED (stage_built_jar, above), so the live path has one fewer file - # than before this run started, even though the running daemon itself was never touched. - if [ "$DO_BUILD" = 1 ] && [ -f "$JAR_STAGED" ]; then - die "aborted — the running daemon was NOT touched, but the freshly built jar is sitting at - $JAR_STAGED, not yet swapped into $JAR. Rerun WITHOUT --no-build to finish the restart — - the freshly built jar is no longer at the live path that --no-build requires — or - remove $JAR_STAGED by hand if you want to discard this build." - fi - die "aborted — nothing changed" + # 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")" fi fi diff --git a/scripts/test-redeploy-fleetd.sh b/scripts/test-redeploy-fleetd.sh index dff94bb..9647a3e 100755 --- a/scripts/test-redeploy-fleetd.sh +++ b/scripts/test-redeploy-fleetd.sh @@ -228,6 +228,24 @@ test_jar_id_defaults_to_live_and_reports_explicit_path() { assert_equals "$staged_hash" "$explicit_result" "jar_id \"\$JAR_STAGED\" must report the hash of the staged jar, not fall back to \$JAR" } +# fleetd #517 — jar_id()'s "absent" branch was unpinned by any test: the existing test above (#511) +# proves both halves of the present-file contract but never exercises the missing-file path. This +# word matters more than a string usually would: "absent" is the #413 signal that a `mvn clean` +# deleted the running daemon's jar out from under it, and the `redeploy-fleetd` skill points +# operators at `--check` for exactly this. Covers both the no-argument default and an explicit path, +# since the mutation (`absent` -> `present`) sits on the single shared `|| echo` and would flip both. +test_jar_id_reports_absent_for_missing_file() { + local saved_jar="$JAR" dir default_result explicit_result + dir="$TMP/jar-id-absent"; mkdir -p "$dir" + JAR="$dir/does-not-exist.jar" + [ ! -f "$JAR" ] || fail "test fixture error: \$JAR unexpectedly exists at $JAR" + default_result="$(jar_id)" + explicit_result="$(jar_id "$dir/also-does-not-exist.jar")" + JAR="$saved_jar" + assert_equals "absent" "$default_result" "jar_id with no arguments must report absent when \$JAR does not exist" + assert_equals "absent" "$explicit_result" "jar_id with an explicit missing path must report absent" +} + # fleetd #493 — never build into the path a running process holds. stage_built_jar/swap_staged_jar # are exercised directly against real files on disk (not stubs), because the whole point is file # behavior (does the content move, does the source disappear, does a failure leave both sides @@ -386,6 +404,54 @@ test_drain_gate_abort_message_says_no_no_build() { || fail "abort message does not say why --no-build cannot finish the restart" } +# fleetd #517 — the drain-gate abort branch itself. Before this, the only test of this message was +# a source-text grep (test_drain_gate_abort_message_says_no_no_build, below): it greps this script's +# own file for the wording, which stays in the file even if the `if` guarding it is mutated to +# `if false` and the branch can never run. These four tests call drain_gate_refusal directly instead, +# so they fail if the branch is unreachable OR if its wording regresses — the grep test is KEPT +# alongside these, not replaced, because it catches a different regression (a re-wording that still +# reaches the right branch would not change which case fires here, but would still be worth pinning). +test_drain_gate_refusal_build_ran_staged_present() { + local dir staged result + dir="$TMP/drain-refusal-build-staged"; mkdir -p "$dir" + staged="$dir/fleetd-new.jar" + printf 'staged jar bytes' > "$staged" + result="$(drain_gate_refusal 1 "$staged")" + printf '%s' "$result" | grep -qF "$staged" \ + || fail "build-ran+staged-present refusal does not name the staged jar path" + printf '%s' "$result" | grep -qF 'Rerun WITHOUT --no-build' \ + || fail "build-ran+staged-present refusal does not tell the operator how to finish the restart" + if printf '%s' "$result" | grep -qF 'nothing changed'; then + fail "build-ran+staged-present refusal must not claim nothing changed — the jar already moved" + fi +} + +test_drain_gate_refusal_build_ran_staged_absent() { + local dir result + dir="$TMP/drain-refusal-build-no-staged"; mkdir -p "$dir" + result="$(drain_gate_refusal 1 "$dir/fleetd-new.jar")" + assert_equals "aborted — nothing changed" "$result" "build-ran+staged-absent refusal wording" +} + +# --no-build itself never builds or stages anything (require_no_build_jar, above), so a staged jar +# found here is a leftover from an earlier, unrelated run — THIS run truly changed nothing. See the +# comment above drain_gate_refusal in redeploy-fleetd.sh for the full reasoning. +test_drain_gate_refusal_no_build_staged_present() { + local dir staged result + dir="$TMP/drain-refusal-no-build-staged"; mkdir -p "$dir" + staged="$dir/fleetd-new.jar" + printf 'leftover staged jar bytes' > "$staged" + result="$(drain_gate_refusal 0 "$staged")" + assert_equals "aborted — nothing changed" "$result" "no-build+staged-present refusal must deliberately say nothing changed" +} + +test_drain_gate_refusal_no_build_staged_absent() { + local dir result + dir="$TMP/drain-refusal-no-build-no-staged"; mkdir -p "$dir" + result="$(drain_gate_refusal 0 "$dir/fleetd-new.jar")" + assert_equals "aborted — nothing changed" "$result" "no-build+staged-absent refusal wording" +} + test_no_errors() { cat > "$TMP/no-errors.log" <<'LOG' 2026-09-05 12:00:00 INFO fleetd listening @@ -598,6 +664,7 @@ test_count_daemon_pids test_assert_single_daemon_accepts_one_pid test_assert_single_daemon_rejects_two_pids test_jar_id_defaults_to_live_and_reports_explicit_path +test_jar_id_reports_absent_for_missing_file test_stage_built_jar_moves_off_live_path test_stage_built_jar_dies_when_build_produced_nothing test_swap_staged_jar_moves_staged_onto_live @@ -609,6 +676,10 @@ test_wait_for_daemon_exit_returns_true_once_pid_clears test_wait_for_daemon_exit_times_out_if_pid_never_clears test_swap_ordered_after_wait_and_before_start test_drain_gate_abort_message_says_no_no_build +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_no_errors test_recovery_patterns_match_source test_attributed_recovered_connection_error