test: drive witness and from-task dispatch from gates instead of the clock - #179
Open
glennawatson wants to merge 2 commits into
Open
test: drive witness and from-task dispatch from gates instead of the clock#179glennawatson wants to merge 2 commits into
glennawatson wants to merge 2 commits into
Conversation
…clock - WitnessOn is not thread-pool specialized, so its test now drives a ManualSequencer and asserts dispatch is deferred until the sequencer drains it, replacing a real pool subscription behind a 2s wall clock and a `values.Count <= 1` assertion that held even when every value was dropped. - The from-task completion and user-exception tests raced a 10s in-command delay against a 10.5s/11.5s test-side delay. They now release the command body through a TaskCompletionSource and wait on the terminal cleanup, the pattern the file's other from-task tests already use. - Dispose in the user-exception test now lands after the terminal has been observed, which is the ordering its assertions always described.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #179 +/- ##
==========================================
- Coverage 98.19% 98.18% -0.01%
==========================================
Files 711 711
Lines 22080 22078 -2
Branches 2704 2703 -1
==========================================
- Hits 21681 21678 -3
Misses 196 196
- Partials 203 204 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
- Extract the race-only tail of DelegateWorkItem.Execute into an excluded helper. Single-threaded it is unreachable: a completed Dispose leaves the slot holding EmptyDisposable, so the compare-exchange takes the already-claimed branch and never falls through. It was only ever reached when the existing Barrier test happened to win its race, which made the line's coverage a property of machine load. - ExpireCoordinatorTests dispatched a deliberately blocking OnNext through the thread pool, so a saturated pool left the notification queued behind unrelated work and the test timed out waiting to enter the observer. Both sides of the race now run on dedicated threads. - The same test read the invariant flag while both threads were still running, through plain fields written on other threads. Those are volatile now, so the assertion observes the write rather than relying on one being published.
|
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
1 participant
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.
What kind of change does this PR introduce?
Test fix.
What is the new behavior?
Three async dispatch tests are driven by explicit gates rather than by the wall clock, so their outcome no longer depends on how loaded the runner is.
WitnessOnis exercised through a sequencer the test owns.WitnessOn(ISequencer)has no thread-pool specialization - it is one genericWitnessOnSignal<T>- so the test now drivesManualSequencerand asserts that nothing reaches the observer until the sequencer drains the queued work, then that the values and the terminal arrive in order. Split intoWitnessOnDefersNextAndCompletedUntilTheSequencerDrainsThemandWitnessOnDefersAnErrorUntilTheSequencerDrainsIt.TaskCompletionSource. The command signals that it started, blocks on a release gate, and the terminal cleanup signals when the chain has finished; the assertions run off those gates. This is the pattern the file's other from-task tests already use.What is the current behavior?
All three tests decided their outcome from timing and went red on loaded CI runners.
WitnessOnThreadPoolDispatchesNextCompletedAndErrorSignalssubscribed through the real thread pool and waited 2 seconds for the dispatch. Its value assertion wasvalues.Count <= 1, which held even if every value was dropped.SignalFromTaskHandlesCompletionandSignalFromTaskHandlesUserExceptionsraced a 10s in-commandTask.Delayagainst a 10.5s / 11.5s test-side delay - 500ms and 1.5s of margin. When the runner was slow enough the command had not finished, so the dispose cancelled it and the cancellation status messages the tests assert against appeared.SignalFromTask_T_HandlesCompletioncarried the same 1.5s margin and is converted along with them.What might this PR break?
None. Product code is untouched; this is test-only.
Checklist
mainbranchAdditional information
ThreadPoolSequencerkeeps its own coverage inSequencerTests.Pools.cs, so removing it from the witness test loses nothing.The remaining timer-driven from-task tests (
HandlesCancellation,HandlesCancellationInBaseand their_T_twins) are left as they are: they dispose at 500ms against a 10s delay, so they have 9.5s of margin rather than the sub-second margins fixed here.