fleetd #608: make anAlreadyCollectedTicketProducesNoNudge deterministic #611

2026-09-20T12:19:13+02:00

agent commented

2026-09-20 12:19:13 +02:00

fleetd #608: MessageServiceTest.anAlreadyCollectedTicketProducesNoNudge was flaky — it passed in isolation but failed on a full-suite run (expected: <false> but was: <true> at the agent.prompt assertion), because it bet a 300ms backoff was wide enough for the test to collect the ticket before ReplyPushLoop's scheduled tick fired. Under load that bet lost.

Fix

Added ManualScheduler (src/test/java/dev/ltms/fleet/msg/ManualScheduler.java): a ScheduledExecutorService fake that records what ReplyPushLoop schedules and only runs it when the test calls runDueTasks(). It implements only the two methods ReplyPushLoop actually calls (schedule(Runnable, long, TimeUnit) and shutdownNow()); everything else throws UnsupportedOperationException by design.
Added wireWithManualScheduler(...) / ManualPushWiring alongside the existing wireWithPushLoop(...) in MessageServiceTest — the real-scheduler overloads are untouched, since other tests in the file rely on a real timer on purpose.
Rewrote anAlreadyCollectedTicketProducesNoNudge to: send → wait for the ticket's terminal notification (via the existing setAfterFinishAsyncTaskCompleteHookForTest hook, so it doesn't race CompletableFuture.complete()'s own publish-then-run-dependents gap, fleetd #399) → collect the ticket → run the one pending tick explicitly with runDueTasks() → assert no nudge. No Thread.sleep, no backoff dependency.

Verified

anAlreadyCollectedTicketProducesNoNudge with backoff set to 1 (the most hostile value) still passes, in isolation.
Mutating ReplyPushLoop.ticketCollected to a no-op (so a collected ticket is never removed from pendingTickets) turns the test red with the same assertion message the original flake reported (a ticket the lead already polled must never be nudged ==> expected: <false> but was: <true>); reverted afterwards (clean git diff on ReplyPushLoop.java).
Three consecutive mvn -o clean install runs from fleetd/: all three Tests run: 1841, Failures: 0, Errors: 0, Skipped: 0, BUILD SUCCESS.

Out of scope (reported, not fixed, per the ticket)

The ticket asked me to report — not fix — whether the file's other large Thread.sleep barriers have the same shape (betting on wall-clock ordering between a scheduled tick and a test action, so they can go red on correct code under load):

line 1910 (Thread.sleep(100), severalAsyncTicketsFinishingTogetherProduceOneCoalescedNudge): same shape — it's part of the same "both tickets must land inside the 300ms backoff" bet as the fixed defect.
line 1916 (Thread.sleep(100), same test): same shape — the second half of that same window.
line 1919 (Thread.sleep(200), same test, the post-awaitNudge settle check): different risk direction — it can only mask a bug (false green) under load, not go red on correct code.
line 2020 (Thread.sleep(300), answeringAQuestionStopsFurtherNudgesAboutIt): not the same shape — the question is already closed (state settled) before the sleep starts, so there's nothing left to race.
line 2112 (Thread.sleep(300), aPrunedTicketIsReclaimedFromThePushLoopNotLeakedForever): not the same shape — it follows awaitNudge, which already deterministically observed the nudge; the trailing sleep and assertion hold regardless of timing since no second ticket exists yet.

fleetd #608: `MessageServiceTest.anAlreadyCollectedTicketProducesNoNudge` was flaky — it passed in isolation but failed on a full-suite run (`expected: <false> but was: <true>` at the `agent.prompt` assertion), because it bet a 300ms backoff was wide enough for the test to collect the ticket before `ReplyPushLoop`'s scheduled tick fired. Under load that bet lost. ## Fix - Added `ManualScheduler` (`src/test/java/dev/ltms/fleet/msg/ManualScheduler.java`): a `ScheduledExecutorService` fake that records what `ReplyPushLoop` schedules and only runs it when the test calls `runDueTasks()`. It implements only the two methods `ReplyPushLoop` actually calls (`schedule(Runnable, long, TimeUnit)` and `shutdownNow()`); everything else throws `UnsupportedOperationException` by design. - Added `wireWithManualScheduler(...)` / `ManualPushWiring` alongside the existing `wireWithPushLoop(...)` in `MessageServiceTest` — the real-scheduler overloads are untouched, since other tests in the file rely on a real timer on purpose. - Rewrote `anAlreadyCollectedTicketProducesNoNudge` to: send → wait for the ticket's terminal notification (via the existing `setAfterFinishAsyncTaskCompleteHookForTest` hook, so it doesn't race `CompletableFuture.complete()`'s own publish-then-run-dependents gap, fleetd #399) → collect the ticket → run the one pending tick explicitly with `runDueTasks()` → assert no nudge. No `Thread.sleep`, no backoff dependency. ## Verified - `anAlreadyCollectedTicketProducesNoNudge` with backoff set to `1` (the most hostile value) still passes, in isolation. - Mutating `ReplyPushLoop.ticketCollected` to a no-op (so a collected ticket is never removed from `pendingTickets`) turns the test red with the same assertion message the original flake reported (`a ticket the lead already polled must never be nudged ==> expected: <false> but was: <true>`); reverted afterwards (clean `git diff` on `ReplyPushLoop.java`). - Three consecutive `mvn -o clean install` runs from `fleetd/`: all three `Tests run: 1841, Failures: 0, Errors: 0, Skipped: 0`, `BUILD SUCCESS`. ## Out of scope (reported, not fixed, per the ticket) The ticket asked me to report — not fix — whether the file's other large `Thread.sleep` barriers have the same shape (betting on wall-clock ordering between a scheduled tick and a test action, so they can go red on correct code under load): - line 1910 (`Thread.sleep(100)`, `severalAsyncTicketsFinishingTogetherProduceOneCoalescedNudge`): same shape — it's part of the same "both tickets must land inside the 300ms backoff" bet as the fixed defect. - line 1916 (`Thread.sleep(100)`, same test): same shape — the second half of that same window. - line 1919 (`Thread.sleep(200)`, same test, the post-`awaitNudge` settle check): different risk direction — it can only mask a bug (false green) under load, not go red on correct code. - line 2020 (`Thread.sleep(300)`, `answeringAQuestionStopsFurtherNudgesAboutIt`): not the same shape — the question is already closed (state settled) before the sleep starts, so there's nothing left to race. - line 2112 (`Thread.sleep(300)`, `aPrunedTicketIsReclaimedFromThePushLoopNotLeakedForever`): not the same shape — it follows `awaitNudge`, which already deterministically observed the nudge; the trailing sleep and assertion hold regardless of timing since no second ticket exists yet.

agent added 1 commit 2026-09-20 12:19:13 +02:00

fleetd #608 : make anAlreadyCollectedTicketProducesNoNudge deterministic

CI / shell-tests (pull_request) Failing after 9s

Details

CI / contract (pull_request) Successful in 1m21s

Details

CI / build (pull_request) Failing after 1m34s

Details

aa517ae0ec

Replace the real ScheduledExecutorService backing ReplyPushLoop in this one
test with ManualScheduler, a fake that only runs a tick when the test calls
runDueTasks(). The old test bet a 300ms backoff was wide enough that
collecting the ticket always won the race against the scheduler's own timer
- true on an idle machine, false under a loaded full-suite run, which is
exactly the flake reported.

The rewritten test also waits on setAfterFinishAsyncTaskCompleteHookForTest
(already used elsewhere in this file) instead of polling Phase.DONE, so it
does not race CompletableFuture.complete()'s own publish-then-run-dependents
gap (fleetd #399) while proving ReplyPushLoop.onTicketTerminal really ran
before the ticket is collected.

Verified: backoff=1 (the most hostile value) still passes; mutating
ReplyPushLoop.ticketCollected to a no-op turns the test red with the same
assertion message the original flake reported; three consecutive full-suite
runs are green (1841/1841 each).

ltms merged commit fa62e9906d into main

2026-09-20 12:26:16 +02:00

ltms referenced this pull request

2026-09-20 12:27:41 +02:00

Flaky: MessageServiceTest.anAlreadyCollectedTicketProducesNoNudge orders an async scheduler with Thread.sleep #608

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: fleet/fleetd#611