Skip to content

chore(Test): Revert #20308 precautionary serial testing - #20343

Open
bartelink wants to merge 1 commit into
dotnet:mainfrom
bartelink:remove-cautionary-isolation
Open

chore(Test): Revert #20308 precautionary serial testing#20343
bartelink wants to merge 1 commit into
dotnet:mainfrom
bartelink:remove-cautionary-isolation

Conversation

@bartelink

@bartelink bartelink commented Aug 25, 2026

Copy link
Copy Markdown
Contributor

Right now, main has precautionary serial running of some Async tests applied in #20308 in response to #20306

The 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 c251d06

My contention is that:

  • the key instability that was present was that a failsafe timeout on a wait in a new test was 5s, as opposed to the 30s as we learned makes sense for heavily loaded CI boxes running parallelized tests
  • whether or not those Async/Task suites are run concurrently or not is not a factor (xunit conservative mode doesn't have 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 to ParallelizationMode.All)

resolves #20306

Copilot AI lite review requested due to automatic review settings August 25, 2026 10:05
@bartelink
bartelink requested a review from a team as a code owner August 25, 2026 10:05

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@github-actions

Copy link
Copy Markdown
Contributor

✅ No release notes required

@bartelink bartelink changed the title chore(Test): Rollback precautionary serial testing chore(Test): Revert #20308 precautionary serial testing Aug 25, 2026
@github-actions github-actions Bot added the AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files label Aug 25, 2026

@T-Gro T-Gro left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 AsyncTaskLikeAwaitTestsAsyncAwaitTaskLikeTests 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.

@github-project-automation github-project-automation Bot moved this from New to In Progress in F# Compiler and Tooling Aug 28, 2026
@T-Gro T-Gro added the AI-reviewed PR reviewed by AI review council label Aug 28, 2026
@T-Gro
T-Gro self-requested a review August 28, 2026 05:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AI-reviewed PR reviewed by AI review council AI-Tooling-Check-Scanned-Clean Tooling check: diff analyzed, no interesting infrastructure files

Projects

Status: In Progress

Development

Successfully merging this pull request may close these issues.

CI timeouts and flakiness related to new Async tests.

3 participants