fleetd #494 follow-up (2nd pass): the grace-release line still had one constant, and its test could not tell the difference
CI / contract (pull_request) Successful in 1m26s
CI / build (pull_request) Successful in 1m55s

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).
This commit is contained in:
Dai Ha
2026-09-12 09:39:39 +07:00
parent c87cc25aa6
commit e966cbadf9
2 changed files with 24 additions and 17 deletions
@@ -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 {
@@ -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);
}