Enable method-level parallelization for true unit test projects#55090
Open
Evangelink wants to merge 5 commits into
Open
Enable method-level parallelization for true unit test projects#55090Evangelink wants to merge 5 commits into
Evangelink wants to merge 5 commits into
Conversation
The repo-wide default (test/Directory.Build.props) sets MSTestParallelizeScope to None, fully serializing every MSTest project. The containerize.UnitTests and Microsoft.NET.Build.Containers.UnitTests projects are genuine unit tests with no shared process-global state, so opt them back in to MethodLevel parallelization. The two classes in Containers.UnitTests that mutate process-global environment variables (AuthHandshakeMessageHandlerTests, DockerDaemonTests) are marked [DoNotParallelize] so they run serially under the method-level pool. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This pull request opts two MSTest.Sdk-based unit test projects back into method-level parallel execution by overriding the repo-wide MSTestParallelizeScope=None default, and adds [DoNotParallelize] to the few test classes that mutate process-global environment variables.
Changes:
- Enable
MSTestParallelizeScope=MethodLevelfortest/containerize.UnitTests. - Enable
MSTestParallelizeScope=MethodLevelfortest/Microsoft.NET.Build.Containers.UnitTests. - Mark environment-variable-mutating test classes as
[DoNotParallelize]in the containers unit test project.
Show a summary per file
| File | Description |
|---|---|
| test/Microsoft.NET.Build.Containers.UnitTests/Microsoft.NET.Build.Containers.UnitTests.csproj | Opt the project into MSTest method-level parallelization via MSTestParallelizeScope=MethodLevel. |
| test/Microsoft.NET.Build.Containers.UnitTests/DockerDaemonTests.cs | Add [DoNotParallelize] to avoid concurrency issues due to DOCKER_HOST mutation. |
| test/Microsoft.NET.Build.Containers.UnitTests/AuthHandshakeMessageHandlerTests.cs | Add [DoNotParallelize] to avoid concurrency issues due to registry-related env var mutations. |
| test/containerize.UnitTests/containerize.UnitTests.csproj | Opt the project into MSTest method-level parallelization via MSTestParallelizeScope=MethodLevel. |
Copilot's findings
- Files reviewed: 4/4 changed files
- Comments generated: 2
Evangelink
commented
Jul 1, 2026
Evangelink
commented
Jul 1, 2026
Address review feedback: capture and restore process-global environment variables in a finally block so a thrown assertion or a value already set on the test host cannot leak into subsequent tests. - GetDockerCredentialsFromEnvironment_ReturnsCorrectValues now restores the registry credential vars in finally. - Authenticate now captures and restores REGISTRY_AUTH_FILE. - DockerDaemonTests restores the original DOCKER_HOST instead of clearing it. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Member
|
Is it reasonable to request more than one validation run prior to merging to validate stability in the affected tests? |
Member
Author
Absolutely! I'll ping when I have 5 passing runs |
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.
What
Opt two genuine unit-test projects back in to method-level test parallelization:
test/containerize.UnitTeststest/Microsoft.NET.Build.Containers.UnitTestsThe repo-wide default in
test/Directory.Build.propssetsMSTestParallelizeScope=None, which emits[assembly: DoNotParallelize]and fully serializes every MSTest project. These two projects are true unit tests (no disk/CWD dependence, no shared process-global state), so they can safely run methods in parallel.Why it's safe
I audited both projects for shared-state hazards (environment variables, current-directory mutation, fixed temp paths, mutable statics):
containerize.UnitTests — single class, pure parsing tests; temp dirs are keyed per test. No shared state.
Microsoft.NET.Build.Containers.UnitTests — the only real hazard is two classes that mutate process-global environment variables. Those are now marked
[DoNotParallelize]so they run in MSTest's serial phase:AuthHandshakeMessageHandlerTests(registry credential vars +REGISTRY_AUTH_FILE)DockerDaemonTests(DOCKER_HOST)The rest use fakes (
RegistryTests'SetEnvironmentVariablethrows), read-only statics, and per-testHttpClientinstances.Verification
Both projects build clean (0 warnings, with
MSTestAnalysisMode=Recommendedactive) and the generatedAssemblyInfoemits[assembly: Parallelize(Scope = MethodLevel)]. Full test execution runs in CI (the repo's self-built runtime isn't available locally).