Skip to content

Conversation

radical
Copy link
Member

@radical radical commented May 15, 2025

The combined error message and stdout were being truncated, which meant that the former would not show, like:

Screenshot 2025-05-15 at 14 31 58

Also:

@Copilot Copilot AI review requested due to automatic review settings May 15, 2025 18:33
@github-actions github-actions bot added the area-integrations Issues pertaining to Aspire Integrations packages label May 15, 2025
@radical radical requested review from RussKie and removed request for Copilot May 15, 2025 18:33
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 ensures that the combined error message and stdout are no longer both truncated, so that the error message is fully visible while only the stdout is truncated.

  • Removed separate StringBuilder for error messages and its truncation logic.
  • Integrated error message display directly into the report, with stdout being truncated separately.
Comments suppressed due to low confidence (1)

tools/GenerateTestSummary/TestSummaryGenerator.cs:150

  • [nitpick] Verify that using '```yml' as the code block delimiter for error messages is appropriate, as the error content might not adhere to YAML formatting.
reportBuilder.AppendLine(test.Output?.ErrorInfo?.InnerText ?? string.Empty);

@radical radical requested review from danmoseley and joperezr May 15, 2025 18:34
var errorMsgBuilder = new StringBuilder();
errorMsgBuilder.AppendLine(test.Output?.ErrorInfo?.InnerText ?? string.Empty);
reportBuilder.AppendLine();
reportBuilder.AppendLine("```yml");
Copy link
Member

Choose a reason for hiding this comment

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

yml? do you mean text maybe?

Copy link
Member Author

Choose a reason for hiding this comment

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

That seems to provide a nice coloring!

radical added 2 commits May 15, 2025 15:15
feedback: `what about taking the first 25_000, the last 25_000, and removing any overlap. So all you lose is in the middle`

```
| [2025-05-15T16:36:23] Aspire.Hosting.Tests.Resources.backend Information: 29: 2025-05-15T16:36:23.8940000Z 14da29b7e816: Download complete
| [2025-05-15T16:36:23] Aspire.Hosting.Tests.Resources.backend Information: 30: 2025-05-15T16

... (snip) ...

... sources.yarp Information: 50: 2025-05-15T16:36:28.1672245Z dbug: Microsoft.Extensions.ServiceDiscovery.ServiceEndpointWatcher[1]
| [2025-05-15T16:36:28] Aspire.Hosting.Tests.Resources.yarp Information: 51: 2025-05-15T16:36:28.1672374Z       Resolving endpoints for service 'http://backend'.
```
@radical radical requested a review from mitchdenny as a code owner May 15, 2025 21:31
@radical radical merged commit 9805da3 into dotnet:main May 15, 2025
254 checks passed
@radical radical deleted the fix-reporter branch May 15, 2025 22:57
@github-actions github-actions bot locked and limited conversation to collaborators Jun 15, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-integrations Issues pertaining to Aspire Integrations packages
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants