diff --git a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java index f5c9587..c7039af 100644 --- a/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java +++ b/fleetd/src/main/java/dev/ltms/fleet/lead/LeadRollover.java @@ -605,18 +605,22 @@ public final class LeadRollover { // whole roll "succeeded" after 438ms of a 20s budget), so raise it to WARN and // print the MEASURED elapsed time next to the target pane, not just the count. // - // fleetd #494 follow-up: the nudge count printed here must be the MEASURED - // `nudges` counter, never the constant `PICKUP_GRACE_POLLS - 1` — the constant - // happens to equal it today (this branch only fires after exactly - // PICKUP_GRACE_POLLS - 1 nudges), but a mutation test caught this exact - // expression once already printing the wrong constant (PICKUP_GRACE_POLLS - // itself, claiming 8 nudges where 7 went out) and a plain code read did not - // catch it. Printing the counter cannot drift from the loop's real behaviour, - // whatever changes later to PICKUP_GRACE_POLLS or the loop itself. + // fleetd #494 follow-up (2nd pass): BOTH numbers in this line must come from + // the loop's own counters, never from the PICKUP_GRACE_POLLS constant. + // `idlePollsAwaitingPickup` and `nudges` each have exactly one write site in + // this loop, on the same branch, so on this branch they cannot differ from + // PICKUP_GRACE_POLLS / PICKUP_GRACE_POLLS - 1 today — no test can prove the + // difference on this line, and printing the counters does not change that. + // What it does buy: one source of truth instead of two, so a later change to + // the loop (an early return, a second increment site, a different exit + // condition) cannot leave this message reporting a number the loop no longer + // produces. The place where `nudges` genuinely varies with the run — and is + // covered by a test that can tell it apart from a constant — is the + // /clear-timeout warn in runRollover, which prints clearResult.nudges(). log.warn("lead-rollover: /clear on {} was never observed as WORKING after {} " + "consecutive IDLE/DONE polls ({} of those were nudged) — " + "releasing rather than wedging the roll (elapsed={}ms)", - target, PICKUP_GRACE_POLLS, nudges, elapsedMillis); + target, idlePollsAwaitingPickup, nudges, elapsedMillis); return new ClearSettleResult(true, elapsedMillis, nudges); } try { diff --git a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java index cfbc813..cda2c64 100644 --- a/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java +++ b/fleetd/src/test/java/dev/ltms/fleet/lead/LeadRolloverTest.java @@ -971,14 +971,17 @@ class LeadRolloverTest { assertTrue(message.contains("elapsed=4500ms"), "must print the MEASURED elapsed time — " + "with this fixture's advancing clock, the wait ran 4500ms before releasing: " + message); - // fleetd #494 follow-up: the nudge count in this line must be the MEASURED counter, not - // the constant PICKUP_GRACE_POLLS - 1 that used to stand in for it — the two happen to - // agree today, but only the counter cannot drift from the loop's real behaviour if - // PICKUP_GRACE_POLLS or the loop ever changes. Reference the constant here (rather than - // hardcoding "7") so this assertion itself does not silently stop discriminating if - // PICKUP_GRACE_POLLS changes later. - assertTrue(message.contains("(" + (LeadRollover.PICKUP_GRACE_POLLS - 1) + " of those were nudged)"), - "must print the measured nudge count: " + message); + // fleetd #494 follow-up (2nd pass): both numbers here are DELIBERATE plain literals, + // not derived from LeadRollover.PICKUP_GRACE_POLLS. A version of this assertion that + // reads "(" + (LeadRollover.PICKUP_GRACE_POLLS - 1) + " of those were nudged)" builds + // its expectation the same way the production code used to build the log line, so it + // cannot tell a fixed constant apart from the measured counter — proved by reverting + // the production fix and re-running: that mutant stayed green under the old assertion. + // If PICKUP_GRACE_POLLS ever changes, THIS TEST MUST FAIL and a human must look at the + // new message and update the literals below, not just re-derive them. + assertTrue(message.contains("after 8 consecutive IDLE/DONE polls (7 of those were nudged)"), + "must print the measured poll count and nudge count as plain numbers, not the " + + "PICKUP_GRACE_POLLS constant standing in for either: " + message); } finally { detachLog(events); }