Uh oh!
There was an error while loading. Please reload this page.
[ResourceLock] Replace class-level DoNotParallelize with ResourceLock(Console) in TerminalOutputDeviceTests - #10790
Draft
nohwnd-bot wants to merge 1 commit into
Conversation
…rminalOutputDeviceTests Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Replaces broad test-class serialization with resource-specific locking for process-global console access.
Changes:
- Adds
[ResourceLock(WellKnownResources.Console)]. - Removes the redundant
SemaphoreSlimand manual lock handling.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
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 freeto 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.
gh-aw-workflow-id: resource-lock-refactoring
Summary
Replaces the class-level
[DoNotParallelize]onTerminalOutputDeviceTests(
test/UnitTests/Microsoft.Testing.Platform.UnitTests/OutputDevice/TerminalOutputDeviceTests.cs)with
[ResourceLock(WellKnownResources.Console)].Project and current scope:
Microsoft.Testing.Platform.UnitTestsalreadydeclares
[assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)]in
Program.cs, so this project is already parallelized at the method level.TerminalOutputDeviceTestswas one of the few classes still opting out entirelyvia a class-level
[DoNotParallelize].Shared resource and conflicting tests: every test method in the class
temporarily redirects the process-global
Console.Errorstream(
Console.SetError(...)) to capture output, then restores the original writerin a
finallyblock. Six methods in the class do this(
AssertListTestsJsonStandardErrorAsync/CaptureListTestsJsonStandardErrorAsyncand
InitializeAndCaptureStandardErrorAsync, used by 8+[TestMethod]s). SinceConsole.Erroris process-global, two tests in this class running concurrentlyunder
MethodLevelparallelization could redirect/restore the stream out oforder and corrupt each other's captured output. The class previously guarded
this with a hand-rolled
SemaphoreSlim ConsoleErrorSemaphoreplus[DoNotParallelize]at the class level — a strictly broader mechanism thanneeded, since it serializes the whole class rather than just the
Console.Errorcritical sections.Why this attribute placement is minimal:
[ResourceLock(WellKnownResources.Console)]at the class level lets the MSTest engine serialize only tests contending on the
same declared resource (
System.Console), instead of forcing full sequentialexecution of the class via
[DoNotParallelize]. This is the narrowestclass-level fix because every test method in the class touches
Console.Error,so a method-level placement would be equivalent but more verbose; there is no
test in the class that doesn't need the lock. The now-redundant
ConsoleErrorSemaphorefield and itsWaitAsync/Releasecalls were removedsince
[ResourceLock]already provides the serialization guarantee at theMSTest engine level.
No assembly-level
Parallelize/DoNotParallelizeattributes,.runsettings,or
testconfig.jsonparallelization settings were touched.Validation
bash ./build.sh— build succeeded (0 errors, 0 warnings).PATH="$PWD/.dotnet:$PATH" dotnet run --project test/UnitTests/Microsoft.Testing.Platform.UnitTests -f net9.0 --no-build -c Debug -- --filter "FullyQualifiedName~TerminalOutputDeviceTests"— 10/10 tests passed under the assembly's actualMethodLevelparallel scheduling (Test Parallelization enabled ... Workers: 4, Scope: MethodLevel)../build.sh -pack) was not run: this change only touches a unit-test project and does not affect any acceptance test asset or packed artifact.Workflow run: https://github.com/microsoft/testfx/actions/runs/33050722434
DOTNET_CLI_TELEMETRY_SESSIONIDused during validation:gha-65080725-33050722434-1.Warning
Firewall blocked 1 domain
The following domain was blocked by the firewall during workflow execution:
southcentralus0.in.applicationinsights.azure.comTo allow these domains, add them to the
network.allowedlist in your workflow frontmatter:See Network Configuration for more information.