Skip to content
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

Remove summary when failing, and double space #47872

Merged
merged 2 commits into from
Mar 28, 2025

Conversation

nohwnd
Copy link
Member

@nohwnd nohwnd commented Mar 25, 2025

Mixed mode has more specific message and reports no test summary:

image

There is one empty line before artifacts and one before summary, instead of 2 before artifacts, and none before summary.

image

Fix https://devdiv.visualstudio.com/DevDiv/_workitems/edit/2397515 (help was fixed by Mariam previously).

@Copilot Copilot bot review requested due to automatic review settings March 25, 2025 16:34
@dotnet-issue-labeler dotnet-issue-labeler bot added the untriaged Request triage from a team member label Mar 25, 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 disables the test run summary for failing tests and adjusts the spacing in the terminal output.

  • Disables the test run summary by invoking DisableTestRunSummary() in MSBuildHandler when tests fail.
  • Adds a flag (_disableTestRunSummary) and updates the terminal output logic in TerminalTestReporter to conditionally skip the summary and include an extra empty line separating artifacts from the summary.

Reviewed Changes

Copilot reviewed 2 out of 16 changed files in this pull request and generated no comments.

File Description
src/Cli/dotnet/commands/Test/MSBuildHandler.cs Calls DisableTestRunSummary() to disable the summary when tests fail.
src/Cli/dotnet/commands/Test/Terminal/TerminalTestReporter.cs Introduces _disableTestRunSummary flag and adjusts the spacing in the output to provide a double space between artifacts and the summary.
Files not reviewed (14)
  • src/Cli/dotnet/commands/Test/LocalizableStrings.resx: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.cs.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.de.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.es.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.fr.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.it.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.ja.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.ko.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.pl.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.pt-BR.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.ru.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.tr.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.zh-Hans.xlf: Language not supported
  • src/Cli/dotnet/commands/Test/xlf/LocalizableStrings.zh-Hant.xlf: Language not supported

Copy link
Member

@Forgind Forgind left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain why it's beneficial to skip the summary?

@nohwnd
Copy link
Member Author

nohwnd commented Mar 27, 2025

That case is shown in the first screenshot, and we did feel like showing the 6 more lines of summary after the error was too confusing. The run will end with an error, and technically did not even start so user is not losing much information.

@nohwnd nohwnd merged commit 173dce4 into dotnet:main Mar 28, 2025
39 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-dotnet test untriaged Request triage from a team member
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants