8000 Adjust dotnet test UX (for MTP) by Youssef1313 · Pull Request #48797 · dotnet/sdk · GitHub
[go: up one dir, main page]

Skip to content

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

Merged
merged 1 commit into from
May 2, 2025
Merged

Adjust dotnet test UX (for MTP) #48797

merged 1 commit into from
May 2, 2025

Conversation

Youssef1313
Copy link
Member
@Youssef1313 Youssef1313 commented May 2, 2025

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.

@Copilot Copilot AI review requested due to automatic review settings May 2, 2025 07:43
@Youssef1313 Youssef1313 changed the title Adjust dotnet test UX Adjust dotnet test UX (for MTP) May 2, 2025
Copy link
Contributor
@Copilot Copilot AI left a 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)); }

@Youssef1313 Youssef1313 merged commit d93c427 into main May 2, 2025
31 checks passed
@Youssef1313 Youssef1313 deleted the dev/ygerges/fix-ux branch May 2, 2025 10:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants
0