chore(Test): Revert #20308 precautionary serial testing - #20343
Conversation
✅ No release notes required |
T-Gro
left a comment
There was a problem hiding this comment.
The reasoning holds up under inspection, so this is a clean revert to make.
The only shared mutable state in either file is Async.CancelDefaultToken, and every call to it lives inside the AsyncType module and the AsyncType class — both of which keep their [<Collection(nameof NotThreadSafeResourceCollection)>]. The four suites you release back to parallel execution (AsyncModuleFunctionsTests, AsyncAwaitTaskLikeTests, AsyncStartTaskImmediateTaskLikeTests, AsyncAwaitStackTraceTests) touch no process-global state, so there was never a correctness reason for them to be serialized — the 30s failsafe in verifyTaskCompletion is the part that actually addressed the loaded-CI flakiness, and that stays.
A full green fsharp-ci run with no host timeouts is the empirical half of the argument.
One tiny thing, not blocking: the AsyncTaskLikeAwaitTests → AsyncAwaitTaskLikeTests rename is a nice readability tidy-up but is unrelated to the revert, so a one-line note in the PR body would save the next archaeologist a git blame.
Right now,
mainhas precautionary serial running of some Async tests applied in #20308 in response to #20306The original increase of parallelization that was suspected of causing the instability arose in #19785 and #20258
Sample size 1, but:
Build WindowsNoRealsig_testDesktop=> 97m vs 100m for c251d06My contention is that:
SynchronizationContext-sensitive behavior, and we there's no reason to think we'd reintroduce that mode at a future point; we're more likely to move toParallelizationMode.All)resolves #20306