-
Notifications
You must be signed in to change notification settings - Fork 323
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
dotnet test output with color #641
Conversation
@smadala, |
scripts/test.ps1
Outdated
|
||
[Parameter(Mandatory=$false)] | ||
[Alias("d")] | ||
[System.String] $ConsoleLogger = '/logger:"console;verbosity=minimal"' |
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.
nit: Just make this take the arguments - "verbosity=minimal"
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.
There is already a variable -verbose
. Can we use that value and set this?
Output.WriteLine(errorMessage, OutputLevel.Information); | ||
using (new ConsoleColorHelper(ConsoleColor.Red)) | ||
{ | ||
Output.WriteLine(errorMessage, OutputLevel.Error); |
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.
nit: how about creating a small method for this?
@@ -33,23 +33,15 @@ public int Execute() | |||
FileName = hostExe, | |||
Arguments = string.Join(" ", this.allArgs), | |||
UseShellExecute = false, | |||
CreateNoWindow = true, |
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.
What are the scenarios we have tried?
dotnet test
in cmd, powershell, bashdotnet test
from Process.CreateProcess of any app- Mimic how
vstest.console
is called from VSTS task or a jenkins/travis or any CI system
We should ensure that CI tasks that run as services (and thus not interactive) don't face any issues with this approach.
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.
Also try "dotnet watch test" and make sure the color flows all the way through.
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.
@codito dotnet test
working fine for above three scenarios. No color is displaying for dotnet test
on VSTS task (same behavior as dotnet build
).
@shanselman Yes, dotnet watch test
displaying colored output.
}; | ||
|
||
Tracing.Trace("VSTest: Starting vstest.console..."); | ||
Tracing.Trace("VSTest: Arguments: " + processInfo.FileName + " " + processInfo.Arguments); | ||
|
||
using (var activeProcess = new Process { StartInfo = processInfo }) | ||
{ | ||
activeProcess.OutputDataReceived += (sender, args) => Console.WriteLine(args.Data); |
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.
Good change 👍
* Set Console color using action rather than "using"
/// <param name="args">Arguments to format into the format string.</param> | ||
public static void Information(this IOutput output, string format, params object[] args) | ||
public static void Information(this IOutput output, string format, ConsoleColor foregroundColor=ConsoleColor.White, params object[] args) |
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.
We don't need to give the color arg for Error and Warning extension methods.
Probably, we can have overload for Information that takes in color ?
@@ -21,42 +22,42 @@ public void TestInit() | |||
[TestMethod] | |||
public void OutputErrorForSimpleMessageShouldOutputTheMessageString() |
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.
Add tests for verifying the color
Also try "dotnet watch test" and make sure the color flows all the way through. |
Related: #639