-
Notifications
You must be signed in to change notification settings - Fork 325
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
Added Level as prefix for loggers. #967
Changes from 1 commit
593e2ab
1193cbc
84d28e3
cc29ea6
a2a7bd2
a814e07
8988ebf
406ac82
6ebeb2b
b0a915c
832276d
a6bbe7a
b671684
9557415
e2f1242
ec5d211
a2a7a0e
62e2d5b
7456c89
4e6c493
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,13 @@ namespace Microsoft.VisualStudio.TestPlatform.Utilities | |
/// </summary> | ||
public static class OutputExtensions | ||
{ | ||
/// <summary> | ||
/// Bool to decide whether Verbose level should be added as prefix or not in log messages. | ||
/// </summary> | ||
internal static bool AppendPrefix; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure if an extension methods class should have state. |
||
|
||
private const string DefaultFormat = "{0}"; | ||
|
||
/// <summary> | ||
/// Output an error message. | ||
/// </summary> | ||
|
@@ -24,7 +31,7 @@ public static void Error(this IOutput output, string format, params object[] arg | |
{ | ||
SetColorForAction(ConsoleColor.Red, () => | ||
{ | ||
Output(output, OutputLevel.Error, Resources.CommandLineError, format, args); | ||
Output(output, OutputLevel.Error, AppendPrefix ? Resources.CommandLineError : DefaultFormat, format, args); | ||
}); | ||
} | ||
|
||
|
@@ -38,7 +45,7 @@ public static void Warning(this IOutput output, string format, params object[] a | |
{ | ||
SetColorForAction(ConsoleColor.Yellow, () => | ||
{ | ||
Output(output, OutputLevel.Warning, Resources.CommandLineWarning, format, args); | ||
Output(output, OutputLevel.Warning, AppendPrefix ? Resources.CommandLineWarning : DefaultFormat, format, args); | ||
}); | ||
} | ||
|
||
|
@@ -50,7 +57,7 @@ public static void Warning(this IOutput output, string format, params object[] a | |
/// <param name="args">Arguments to format into the format string.</param> | ||
public static void Information(this IOutput output, string format, params object[] args) | ||
{ | ||
Information(output, Console.ForegroundColor, format, args); | ||
Information(output, Console.ForegroundColor, format, args); | ||
} | ||
|
||
/// <summary> | ||
|
@@ -64,7 +71,7 @@ public static void Information(this IOutput output, ConsoleColor foregroundColor | |
{ | ||
SetColorForAction(foregroundColor, () => | ||
{ | ||
Output(output, OutputLevel.Information, Resources.CommandLineInformational, format, args); | ||
Output(output, OutputLevel.Information, AppendPrefix ? Resources.CommandLineInformational : DefaultFormat, format, args); | ||
}); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,6 +45,11 @@ internal class ConsoleLogger : ITestLoggerWithParameters | |
/// </summary> | ||
public const string VerbosityParam = "verbosity"; | ||
|
||
/// <summary> | ||
/// Parameter for log message prefix | ||
/// </summary> | ||
public const string PrefixParam = "prefix"; | ||
|
||
#endregion | ||
|
||
internal enum Verbosity | ||
|
@@ -66,7 +71,6 @@ internal enum Verbosity | |
private int testsPassed = 0; | ||
private int testsFailed = 0; | ||
private int testsSkipped = 0; | ||
|
||
#endregion | ||
|
||
#region Constructor | ||
|
@@ -89,7 +93,8 @@ internal ConsoleLogger(IOutput output) | |
|
||
#endregion | ||
|
||
#region Properties | ||
#region Properties | ||
|
||
/// <summary> | ||
/// Gets instance of IOutput used for sending output. | ||
/// </summary> | ||
|
@@ -150,6 +155,12 @@ public void Initialize(TestLoggerEvents events, Dictionary<string, string> param | |
this.verbosityLevel = verbosityLevel; | ||
} | ||
|
||
var prefixExists = parameters.TryGetValue(ConsoleLogger.PrefixParam, out string prefix); | ||
if (prefixExists) | ||
{ | ||
bool.TryParse(prefix, out OutputExtensions.AppendPrefix); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this returns false, we don't need to log? |
||
} | ||
|
||
this.Initialize(events, String.Empty); | ||
} | ||
#endregion | ||
|
@@ -416,11 +427,11 @@ private void TestRunCompleteHandler(object sender, TestRunCompleteEventArgs e) | |
{ | ||
Output.Error(CommandLineResources.TestRunCanceled); | ||
} | ||
else if(e.IsAborted) | ||
else if (e.IsAborted) | ||
{ | ||
Output.Error(CommandLineResources.TestRunAborted); | ||
} | ||
else if(this.testOutcome == TestOutcome.Failed) | ||
else if (this.testOutcome == TestOutcome.Failed) | ||
{ | ||
Output.Error(CommandLineResources.TestRunFailed); | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,12 +3,10 @@ | |
|
||
namespace Microsoft.TestPlatform.CoreUtilities.UnitTests.Output | ||
{ | ||
using System; | ||
|
||
using Microsoft.VisualStudio.TestPlatform.Utilities; | ||
using Microsoft.VisualStudio.TestTools.UnitTesting; | ||
|
||
using Microsoft.VisualStudio.TestPlatform.Utilities; | ||
using Moq; | ||
using System; | ||
|
||
[TestClass] | ||
public class OutputExtensionsTests | ||
|
@@ -47,8 +45,18 @@ public void CleanUp() | |
[TestMethod] | ||
public void OutputErrorForSimpleMessageShouldOutputTheMessageString() | ||
{ | ||
this.mockOutput.Object.Error("HelloError", null); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloError", OutputLevel.Error), Times.Once()); | ||
} | ||
|
||
[TestMethod] | ||
public void OutputErrorForSimpleMessageShouldOutputTheMessageStringWithPrefixIfSet() | ||
{ | ||
OutputExtensions.AppendPrefix = true; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Do we need to validate other scenarios? Can it be a data driven test? |
||
this.mockOutput.Object.Error("HelloError", null); | ||
this.mockOutput.Verify(o => o.WriteLine("Error: HelloError", OutputLevel.Error), Times.Once()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can we use |
||
|
||
OutputExtensions.AppendPrefix = false; | ||
} | ||
|
||
[TestMethod] | ||
|
@@ -67,14 +75,14 @@ public void OutputErrorForSimpleMessageShouldSetConsoleColorToRed() | |
public void OutputErrorForMessageWithParamsShouldOutputFormattedMessage() | ||
{ | ||
this.mockOutput.Object.Error("HelloError {0} {1}", "Foo", "Bar"); | ||
this.mockOutput.Verify(o => o.WriteLine("Error: HelloError Foo Bar", OutputLevel.Error), Times.Once()); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloError Foo Bar", OutputLevel.Error), Times.Once()); | ||
} | ||
|
||
[TestMethod] | ||
public void OutputWarningForSimpleMessageShouldOutputTheMessageString() | ||
{ | ||
this.mockOutput.Object.Warning("HelloWarning", null); | ||
this.mockOutput.Verify(o => o.WriteLine("Warning: HelloWarning", OutputLevel.Warning), Times.Once()); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloWarning", OutputLevel.Warning), Times.Once()); | ||
} | ||
|
||
[TestMethod] | ||
|
@@ -93,14 +101,14 @@ public void OutputWarningForSimpleMessageShouldSetConsoleColorToYellow() | |
public void OutputWarningForMessageWithParamsShouldOutputFormattedMessage() | ||
{ | ||
this.mockOutput.Object.Warning("HelloWarning {0} {1}", "Foo", "Bar"); | ||
this.mockOutput.Verify(o => o.WriteLine("Warning: HelloWarning Foo Bar", OutputLevel.Warning), Times.Once()); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloWarning Foo Bar", OutputLevel.Warning), Times.Once()); | ||
} | ||
|
||
[TestMethod] | ||
public void OutputInformationForSimpleMessageShouldOutputTheMessageString() | ||
{ | ||
this.mockOutput.Object.Information(ConsoleColor.Green, "HelloInformation", null); | ||
this.mockOutput.Verify(o => o.WriteLine("Information: HelloInformation", OutputLevel.Information), Times.Once()); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloInformation", OutputLevel.Information), Times.Once()); | ||
} | ||
|
||
[TestMethod] | ||
|
@@ -119,7 +127,7 @@ public void OutputInformationForSimpleMessageShouldSetConsoleColorToGivenColor() | |
public void OutputInformationForMessageWithParamsShouldOutputFormattedMessage() | ||
{ | ||
this.mockOutput.Object.Information("HelloInformation {0} {1}", "Foo", "Bar"); | ||
this.mockOutput.Verify(o => o.WriteLine("Information: HelloInformation Foo Bar", OutputLevel.Information), Times.Once()); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloInformation Foo Bar", OutputLevel.Information), Times.Once()); | ||
} | ||
|
||
[TestMethod] | ||
|
@@ -137,7 +145,7 @@ public void OutputInformationShouldNotChangeConsoleOutputColor() | |
}); | ||
|
||
this.mockOutput.Object.Information("HelloInformation {0} {1}", "Foo", "Bar"); | ||
this.mockOutput.Verify(o => o.WriteLine("Information: HelloInformation Foo Bar", OutputLevel.Information), Times.Once()); | ||
this.mockOutput.Verify(o => o.WriteLine("HelloInformation Foo Bar", OutputLevel.Information), Times.Once()); | ||
Assert.IsTrue(color1 == color2); | ||
} | ||
|
||
|
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.
I am not sure if an Extension methods unit should contain state or configuration.