-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Adjust dotnet test UX (for MTP) #48797
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves the dotnet test user experience by adjusting retry feature reporting and test progress tracking. Key changes include:
- Adding a new test method for the retry feature.
- Refactoring event handlers to use TryCount instead of tracking instance IDs.
- Updating test progress state logic to recalculate and report test outcomes based on retries.
Reviewed Changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 1 comment.
Show a summary per file
File | Description |
---|---|
test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs | Added a new test method for verifying retry behavior. |
test/TestAssets/TestProjects/TestAppSimpleWithRetry/TestClass1.cs | Introduced a test project to simulate retry scenarios. |
src/Cli/dotnet/Commands/Test/TestApplicationEventHandlers.cs | Updated event handling to remove instance ID tracking and adjust retry logic. |
src/Cli/dotnet/Commands/Test/Terminal/TestProgressState.cs | Introduced dictionary-based test result tracking and modified reporting methods. |
src/Cli/dotnet/Commands/Test/Terminal/TerminalTestReporter.cs | Refactored retry count usage and updated test completion reporting. |
Files not reviewed (2)
- test/TestAssets/TestProjects/TestAppSimpleWithRetry/TestAppSimpleWithRetry.csproj: Language not supported
- test/TestAssets/TestProjects/TestAppSimpleWithRetry/dotnet.config: Language not supported
Comments suppressed due to low confidence (2)
test/dotnet.Tests/CommandTests/Test/GivenDotnetTestBuildsAndRunsTests.cs:45
- [nitpick] The method name 'RunTestProjectWithWithRetryFeature_ShouldSucceed' contains a duplicate 'With' which could be confusing. Consider renaming it to 'RunTestProjectWithRetryFeature_ShouldSucceed'.
public void RunTestProjectWithWithRetryFeature_ShouldSucceed(string configuration)
src/Cli/dotnet/Commands/Test/Terminal/TestProgressState.cs:131
- [nitpick] Incrementing PassedTests in DiscoverTest could lead to double counting if test results are also reported elsewhere. Verify that counting discovered tests as passed is intentional or adjust the logic accordingly.
public void DiscoverTest(string displayName, string uid) { PassedTests++; DiscoveredTests.Add(new(displayName, uid)); }
Related to #45927
Previously, we were calling AssemblyRunStarted twice for a single assembly for the case where a test host controller is involved (e.g, an extension such as crash dump, etc). In this case, both the test host and test host controller will handshake with us. This PR modifies the logic to only call AssemblyRunStarted when we are handshaking with the test host. I'm also refactoring a bit the retry logic.