-
Notifications
You must be signed in to change notification settings - Fork 418
New issue
Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? # to your account
Return the stdout messages to improve test logging #1203
Conversation
ErrorStackTrace = testResult.ErrorStackTrace | ||
ErrorStackTrace = testResult.ErrorStackTrace, | ||
StdOutMessages = testResult.Messages | ||
.Where(message => message.Category == TestResultMessage.StandardOutCategory) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we be picking up standard error here too?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And keeping it in its own category
@@ -6,5 +6,6 @@ public class DotNetTestResult | |||
public string Outcome { get; set; } | |||
public string ErrorMessage { get; set; } | |||
public string ErrorStackTrace { get; set; } | |||
public string[] StdOutMessages {get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StandardOutput
@@ -6,6 +6,7 @@ public class DotNetTestResult | |||
public string Outcome { get; set; } | |||
public string ErrorMessage { get; set; } | |||
public string ErrorStackTrace { get; set; } | |||
public string[] StdOutMessages {get; set; } | |||
public string[] StandardOutputMessages { get; set; } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
StandardOutput
(no "messages")
@@ -240,7 +240,12 @@ public override RunTestResponse RunTest(string[] methodNames, string testFramewo | |||
MethodName = testResult.TestCase.FullyQualifiedName, | |||
Outcome = testResult.Outcome.ToString().ToLowerInvariant(), | |||
ErrorMessage = testResult.ErrorMessage, | |||
ErrorStackTrace = testResult.ErrorStackTrace | |||
ErrorStackTrace = testResult.ErrorStackTrace, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is it possible to unit test this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good!
dotnet test also prints the stdout messages. So added a property to
DotNetTestResult
to return an array of these messages.omnisharp-vscode side : dotnet/vscode-csharp#2343
Please review : @rchande @DustinCampbell