Aggregate Inner Exception Messages and Stacktraces #3784
Conversation
thomhurst
requested review from
MarcoRossignoli and
Evangelink
as code owners
|
@nohwnd can you review? |
There was a problem hiding this comment.
We can merge as is (once the regex change question is answered).
But my preferred way of presenting multiple errors would be more along the lines of:
Failed with 3 errors:
[1] PlaygroundWrappedException: exception message
Stack Trace:
at ...
[2] Exception: exception message
Stack Trace:
at ...
[3] Exception: exception message
Stack Trace:
at ...
What do you think?
| @@ -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)] | |||
There was a problem hiding this comment.
There should always be exactly 3 spaces in the stack frame, is this change needed or just done to make the regex more regexy?
There was a problem hiding this comment.
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
| _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), |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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
Fix #3783
After:
I don't know if this actually works... I was trying to run
build.cmd -pack -test -integrationTestbut I just get:And I don't know how to resolve it. I've installed the aspire workload and ran workload repair but still getting it.