fleetd #544: pin the sticky-STOPPED-across-restart invariant
Review of PR #559 (issue comment #16944) found a surviving mutant: removing watchdog.reset() from StatusPoller.start() (and the identical line in SessionReaper.start()) passed the entire suite. stoppedByCaller is sticky and reset() — called only from start() — is the only thing that clears it. Both loops document start() as idempotent and loop()'s own error log says "it can be restarted", so stop() followed by start() is an anticipated path. Without reset() wired into start(), health() would report STOPPED forever after a restart even though the loop is genuinely running again. Add aRestartedLoopReportsRunningAgainNotStoppedForever to both StatusPollerWatchdogTest and SessionReaperWatchdogTest, pinning "an intentional stop must not outlive the restart that follows it". Verified via the standard mutation cycle: exact-line anchor (not regex, to avoid the \Q-style false match the reviewer flagged) counted pristine 1 -> mutated 0, test goes red with its own message, restored, shasum -a 256 byte-identical, green again. mvn clean install: exit 0, BUILD SUCCESS, Tests run: 1734, Failures: 0, Errors: 0, Skipped: 0 (cross-checked against 130 surefire report files). No production code changed — the reset() call under test was already correct; it simply had nothing pinning it. 🤖 Generated with Claude Code Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -84,6 +84,24 @@ class StatusPollerWatchdogTest {
|
||||
"stop() must report STOPPED, never STALLED, however stale the last round looks");
|
||||
}
|
||||
|
||||
@Test
|
||||
void aRestartedLoopReportsRunningAgainNotStoppedForever() throws Exception {
|
||||
// fleetd #544 review (issue comment #16944): stoppedByCaller is sticky, and reset() —
|
||||
// called only from start() — is the sole thing that clears it. start() is documented
|
||||
// idempotent and loop()'s own error line says "it can be restarted", so stop() followed
|
||||
// by start() is an anticipated path. Without the reset() call in start(), health() would
|
||||
// report STOPPED forever after a restart even though the loop is genuinely running again.
|
||||
AtomicLong clock = new AtomicLong(0);
|
||||
StatusPoller poller = new StatusPoller(new AgentControl(new IdleHerdr()), new Injector(new AgentControl(new IdleHerdr())),
|
||||
new StatusRefiner(new AgentControl(new IdleHerdr())), INTERVAL_MILLIS, clock::get);
|
||||
poller.start();
|
||||
poller.stop();
|
||||
poller.start();
|
||||
|
||||
assertEquals(LoopWatchdog.State.RUNNING, poller.health(),
|
||||
"an intentional stop must not outlive the restart that follows it");
|
||||
}
|
||||
|
||||
@Test
|
||||
void aHealthyLoopReportsRunning() throws Exception {
|
||||
// Real elapsed time on purpose, unlike the other tests here: a clock frozen at 0 would read
|
||||
|
||||
@@ -88,6 +88,23 @@ class SessionReaperWatchdogTest {
|
||||
"stop() must report STOPPED, never STALLED, however stale the last round looks");
|
||||
}
|
||||
|
||||
@Test
|
||||
void aRestartedLoopReportsRunningAgainNotStoppedForever() throws Exception {
|
||||
// fleetd #544 review (issue comment #16944): stoppedByCaller is sticky, and reset() —
|
||||
// called only from start() — is the sole thing that clears it. start() is documented
|
||||
// idempotent and loop()'s own error line says "it can be restarted", so stop() followed
|
||||
// by start() is an anticipated path. Without the reset() call in start(), health() would
|
||||
// report STOPPED forever after a restart even though the loop is genuinely running again.
|
||||
AtomicLong watchdogClock = new AtomicLong(0);
|
||||
SessionReaper reaper = new SessionReaper(sessionManager(System::nanoTime), 60, INTERVAL_MILLIS, watchdogClock::get);
|
||||
reaper.start();
|
||||
reaper.stop();
|
||||
reaper.start();
|
||||
|
||||
assertEquals(LoopWatchdog.State.RUNNING, reaper.health(),
|
||||
"an intentional stop must not outlive the restart that follows it");
|
||||
}
|
||||
|
||||
@Test
|
||||
void aHealthyLoopReportsRunning() throws Exception {
|
||||
// Real elapsed time on purpose, unlike the other tests here: a clock frozen at 0 would read
|
||||
|
||||
Reference in New Issue
Block a user