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.

Sign up for GitHub

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

Jump to bottom

Aggregate Inner Exception Messages and Stacktraces #3784

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Aggregate Inner Exception Messages and Stacktraces #3784

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
cbde275
Aggregate Inner Exception Messages and Stacktraces
thomhurst Sep 7, 2024
2df9965
Remove redundant parts from test
thomhurst Sep 7, 2024
822d400
Update TerminalTestReporter.cs
thomhurst Sep 7, 2024
b9270e2
Update TerminalTestReporter.cs
thomhurst Sep 7, 2024
8c63809
Update TerminalTestReporter.cs
thomhurst Sep 7, 2024
c08dd26
Update TerminalOutputDevice.cs
thomhurst Sep 7, 2024
cc3c846
Tweak
thomhurst Sep 8, 2024
64a0033
Format
thomhurst Sep 8, 2024
62e63b5
Fix
thomhurst Sep 8, 2024
025ce48
Fix test assertion
thomhurst Sep 8, 2024
629bf3f
Fix
thomhurst Sep 8, 2024
db8d816
Fix
thomhurst Sep 8, 2024
3cd8cbd
Tweak
thomhurst Sep 8, 2024
5814aca
Remove test
thomhurst Sep 8, 2024
7b53b40
Tweak
thomhurst Sep 8, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions src/Platform/Microsoft.Testing.Platform/OutputDevice/Terminal/TerminalTestReporter.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ internal sealed partial class TerminalTestReporter : IDisposable
private bool _wasCancelled;

#if NET7_0_OR_GREATER
[GeneratedRegex(@$"^ at ((?<code>.+) in (?<file>.+):line (?<line>\d+)|(?<code1>.+))$", RegexOptions.ExplicitCapture, 1000)]
[GeneratedRegex(@$"^\s*at ((?<code>.+) in (?<file>.+):line (?<line>\d+)|(?<code1>.+))$", RegexOptions.ExplicitCapture, 1000)]
Copy link
Member

Choose a reason for hiding this comment

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

There should always be exactly 3 spaces in the stack frame, is this change needed or just done to make the regex more regexy?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When testing I think the first line of inner exceptions weren't rendering properly without this change. Might be that I messed up formatting then

private static partial Regex GetFrameRegex();
#else
private static Regex? s_regex;
Expand Down Expand Up @@ -190,8 +190,8 @@ private void AppendTestRunSummary(ITerminal terminal)
{
terminal.AppendLine();

IEnumerable<IGrouping<bool, TestRunArtifact>> artifactGroups = _artifacts.GroupBy(a => a.OutOfProcess);
if (artifactGroups.Any())
IGrouping<bool, TestRunArtifact>[] artifactGroups = _artifacts.GroupBy(a => a.OutOfProcess).ToArray();
if (artifactGroups.Length != 0)
{
terminal.AppendLine();
}
Expand Down
92 changes: 84 additions & 8 deletions src/Platform/Microsoft.Testing.Platform/OutputDevice/TerminalOutputDevice.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -392,6 +392,10 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella
return;
}

Exception[] exceptions = [];
IEnumerable<string> exceptionMessages = [];
IEnumerable<string> exceptionStacktraces = [];

