test: move ready-seam category-2 sleeps onto the virtual clock and observables - #185
Conversation
Converts the wall-clock sleeps flagged in docs/reviews/2026-09-25-delay-sleep-audit.md
("Actor and deadlines" and "Bus state monitor" rows) to a VirtualClock, per #171:
- DeadlineTests: Complete_Before_Expiry_Prevents_OnExpired_And_Is_Idempotent,
Cancel_Before_Expiry_Prevents_OnExpired,
Disposing_The_Owning_Actor_While_Pending_Never_Fires_The_Deadline_And_Escapes_No_Exception,
Rearm_Before_Original_Expiry_Extends_The_Deadline (also drops the #130 wall-clock
thread-pool-starvation workaround, since a VirtualClock has no thread pool to starve).
- ProtocolActorTests: Disposing_The_Schedule_Handle_Before_Due_Prevents_The_Callback_From_Firing
converted; Dispose_Called_Reentrantly_From_A_Posted_Callback_Does_Not_Deadlock had its
300 ms sleep removed outright (ProtocolActor.Dispose() joins the loop thread synchronously,
so the post-dispose state is already settled the moment it returns; no clock is needed).
- BusStateMonitorTests: StateChanged_Is_Not_Raised_While_The_State_Is_Unchanged,
Dispose_Stops_Further_StateChanged_Events_And_Is_Idempotent (the latter now arms the
first poll deterministically via WaitUntilTimerArmedAsync before disposing, removing a
race the wall-clock version had between construction's Post(RearmPoll) and Dispose()).
Every converted test was mutation-checked against the product code it guards (guard
removed/disabled, confirmed red, restored) -- see the session report for the mutation used
per test.
Refs #171
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
Converts the four J1939 node claim-backoff/arbitration tests flagged in
docs/reviews/2026-09-25-delay-sleep-audit.md ("J1939 node -- actor injectable for
node timers" rows), matching the pattern the file's other VirtualClock-based claim
tests already use, per #171:
- A_Delayed_Cannot_Claim_Is_Dropped_Once_A_New_Claim_Has_Started
- A_Request_During_The_Backoff_Starts_The_Round_With_A_Single_Announcement
- A_Lost_Claim_Faults_Only_Once_Its_Cannot_Claim_Is_On_The_Bus
- A_Request_During_The_Cannot_Claim_Backoff_Is_Answered_By_That_One_Frame (also
redesigned from a Stopwatch-delta assertion taken after the fact into a direct
two-sided bracket of the backoff, since the delta no longer means anything once
the node's clock is virtual)
Each converted node ("loser") is now a caller-constructed J1939NodeImpl with an
injected VirtualClock actor; the other node ("winner") stays on J1939Node.Open and
the real clock. The initial loss in each test is still awaited on the real clock
(AsTaskWithTimeout) rather than via VirtualClock.RunUntilAsync: the loss is a wire
round trip with the real-clock winner, not something blocked on the frozen clock,
and advancing the virtual clock while waiting for it raced ahead of winner's real
defensive re-announce and made the loser see itself as uncontested -- caught by
running the initial conversion 10x and by two of these four failing outright.
VirtualClock.RunUntilAsync/WaitUntilTimerArmedAsync/AdvanceAsync are used only for
what is genuinely blocked on the loser's own clock (the second claim's completion,
the Cannot Claim's own backoff).
Every converted test was mutation-checked against the product code it guards
(CompleteLostClaim's new-claim drop, AnswerRequestForAddressClaimed's single-answer
branch, AnswerRequestWithCannotClaim's coalescing guard, and ScheduleCannotClaim's
fault-after-send-not-before ordering) -- see the session report for the mutation
used per test. All four also ran stable across 10 repeated runs.
Refs #171
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
…irtualClock Converts the remaining #171 J1939 rows from docs/reviews/2026-09-25-delay-sleep-audit.md: - ClaimAddressAsync_CancelDuringArbitration_TearsDownPendingClaim: the node now runs on an injected VirtualClock actor, so "wait past the original arbitration window" is a deterministic clock advance instead of a 700 ms wall-clock sleep against a 500 ms window. - StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriod (the audit's "still tight" budget: a 10.56 s wall-clock collection budget a 3x-loaded runner could still exhaust). Rebuilt on the same exact-grid VirtualClock pattern StartPeriodicSend_MultiFrame_Emits_On_An_ Exact_Grid_On_A_Clock_The_Test_Drives already uses, replacing the old mean-gap heuristic (which deliberately did not assert grid alignment, for lack of wall-clock resolution) with the stronger, exact assertion the multi-frame sibling makes. - StartPeriodicSend_SingleFrame_StopsAfterAddressLoss: the owner node runs on an injected VirtualClock actor; both the pre-contest "the schedule really was running" wait and the post-loss quiet window are now deterministic advances instead of wall-clock polling and a 130 ms sleep. The post-loss window steps the clock tick-by-tick without arming a specific timer first -- the loss also schedules the owner's own Cannot Claim backoff, which can legitimately become the actor's nearer timer, so "the periodic tick is next due" is not a safe thing to assert there -- and waits on PeriodicEmissionsCompleted (bumped whether a tick's SendAsync succeeded or was gated) rather than merely settling the actor, since the effect being checked is dispatched onto the thread pool. Every converted test was mutation-checked against the product code it guards (the already-completed guard together with the cancel teardown post in OnClaimAnnounceElapsed / CancelPendingClaimOnLoop, PeriodicSchedule's period-tick arithmetic, and SendAsync's claim-state gate) -- see the session report for the mutation used per test. All three also ran stable across 10 repeated runs. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
…al clock Converts the "ISO-TP channel -- actor injectable" rows from the #171 audit (docs/reviews/2026-09-25-delay-sleep-audit.md) and the negative-window / FC-processed sites from its conversion-order item 3. Task.Delay sleeps that stood in for a wait-for-effect are replaced with: - a frame-on-the-wire observable (FrameObserved / OnTransmitting) where the property is "this frame has reached the peer's bus", combined with an actor round-trip (SettleAsync/PostAsync) where the property also needs the receiving actor to have processed it; - clock.WaitUntilTimerArmedAsync + clock.AdvanceAsync where the wait is for an actor-armed timer (A_Stale_StMin_Timer, and the two IsoTpFunctionalClientTests conversions using the injected-clock IsoTpFunctionalClient constructor from #171/#183); - a TaskCompletionSource resolved from inside the counting handler itself, where the sleep was giving an independent sniffer subscription time to catch up with the last frame. Two sites (Send_Cancelled_Before_Actor_Delivery, Cancelled_Send_Holds_Gate, and A_Stale_StMin_Timer's trailing check) keep a bounded residual wait after the round-trip: the actor round-trip proves the actor's decision deterministically, but the actual bus write those tests guard against is dispatched onto the thread pool (SendFrameOnBus's Task.Run) with no actor-side observable, so a small margin remains for that one hop -- unchanged from the original, not lengthened. Each conversion was mutation-checked: the guarded product behaviour was broken in isoTp source, the test observed to fail, then the source was restored (no product changes are part of this commit). Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
…r seam #171: the seam from #183 (an injectable ProtocolActor on J1939TpChannel's internal constructor) lets the eight J1939-TP sleeps the delay/sleep audit flagged move off the wall clock: - Bam_AnnounceTxRejected_FailsSendAndDoesNotEmitDt: reads the injected, frozen-clock actor's own timer list instead of waiting 80 ms for a wrongly-scheduled TP.DT to fire. - SendCm_CanceledBeforeStart_DoesNotTransmit: an actor round trip proves BeginTxOnLoop (already enqueued on the same thread, in order, before the send even returns) has run; a short residual window covers the fire-and-forget wire transmit outside the actor's mailbox. - Cm_Sender_EomSizeMismatch_FailsSend: waits for WaitEom's T3 to actually be armed instead of guessing 20 ms is enough for OnCmDtConfirmed to run. - Cm_Receiver_Survives_A_First_Dt_That_Arrives_300ms_After_Cts: arms T2 on the actor's own clock, then advances by an exact 300 ms bracketed strictly inside T2 (1250 ms), instead of a 300 ms wall-clock lower bound. - A_Pdu1_Pgn_With_A_Low_Byte_Is_Refused_Before_Anything_Goes_Out: validation throws synchronously before either send ever posts to the actor, so the 50 ms "chance to transmit" was never needed at all; removed outright. - A_Retransmit_Request_Mid_Block_Takes_Effect_After_The_Outstanding_Packet and A_Cts_For_An_Unsent_Packet_Of_The_Block_Is_A_Sequence_Error_Not_A_Retransmit: actor round trips replace the "the CTS is on the actor" and "wire stays quiet" sleeps; the second also waits for the one expected abort's actual transmit before checking for a duplicate. Bam_Sender_Does_Not_Receive_Its_Own_Broadcast (the audit's Bam_Sender_On_An_Unflagged_Echo_Bus_Does_Not_Receive_Its_Own_Broadcast) was already converted to an ordering-only test with no sleep; left as-is. Send_InFlightAcrossReclaim_FailsWithNoAddressException (J1939NodeTests) still waits on a TP session the node opens internally, with no actor to inject; left, per the task. Every conversion is mutation-checked against the product behaviour it guards (details in the session record): each fails when the guarded behaviour is broken and passes once restored. No product code changes. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
…windows Review of the converted sleeps: - A_Retransmit_Request_Mid_Block_...: two actor round trips are not a barrier, because the CTS reaches the actor through the reader task's own post. The test now waits for the counting subscription to see the reader hand that CTS over, then for one round trip on the injected actor. - SendCm_CanceledBeforeStart_DoesNotTransmit and A_Cts_For_An_Unsent_Packet_...: the residual windows after the barrier are back to their original 100 ms and 50 ms. They cover a transmit that leaves through Task.Run outside the actor, and a shorter window only weakens the negative. - Send_Cancelled_Before_Actor_Delivery_... (ISO-TP): the actor round trip does not see SendFrameOnBus's Task.Run either, so the original 100 ms window follows it again. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
PR SummaryLow Risk Overview Virtual clock: Observables: ISO-TP and J1939-TP tests wait on wire events ( Residual wall time: Unshortened delays remain only where negatives depend on Reviewed by Cursor Bugbot for commit 0e967b3. Bugbot is set up for automated code reviews on this repo. Configure here. |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 28dea03169
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
- StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriod now charges 1 ms of virtual time per transmitted frame, as its multi-frame sibling does. Without that, a send-then-delay schedule lands on the same grid as an anchored one. Checked by mutation: re-anchoring each period after the send passes without the charge and fails with it. - Functional_Collect_Does_Not_Accept_Frames_After_Window_Expiry now puts the late frame in front of a collection that is still reading. The clock is moved past the deadline without waking the actor, the counting subscription shows the collector took the frame, and only then does the window's timer fire. Removing the deadline check in the collection loop fails the test. - A_Request_During_The_Cannot_Claim_Backoff_... waits for the node's reader to hand the Request to the loser's actor, then for one round trip, before it moves the clock. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9168aa47ca
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…hedule Codex on #185: - Functional_Collect_Discards_Frames_That_Arrived_Before_Send now puts the stale frame into the collection's own subscription, after that subscription exists and before SendAndCollectAsync drains it. The counting service's OnNextSubscribe hook does this. The frame is dropped both by the drain and by the handoff cutoff: removing both fails the test, removing the drain alone does not. - A_Request_During_The_Cannot_Claim_Backoff_... awaits each Cannot Claim from the observer. Each negative gets a 200 ms wall window for a frame already on its way, because the send and the hub delivery run off the actor's loop. - StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriod checks after Dispose that no tick is armed, and advances the quiet periods one at a time, with no extra-frame tolerance. A single jump would let a live schedule coalesce its ticks into one. Leaving the tick undisposed fails the test. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
…Claim Codex on #185: - MultiFrame_Send_Accepts_FlowControl_Arriving_During_Last_Cf_Confirm consumes the FC that answers the FF before its loop. Each wait in the loop is then for the FC the peer sent while that CF's confirmation was parked. Disabling the SendingCf defer branch fails the test. - A_Lost_Claim_Faults_Only_Once_Its_Cannot_Claim_Is_On_The_Bus awaits the spectator's Cannot Claim from its observer. The check for a second copy after the clock moves gets a 200 ms wall window. Faulting the claim before the Cannot Claim is sent fails the test. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 5470044. Configure here.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 5470044138
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Bugbot on #185, and the same pattern swept across the node tests this branch converted: - A_Request_During_The_Backoff_Starts_The_Round_With_A_Single_... waits for the node's reader to hand over the Request, then for one round trip on its actor, before any clock move. Announcing on the request instead of starting the round now fails the test. - A_Delayed_Cannot_Claim_Is_Dropped_..., A_Request_During_The_Backoff_... and StartPeriodicSend_SingleFrame_StopsAfterAddressLoss read the wire only after a WireWindow. A frame the actor sent still crosses the service and the hub on their own threads. - WireWindow (200 ms) is one constant for every such negative in the file, replacing the local and literal copies. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
…uffered Codex on #185: the OnNextSubscribe hook only started the cross-bus transmit. The drain could therefore run before the stale frame reached the subscription, and the frame would then be admitted afterwards. The hook now waits until busA has raised the frame. The service attached to busA before that handler, so it has already dispatched the frame into the new subscription. Removing both the drain and the handoff cutoff fails the test in 3 of 3 runs. Refs #171 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
|
The failing test is Why this is not caused by this PR:
The cause is a wall-clock margin inside that test:
This is the same kind of clock-bound test #171 tracks. The UDS rows are the next wave after this PR. The macOS job is already re-running (started 20:12Z). That is the one re-run; I'm not requesting another. Generated by Claude Code |

What does this change?
This is the second slice of #171. It converts the category-2 sleeps from
docs/reviews/2026-09-25-delay-sleep-audit.mdin the areas whose clock can already be injected: the J1939 node,DeadlineScheduler/ProtocolActor,BusStateMonitor, the ISO-TP channel, and J1939-TP (injectable since #183). A category-2 sleep assumes some work has finished within a wall-clock window. The changes are tests only. No product code changes.Each converted sleep became one of the following:
WaitUntilTimerArmedAsync). Where the test is about an interval, it checks both sides: nothing has fired just before the configured interval, and it has fired just after.SettleAsync, which pulls buffered frames through;TaskCompletionSourcecompleted from the event handler itself.Where a transmit leaves the actor through
Task.Run, no actor round trip can see it. Those negative checks keep their original wall-clock window, unshortened, after the new deterministic barrier.Notable:
StartPeriodicSend_SingleFrame_FiresAtConfiguredPeriodno longer has the 10.56 s whole-test budget that the audit lists as still tight. The test now checks the exact emission grid on the virtual clock.Rearm_Before_Original_Expiry_Extends_The_Deadlineno longer needs the Rearm_Before_Original_Expiry_Extends_The_Deadline schlägt auf net48 fehl — Last erklärt es nicht #130 blocking waits. It checks the re-armed deadline from both sides.Cm_Receiver_Survives_A_First_Dt_That_Arrives_300ms_After_Ctswaits until T2 is armed, then advances exactly 300 ms.Each converted test was mutation-checked. Its guarded behaviour was broken in
src/, the test failed, and the source was restored. Each converted test also ran 10 times without a failure.What stays for follow-up (#171)
J1939NodeTests:ReClaim_RejectsSendUntilNewClaimSucceeds,RebindTransport_DoesNotDeliverBamMoreThanOncePerRebind, andSend_InFlightAcrossReclaim_FailsWithNoAddressException. The last one waits on a TP session the node opens internally without handing it its actor.UdsClientImpland new observables.Type of change
feat— new behaviour (minor release)fix/perf— bug or performance fix (patch release)docs/test/refactor/chore/ci— no release!in the title, plus aBREAKING CHANGE:footer explaining the migration)All commits are
test(...)orstyle(...).Checklist
dotnet build CanKit.Pro.sln -c Releasesucceeds (-p:CI=true)dotnet test CanKit.Pro.sln -c Releasepasses (net10.0)dotnet format --verify-no-changesanddotnet pack+eng/verify-packages.pyalso pass.🤖 Generated with Claude Code
https://claude.ai/code/session_013WJ8h1ahw4Nj5dYuEWy34s
Generated by Claude Code