Skip to content

Document [DoNotParallelize] defer-to-end ordering behaviour - #10245

Merged
Evangelink merged 2 commits into
mainfrom
dev/amauryleve/donotparallelize-ordering-docs
Jul 27, 2026
Merged

Document [DoNotParallelize] defer-to-end ordering behaviour#10245
Evangelink merged 2 commits into
mainfrom
dev/amauryleve/donotparallelize-ordering-docs

Conversation

@Evangelink

Copy link
Copy Markdown
Member

What was undocumented

[DoNotParallelize]'s XML doc described it only as "disables parallelization", but the attribute actually provides two guarantees at once:

  1. Mutual exclusion — an affected test never runs concurrently with any other test in the same source.
  2. Deferral — when in-assembly parallelization is enabled, the scheduler runs the entire parallelizable set to completion first, then runs the non-parallelizable set sequentially at the very end of that source's run.

The deferral (guarantee 2) was invisible to users. A [DoNotParallelize] test appearing last looks like a scheduling anomaly, and the attribute's true cost — a deferred test cannot overlap with anything, so its duration is added to the run rather than absorbed by parallel work — is easy to underestimate.

This PR rewrites the XML doc to state both guarantees plainly, keeping the "per test source (assembly)" scope explicit so "runs last" is not misread in multi-source runs.

Cancellation interaction (new even for those who knew about the deferral)

Because the deferred set runs only after the parallelizable phase, a run canceled during the parallelizable phase can complete without executing any deferred tests. Traced in source: workers swallow OperationCanceledException so Task.WhenAll completes normally and control reaches the deferred block, but ExecuteTestsWithTestRunnerAsync calls ThrowIfCancellationRequested() before each test, so the first deferred test throws before running. "Runs last" and "may not run at all when canceled early" are both true — documented as such.

Documentation-only — no behaviour change

This PR changes no scheduler behaviour. The deferral is long-standing shipped behaviour; the goal is to describe it accurately, not to change it. The only code change is the acceptance test below.

Acceptance test pins the ordering

Adds a deterministic (timing-free) acceptance test in ParallelExecutionTests.cs: parallelizable tests increment a shared in-process counter, and the [DoNotParallelize] test asserts the counter already equals the full expected count — so it can only pass if it ran after all parallelizable tests finished. This prevents a future refactor from silently changing the ordering.

Verified it can actually fail: against a deliberately reordered scheduler (deferred set run first) the test fails on all three TFMs (expected 4, actual 0); reverting restores green (net8.0 / net10.0 / net462).

RFC 004 intentionally left untouched

docs/RFCs/004-In-Assembly-Parallel-Execution.md already documents the deferral (lines 66-68), so it was intentionally not modified. The gap was in the XML doc, not the RFC.

Follow-up: dotnet/docs

The public docs on learn.microsoft.com describe [DoNotParallelize] only as disabling parallelization and omit both the deferral and the cancellation behaviour. Recommend filing a dotnet/docs issue to update them — not done here (no cross-repo PR).

Rewrite the DoNotParallelizeAttribute XML doc to describe both guarantees
(mutual exclusion and deferral to the end of the source's run), the actionable
tail-latency cost, that deferred tests may not run at all when a run is
canceled during the parallel phase, the no-op behaviour when parallelization
is disabled, the three placements, and the relationship to [ResourceLock]
(precedence wording aligned with ResourceLockAttribute). Add a deterministic
acceptance test pinning the defer-to-end ordering.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 25872624-3e9c-4f84-b3c2-5ed114d8e7d2
Copilot AI review requested due to automatic review settings July 27, 2026 08:04

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Documents [DoNotParallelize] ordering and cancellation behavior and adds an acceptance test for deferred execution.

Changes:

  • Expands XML documentation for scheduling guarantees and scope.
  • Adds a multi-targeted ordering acceptance test.
Show a summary per file
File Description
DoNotParallelizeAttribute.cs Documents exclusion, deferral, cancellation, and resource-lock behavior.
ParallelExecutionTests.cs Verifies deferred tests run after parallelizable tests.

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 1
  • Review effort level: Medium

Comment thread src/TestFramework/TestFramework/Attributes/Lifecycle/DoNotParallelizeAttribute.cs Outdated
@github-actions

This comment has been minimized.

Address PR review: the deferred-test cost extends the source's critical path, not necessarily the whole multi-source run; a DoNotParallelize test can still overlap with tests from other sources (which may run in separate hosts).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

Copilot-Session: 25872624-3e9c-4f84-b3c2-5ed114d8e7d2
Copilot AI review requested due to automatic review settings July 27, 2026 09:48

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review details

  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Medium

@github-actions

Copy link
Copy Markdown
Contributor

🧪 Test quality grade — PR #10245

GradeTestMutationNotesHow to improve
A (90–100) new ParallelExecutionTests.
DoNotParallelizeTestRunsAfterAllParallelizableTests
3/4 killed Two meaningful assertions (exit code + success count) deterministically verify the defer-to-end scheduling guarantee via the counter mechanism in the test asset.

This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Re-run with
/grade-tests.

🤖 Automated content by GitHub Copilot. Generated by the Grade Tests on PR (on open / sync) workflow. · sonnet46 51.7 AIC · ⌖ 5.84 AIC · ⊞ 10.3K · [◷]( · )

@Evangelink
Evangelink merged commit 6b31b73 into main Jul 27, 2026
39 checks passed
@Evangelink
Evangelink deleted the dev/amauryleve/donotparallelize-ordering-docs branch July 27, 2026 11:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

state/needs-review Awaiting review from the team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants