Repository navigation
test(run-commands): ThrottledSignalTests wait for the event, not a fixed 500ms - #176
Merged
Merged
Conversation
…xed 500ms A_burst_of_signals_runs_the_action_once failed on main's CI right after #160 merged (Expected: 1, Actual: 0) while passing in #160's own run and locally. Each test slept a fixed 500ms of wall clock for a 50ms timer; xunit runs test classes in parallel, and on a loaded runner the Task.Delay continuation can land later than that. A_signal_after_the_action_ran_schedules_another had a second form of the same race: if the first run was late, the second Signal piggybacked on it and the test saw one run instead of two. Now each test polls until the expected count is reached (10s ceiling, only for a broken implementation), then watches a further 300ms to catch an extra run. ThrottledSignal itself is unchanged. Mutation-checked: removing the coalescing fails the burst test; clearing _pending after the action instead of before fails the during-action test. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes a timing flake in
ThrottledSignalTests(added in #160) that failedmain's CI right after the merge:A_burst_of_signals_runs_the_action_once— Expected: 1, Actual: 0.ThrottledSignalitself is unchanged.Why
Each test slept a fixed 500ms of wall clock and then asserted, for a 50ms timer. xunit runs test classes in parallel; on a loaded runner the
Task.Delaycontinuation can land later than 500ms. It passed in #160's own CI and locally, then failed onmain. A release tag runs the same tests, so this would fail releases at random.A_signal_after_the_action_ran_schedules_anotherhad a second form of the race: a late first run makes the secondSignalpiggyback on it.Implementation notes
Each test now waits until the expected run count is reached (10s ceiling — only a broken implementation gets near it), then watches 300ms more to catch an extra run.
Test plan
_pendingafter the action instead of before fails the during-action test.🤖 Generated with Claude Code
https://claude.ai/code/session_01NsYm9iLc2aRnRDfV2uZRQX