switch (value)
{
case TestNodeUpdateMessage testNodeStateChanged:
Expand All @@ -404,57 +408,73 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella
break;

case ErrorTestNodeStateProperty errorState:
exceptions = GetExceptionsInThrownOrder(errorState.Exception);
exceptionMessages = GetExceptionMessages(exceptions, errorState.Explanation);
exceptionStacktraces = GetExceptionStackTraces(exceptions);

_terminalTestReporter.TestCompleted(
_assemblyName,
_targetFramework,
_shortArchitecture,
testNodeStateChanged.TestNode.DisplayName,
TestOutcome.Error,
duration,
errorMessage: errorState.Exception?.Message ?? errorState.Explanation,
errorState.Exception?.StackTrace,
errorMessage: string.Join(_environment.NewLine, exceptionMessages),
errorStackTrace: string.Join(_environment.NewLine, exceptionStacktraces),
expected: null,
actual: null);
break;

case FailedTestNodeStateProperty failedState:
exceptions = GetExceptionsInThrownOrder(failedState.Exception);
exceptionMessages = GetExceptionMessages(exceptions, failedState.Explanation);
exceptionStacktraces = GetExceptionStackTraces(exceptions);

_terminalTestReporter.TestCompleted(
_assemblyName,
_targetFramework,
_shortArchitecture,
testNodeStateChanged.TestNode.DisplayName,
TestOutcome.Fail,
duration,
errorMessage: failedState.Exception?.Message ?? failedState.Explanation,
failedState.Exception?.StackTrace,
errorMessage: string.Join(_environment.NewLine, exceptionMessages),
errorStackTrace: string.Join(_environment.NewLine, exceptionStacktraces),
expected: failedState.Exception?.Data["assert.expected"] as string,
actual: failedState.Exception?.Data["assert.actual"] as string);
break;

case TimeoutTestNodeStateProperty timeoutState:
exceptions = GetExceptionsInThrownOrder(timeoutState.Exception);
exceptionMessages = GetExceptionMessages(exceptions, timeoutState.Explanation);
exceptionStacktraces = GetExceptionStackTraces(exceptions);

_terminalTestReporter.TestCompleted(
_assemblyName,
_targetFramework,
_shortArchitecture,
testNodeStateChanged.TestNode.DisplayName,
TestOutcome.Timeout,
duration,
errorMessage: timeoutState.Exception?.Message ?? timeoutState.Explanation,
timeoutState.Exception?.StackTrace,
errorMessage: string.Join(_environment.NewLine, exceptionMessages),
errorStackTrace: string.Join(_environment.NewLine, exceptionStacktraces),
expected: null,
actual: null);
break;

case CancelledTestNodeStateProperty cancelledState:
exceptions = GetExceptionsInThrownOrder(cancelledState.Exception);
exceptionMessages = GetExceptionMessages(exceptions, cancelledState.Explanation);
exceptionStacktraces = GetExceptionStackTraces(exceptions);

_terminalTestReporter.TestCompleted(
_assemblyName,
_targetFramework,
_shortArchitecture,
testNodeStateChanged.TestNode.DisplayName,
TestOutcome.Canceled,
duration,
errorMessage: cancelledState.Exception?.Message ?? cancelledState.Explanation,
cancelledState.Exception?.StackTrace,
errorMessage: string.Join(_environment.NewLine, exceptionMessages),
Copy link
Member

Choose a reason for hiding this comment

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

the receiving api can be changed to take array of strings, (we use it both here and behind a remote call in dotnet test so we don't want exception objects).

That will allow us to render the erros and stack traces under each other, so you don't have to jump back and forth.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sounds good. Do you want to take over this pr because I can't actually build it locally so it makes Dev a bit harder

Copy link
Member

Choose a reason for hiding this comment

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

Sure.

errorStackTrace: string.Join(_environment.NewLine, exceptionStacktraces),
expected: null,
actual: null);
break;
Expand Down Expand Up @@ -536,6 +556,62 @@ public async Task ConsumeAsync(IDataProducer dataProducer, IData value, Cancella
}
}

private static IEnumerable<string> GetExceptionMessages(Exception[] exceptions, string? fallback)
{
if (exceptions.Length == 0)
{
if (fallback != null)
{
yield return fallback;
}

yield break;
}

for (int index = 0; index < exceptions.Length; index++)
{
Exception exception = exceptions[index];

if (exception.StackTrace == null)
{
continue;
}

string prefix = index == 0 ? string.Empty : " ---> ";
yield return $"{prefix}{exception.GetType().FullName}: {exception.Message}";
}
}

private IEnumerable<string> GetExceptionStackTraces(Exception[] exceptions)
{
for (int index = 0; index < exceptions.Length; index++)
{
Exception exception = exceptions[index];

if (exception.StackTrace == null)
{
continue;
}

string suffix = index != exceptions.Length - 1
? $"{_environment.NewLine} --- End of inner exception stack trace ---"
: string.Empty;

yield return $"{exception.StackTrace.Trim()}{suffix}";
}
}

private static Exception[] GetExceptionsInThrownOrder(Exception? exception) => GetExceptions(exception).Reverse().ToArray();

private static IEnumerable<Exception> GetExceptions(Exception? exception)
{
while (exception is not null)
{
yield return exception;
exception = exception.InnerException;
}
}

public void Dispose()
=> _terminalTestReporter?.Dispose();
}
Loading