From e966cbadf9e79187dabba3f17c027a236472fd0b Mon Sep 17 00:00:00 2001 From: Dai Ha Date: Sat, 12 Sep 2026 09:39:39 +0700 Subject: [PATCH] fleetd #494 follow-up (2nd pass): the grace-release line still had one constant, and its test could not tell the difference MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two more fixes on the same line, LeadRollover.java:600-624: 1. The poll-count argument (second, was PICKUP_GRACE_POLLS) now prints the loop's own idlePollsAwaitingPickup counter instead of the constant. Same defect shape as the nudges fix from c87cc25, one argument over. 2. LeadRolloverTest's clearGraceReleaseLogIsWarnWithMeasuredElapsed asserted its nudge-count expectation as `LeadRollover.PICKUP_GRACE_POLLS - 1` — the same expression the production code used to build the log line from, so it could not discriminate a reverted fix. Rewritten to plain literals ("after 8 consecutive IDLE/DONE polls (7 of those were nudged)"), proven to trip when PICKUP_GRACE_POLLS's value changes (2 failures under a PICKUP_GRACE_POLLS=5 mutation: this test and successLogPrintsMeasured...). Also corrected the comment above the log line: idlePollsAwaitingPickup and nudges each have exactly one write site on this loop's release branch, so they cannot differ from PICKUP_GRACE_POLLS / PICKUP_GRACE_POLLS - 1 at this call site — confirmed by re-deriving the loop's control flow, and by reverting just the nudges argument (M1) and observing 33/0 stayed green even after the test rewrite. That is an equivalent mutant on this line, not a gap the test rewrite could close; the honest value of printing the counters is one source of truth for the loop, not a provable-by-test difference here. The place nudges truly varies with the run — and is covered by a test that can tell it apart from a constant — is the /clear-timeout warn's clearResult.nudges() in runRollover. mvn -Dtest=LeadRolloverTest test: 33/0. mvn clean install: 1681/0, BUILD SUCCESS. Same-shape sweep (found, not fixed, per instructions): Injector.java:383-387 — the readiness-grace warn prints the constant READINESS_GRACE_POLLS where the measured per-target counter t.notReadySincePoll is in scope (single increment site, just reached the threshold at this call site — same equivalent-mutant situation as this fix). --- .../dev/ltms/fleet/lead/LeadRollover.java | 22 +++++++++++-------- .../dev/ltms/fleet/lead/LeadRolloverTest.java | 19 +++++++++------- 2 files changed, 24 insertions(+), 17 deletions(-) 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); }