fleetd #603 review: close the untested fall-through path
PR review (comment 17358) found a real gap via mutation testing: replacing the "pid never found" warn with die "no process appeared" left the whole suite green, because neither existing test drove the case the fall-through exists for -- running_pid() never finds anything (as its own doc comment says it eventually will) while /healthz answers anyway. Adds test_await_daemon_started_pid_never_found_but_healthy_warns_and_survives: running_pid always empty, poll_health_body succeeds. Asserts DIED_CALLED=0, the warn line is emitted, and NEW_PID stays empty (the honest "could not establish this" answer, never a guessed pid). Verified both halves myself: reverting the warn to die "no process appeared" turns this one test red (FAIL: await_daemon_started must not die...); restoring it returns the suite to green.
This commit is contained in:
@@ -857,6 +857,43 @@ test_await_daemon_started_never_appears_dies_with_log_tail() {
|
|||||||
source "$ROOT/scripts/redeploy-fleetd.sh"
|
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# fleetd #603 review — the path the fall-through actually exists for, and the one gap a lead
|
||||||
|
# mutation found in the first version of this test file: running_pid() NEVER finds anything (as its
|
||||||
|
# own doc comment says it eventually will, once the daemon stops being launched as a plain
|
||||||
|
# `java -jar` its allowlist recognises), while /healthz answers anyway. Neither of the two tests
|
||||||
|
# above drives this: the slow-pid test has the pid appear, so the `else` branch never runs, and the
|
||||||
|
# never-appears test fails BOTH checks, so it dies either way and cannot tell which branch fired.
|
||||||
|
# This must not die, must warn (so the operator is told the pid could not be identified), and must
|
||||||
|
# leave NEW_PID empty — the honest "could not establish this" answer, never a guessed pid, which is
|
||||||
|
# what the final result line's `${NEW_PID:-unknown}` fallback exists to print truthfully.
|
||||||
|
#
|
||||||
|
# Proof this actually pins the behavior, not just the source text (paste from a real run, not
|
||||||
|
# claimed): reverting the `warn` below back to `die "no process appeared"` (the old fleetd #603
|
||||||
|
# defect, reintroduced) turns this one test red —
|
||||||
|
# FAIL: await_daemon_started must not die when the pid is never found but healthz answers
|
||||||
|
# — and restoring `warn` turns the whole suite green again. Both halves observed, not asserted.
|
||||||
|
test_await_daemon_started_pid_never_found_but_healthy_warns_and_survives() {
|
||||||
|
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||||
|
stub_die_recorder
|
||||||
|
local out_file="$TMP/await-pid-never-found.out" output
|
||||||
|
printf 'boot line\n' > "$out_file"
|
||||||
|
running_pid() { printf ''; }
|
||||||
|
sleep() { :; }
|
||||||
|
poll_health_body() { printf '{"status":"ok"}'; return 0; }
|
||||||
|
# NOT `output="$(await_daemon_started ...)"` — see the slow-pid test above for why that would
|
||||||
|
# drop NEW_PID's assignment in a subshell instead of reaching this test's own shell.
|
||||||
|
await_daemon_started 3 "" "http://ignored/healthz" "$out_file" \
|
||||||
|
> "$TMP/await-pid-never-found-output.log" 2>&1
|
||||||
|
output="$(cat "$TMP/await-pid-never-found-output.log")"
|
||||||
|
[ "$DIED_CALLED" = 0 ] \
|
||||||
|
|| fail "await_daemon_started must not die when the pid is never found but healthz answers: $DIED_MESSAGE"
|
||||||
|
printf '%s' "$output" | grep -qF 'falling through to the health check' \
|
||||||
|
|| fail "await_daemon_started did not warn that the pid could not be identified"
|
||||||
|
assert_equals "" "$NEW_PID" \
|
||||||
|
"await_daemon_started NEW_PID when the pid is never found but healthz answers — must stay empty, never a guessed pid"
|
||||||
|
source "$ROOT/scripts/redeploy-fleetd.sh"
|
||||||
|
}
|
||||||
|
|
||||||
test_await_daemon_started_call_site_present() {
|
test_await_daemon_started_call_site_present() {
|
||||||
local src="$ROOT/scripts/redeploy-fleetd.sh" call_line
|
local src="$ROOT/scripts/redeploy-fleetd.sh" call_line
|
||||||
call_line="$(grep -Fn 'await_daemon_started "$HEALTH_WAIT" "$OLD_PID" "$HEALTH" "$OUT"' "$src" | head -1 | cut -d: -f1 || true)"
|
call_line="$(grep -Fn 'await_daemon_started "$HEALTH_WAIT" "$OLD_PID" "$HEALTH" "$OUT"' "$src" | head -1 | cut -d: -f1 || true)"
|
||||||
@@ -2141,6 +2178,7 @@ test_wait_for_new_pid_returns_true_once_pid_appears
|
|||||||
test_wait_for_new_pid_times_out_if_pid_never_appears
|
test_wait_for_new_pid_times_out_if_pid_never_appears
|
||||||
test_await_daemon_started_slow_pid_then_healthy_succeeds
|
test_await_daemon_started_slow_pid_then_healthy_succeeds
|
||||||
test_await_daemon_started_never_appears_dies_with_log_tail
|
test_await_daemon_started_never_appears_dies_with_log_tail
|
||||||
|
test_await_daemon_started_pid_never_found_but_healthy_warns_and_survives
|
||||||
test_await_daemon_started_call_site_present
|
test_await_daemon_started_call_site_present
|
||||||
test_swap_ordered_after_wait_and_before_start
|
test_swap_ordered_after_wait_and_before_start
|
||||||
test_drain_gate_abort_message_says_no_no_build
|
test_drain_gate_abort_message_says_no_no_build
|
||||||
|
|||||||
Reference in New Issue
Block a user