[release/9.0-staging] Backport "Dispose Xunit ToolCommand" #116685
Merged
Conversation
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
There was a problem hiding this comment.
Pull Request Overview
This PR is a backport to release/9.0-staging that improves resource management in various test commands by refactoring to use using declarations and ensuring proper disposal of command objects. Key changes include refactoring DotNetCommand, RunCommand, and ToolCommand usages to ensure disposal, and adding an isDisposed flag to ToolCommand to prevent post‑disposal events from processing.
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/mono/wasm/Wasm.Build.Tests/WasmTemplateTestBase.cs | Refactored DotNetCommand usage to a using block for proper disposal. |
| src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/WasmAppBuilderDebugLevelTests.cs | Updated RunCommand usage with a using declaration. |
| src/mono/wasm/Wasm.Build.Tests/TestAppScenarios/SatelliteLoadingTests.cs | Converted DotNetCommand to using block for resource management. |
| src/mono/wasm/Wasm.Build.Tests/Templates/WasmTemplateTests.cs | Refactored multiple RunCommand and DotNetCommand usages to using blocks. |
| src/mono/wasm/Wasm.Build.Tests/Templates/NativeBuildTests.cs | Updated command execution to use using blocks. |
| src/mono/wasm/Wasm.Build.Tests/NonWasmTemplateBuildTests.cs | Refactored DotNetCommand usage to use a using block with ToolCommand. |
| src/mono/wasm/Wasm.Build.Tests/Common/ToolCommand.cs | Added an isDisposed flag and updated event handlers to check disposal state. |
| src/mono/wasm/Wasm.Build.Tests/BuildTestBase.cs | Converted DotNetCommand usage to a using block via ToolCommand. |
| src/mono/wasm/Wasm.Build.Tests/Blazor/MiscTests3.cs, MiscTests2.cs, CleanTests.cs, BlazorWasmTestBase.cs | Consistently applied using patterns to improve resource cleanup in test scenarios. |
maraf
approved these changes
Jun 16, 2025
This was referenced Jun 16, 2025
Co-authored-by: Marek Fišera <[email protected]>
This was referenced Jun 16, 2025
Open
jeffschwMSFT
approved these changes
Jun 17, 2025
jeffschwMSFT
added
Servicing-approved
|
Tagging subscribers to 'arch-wasm': @lewing |
|
/ba-g Build analysis is red, without failures
|
ilonatommy
merged commit 22cf0cd
into
dotnet:release/9.0-staging
30 of 34 checks passed
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.
Backport of #108319, #108387 to release/9.0-staging
/cc @lewing
Customer Impact
This is a tell mode.
Merging these changes helped reduce #105315 hit on main. We're still having problems with net9 branch. This PR should mitigate it.
Regression
Testing
Manual testing.
Risk
Low. This is a change to tests only.