test(uds): take R's reference before the cancellation, not after the throw - #187
Conversation
…throw R measured the second send's gap from a timestamp read once the test had observed the cancelled send's exception -- after the client's note, by however long the test's continuation took to be scheduled. Every millisecond of that came off the gap, so the 5 ms margin was a bound on host scheduling. Under a 4-process CPU load on a 4-CPU container it failed 14 runs in 20, short by up to 13 ms. The send is now cancelled by hand and the reference is read just before Cancel(), which is no later than the note the cancellation causes: delay can only lengthen the measured gap. A precondition asserts the provisional window was over at cancellation. 30 of 30 under the same load; with the re-note in the send's catch removed it fails 5 of 5 (gap ~18 ms against the 75 ms floor). Refs #92 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Kv8GdFfMkktfWdqjNyCDtW
PR SummaryLow Risk Overview Instead of a 150 ms timer on A new assertion on Reviewed by Cursor Bugbot for commit 02c0779. 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. |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
2 participants
What does this change?
UdsExpiredDeadlineTests.R_A_Suppressed_Send_Cancelled_After_Its_Provisional_Window_Still_Opens_Onemeasured the second send's gap from a timestamp it read after it saw the cancelled send throw. By then the client had already noted the window, so the time the test's continuation spent waiting to be scheduled was subtracted from the gap. The 5 ms margin was therefore a limit on host scheduling delay, and a loaded runner goes over it.The test now cancels the send itself and reads the reference just before
Cancel(). That instant comes no later than the client's note, which the cancellation triggers. Scheduling delay can now only lengthen the measured gap. The margin itself is unchanged: widening it would not have fixed the problem, because the error grows with load and has no upper limit. A new precondition checks that the provisional window really was over when the send was cancelled, so the test cannot silently turn into test O.Evidence, all runs on a 4-CPU container with 4 CPU-bound busy loops as load:
main)catchremoved (mutation)In the base run, O and Q (same file, same 5 ms margin) ran next to R and passed 20 of 20. Their reference points come before the note, not after, so they do not have this flaw. They are unchanged here.
Refs #92. This is out of scope for #171 because it does not touch this test's code path. It is split out at the maintainer's request.
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)Checklist
dotnet build CanKit.Pro.sln -c Releasesucceeds (with-p:CI=true, 0 warnings)dotnet test CanKit.Pro.sln -c Releasepasses (--framework net10.0only: 1212/1212;dotnet format --verify-no-changesand pack +eng/verify-packages.pyalso clean)FR-RAW-031,ADR-7), if any🤖 Generated with Claude Code
https://claude.ai/code/session_01Kv8GdFfMkktfWdqjNyCDtW
Generated by Claude Code