Skip to content

Commit

Permalink
Remove duplicate counting of test results in Consolelogger (#2267)
Browse files Browse the repository at this point in the history 
* Remove duplicate counting of test result outcome in Consolelogger

* Fixed order tests accptance test

* Remove comment and ignore out value

* Compare to empty Guid

Co-authored-by: Jakub Jareš <[email protected]>
  • Loading branch information
@hvinett @nohwnd
hvinett and nohwnd committed Jan 2, 2020
1 parent 33531d4 commit 5ef1f3b
Show file tree
Hide file tree
Showing 3 changed files with 125 additions and 12 deletions.
90 changes: 79 additions & 11 deletions src/vstest.console/Internal/ConsoleLogger.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Microsoft.VisualStudio.TestPlatform.CommandLine.Internal
{
using System;
using System.Collections.Concurrent;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
Expand Down Expand Up @@ -85,6 +86,16 @@ internal class ConsoleLogger : ITestLoggerWithParameters
/// </summary>
public const string ProgressIndicatorParam = "progress";

/// <summary>
/// Property Id storing the ParentExecutionId.
/// </summary>
public const string ParentExecutionIdPropertyIdentifier = "ParentExecId";

/// <summary>
/// Property Id storing the ExecutionId.
/// </summary>
public const string ExecutionIdPropertyIdentifier = "ExecutionId";

#endregion

internal enum Verbosity
Expand All @@ -107,11 +118,8 @@ internal enum Verbosity
private Verbosity verbosityLevel = Verbosity.Minimal;
#endif

private int testsTotal = 0;
private int testsPassed = 0;
private int testsFailed = 0;
private int testsSkipped = 0;
private bool testRunHasErrorMessages = false;
private ConcurrentDictionary<Guid, TestOutcome> leafExecutionIdAndTestOutcomePairDictionary = new ConcurrentDictionary<Guid, TestOutcome>();

#endregion

Expand Down Expand Up @@ -371,6 +379,39 @@ private static void DisplayFullInformation(TestResult result)
}
}

/// <summary>
/// Returns the parent Execution id of given test result.
/// </summary>
/// <param name="testResult"></param>
/// <returns></returns>
private Guid GetParentExecutionId(TestResult testResult)
{
var parentExecutionIdProperty = testResult.Properties.FirstOrDefault(property =>
property.Id.Equals(ParentExecutionIdPropertyIdentifier));
return parentExecutionIdProperty == null
? Guid.Empty
: testResult.GetPropertyValue(parentExecutionIdProperty, Guid.Empty);
}

/// <summary>
/// Returns execution id of given test result
/// </summary>
/// <param name="testResult"></param>
/// <returns></returns>
private Guid GetExecutionId(TestResult testResult)
{
var executionIdProperty = testResult.Properties.FirstOrDefault(property =>
property.Id.Equals(ExecutionIdPropertyIdentifier));
var executionId = Guid.Empty;

if (executionIdProperty != null)
{
executionId = testResult.GetPropertyValue(executionIdProperty, Guid.Empty);
}

return executionId.Equals(Guid.Empty) ? Guid.NewGuid() : executionId;
}

#endregion

#region Event Handlers
Expand Down Expand Up @@ -468,9 +509,6 @@ private void TestResultHandler(object sender, TestResultEventArgs e)
ValidateArg.NotNull<object>(sender, "sender");
ValidateArg.NotNull<TestResultEventArgs>(e, "e");

// Update the test count statistics based on the result of the test.
this.testsTotal++;

var testDisplayName = e.Result.DisplayName;

if (string.IsNullOrWhiteSpace(e.Result.DisplayName))
Expand All @@ -484,11 +522,20 @@ private void TestResultHandler(object sender, TestResultEventArgs e)
testDisplayName = string.Format("{0} [{1}]", testDisplayName, formattedDuration);
}

var executionId = GetExecutionId(e.Result);
var parentExecutionId = GetParentExecutionId(e.Result);

if (parentExecutionId != Guid.Empty && leafExecutionIdAndTestOutcomePairDictionary.ContainsKey(parentExecutionId))
{
leafExecutionIdAndTestOutcomePairDictionary.TryRemove(parentExecutionId, out _);
}

leafExecutionIdAndTestOutcomePairDictionary.TryAdd(executionId, e.Result.Outcome);

switch (e.Result.Outcome)
{
case TestOutcome.Skipped:
{
this.testsSkipped++;
if (this.verbosityLevel == Verbosity.Quiet)
{
break;
Expand All @@ -512,7 +559,6 @@ private void TestResultHandler(object sender, TestResultEventArgs e)

case TestOutcome.Failed:
{
this.testsFailed++;
if (this.verbosityLevel == Verbosity.Quiet)
{
break;
Expand All @@ -533,7 +579,6 @@ private void TestResultHandler(object sender, TestResultEventArgs e)

case TestOutcome.Passed:
{
this.testsPassed++;
if (this.verbosityLevel == Verbosity.Normal || this.verbosityLevel == Verbosity.Detailed)
{
// Pause the progress indicator before displaying test result information
Expand Down Expand Up @@ -617,6 +662,10 @@ private string GetFormattedDurationString(TimeSpan duration)
/// </summary>
private void TestRunCompleteHandler(object sender, TestRunCompleteEventArgs e)
{
var testsTotal = 0;
var testsPassed = 0;
var testsFailed = 0;
var testsSkipped = 0;
// Stop the progress indicator as we are about to print the summary
this.progressIndicator?.Stop();
Output.WriteLine(string.Empty, OutputLevel.Information);
Expand All @@ -636,6 +685,25 @@ private void TestRunCompleteHandler(object sender, TestRunCompleteEventArgs e)
}
}

foreach (KeyValuePair<Guid, TestOutcome> entry in leafExecutionIdAndTestOutcomePairDictionary)
{
testsTotal++;
switch (entry.Value)
{
case TestOutcome.Failed:
testsFailed++;
break;
case TestOutcome.Passed:
testsPassed++;
break;
case TestOutcome.Skipped:
testsSkipped++;
break;
default:
break;
}
}

if (e.IsCanceled)
{
Output.Error(false, CommandLineResources.TestRunCanceled);
Expand All @@ -644,7 +712,7 @@ private void TestRunCompleteHandler(object sender, TestRunCompleteEventArgs e)
{
Output.Error(false, CommandLineResources.TestRunAborted);
}
else if (this.testsFailed > 0 || this.testRunHasErrorMessages)
else if (testsFailed > 0 || this.testRunHasErrorMessages)
{
Output.Error(false, CommandLineResources.TestRunFailed);
}
Expand Down
2 changes: 1 addition & 1 deletion test/Microsoft.TestPlatform.AcceptanceTests/OrderedTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ public void OlderOrderedTestsShouldWorkFine(RunnerInfo runnerInfo)
this.ValidateFailedTests("FailingTest1");
this.ValidateSkippedTests("FailingTest2");
// Parent test result should fail as inner results contain failing test.
this.ValidateSummaryStatus(2, 2, 1);
this.ValidateSummaryStatus(2, 1, 1);
}
}
}
45 changes: 45 additions & 0 deletions test/vstest.console.UnitTests/Internal/ConsoleLoggerTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1046,6 +1046,51 @@ public void AttachmentInformationShouldBeWrittenToConsoleIfAttachmentsArePresent
this.mockOutput.Verify(o => o.WriteLine(string.Format(CultureInfo.CurrentCulture, CommandLineResources.AttachmentOutputFormat, uriDataAttachment1.Uri.LocalPath), OutputLevel.Information), Times.Once());
}

[TestMethod]
public void ResultsInHeirarchichalOrderShouldReportCorrectCount()
{
var loggerEvents = new InternalTestLoggerEvents(TestSessionMessageLogger.Instance);
loggerEvents.EnableEvents();
var parameters = new Dictionary<string, string>();
parameters.Add("verbosity", "normal");
this.consoleLogger.Initialize(loggerEvents, parameters);

TestCase testCase1 = CreateTestCase("TestCase1");
TestCase testCase2 = CreateTestCase("TestCase2");
TestCase testCase3 = CreateTestCase("TestCase3");

Guid parentExecutionId = Guid.NewGuid();
TestProperty ParentExecIdProperty = TestProperty.Register("ParentExecId", "ParentExecId", typeof(Guid), TestPropertyAttributes.Hidden, typeof(ObjectModel.TestResult));
TestProperty ExecutionIdProperty = TestProperty.Register("ExecutionId", "ExecutionId", typeof(Guid), TestPropertyAttributes.Hidden, typeof(ObjectModel.TestResult));
TestProperty TestTypeProperty = TestProperty.Register("TestType", "TestType" , typeof(Guid), TestPropertyAttributes.Hidden, typeof(ObjectModel.TestResult));

var result1 = new ObjectModel.TestResult(testCase1) { Outcome = TestOutcome.Failed };
result1.SetPropertyValue(ExecutionIdProperty, parentExecutionId);

var result2 = new ObjectModel.TestResult(testCase2) { Outcome = TestOutcome.Passed};
result2.SetPropertyValue(ExecutionIdProperty, Guid.NewGuid());
result2.SetPropertyValue(ParentExecIdProperty, parentExecutionId);

var result3 = new ObjectModel.TestResult(testCase3) { Outcome = TestOutcome.Failed };
result3.SetPropertyValue(ExecutionIdProperty, Guid.NewGuid());
result3.SetPropertyValue(ParentExecIdProperty, parentExecutionId);

loggerEvents.RaiseTestResult(new TestResultEventArgs(result1));
loggerEvents.RaiseTestResult(new TestResultEventArgs(result2));
loggerEvents.RaiseTestResult(new TestResultEventArgs(result3));

loggerEvents.CompleteTestRun(null, false, false, null, null, new TimeSpan(1, 0, 0, 0));

this.mockOutput.Verify(o => o.WriteLine(string.Format(CultureInfo.CurrentCulture, CommandLineResources.TestRunSummaryFailedTests, 1), OutputLevel.Information), Times.Once());
this.mockOutput.Verify(o => o.WriteLine(string.Format(CultureInfo.CurrentCulture, CommandLineResources.TestRunSummaryPassedTests, 1), OutputLevel.Information), Times.Once());
this.mockOutput.Verify(o => o.WriteLine(string.Format(CultureInfo.CurrentCulture, CommandLineResources.TestRunSummaryTotalTests, 2), OutputLevel.Information), Times.Once());
}

private TestCase CreateTestCase(string testCaseName)
{
return new TestCase(testCaseName, new Uri("some://uri"), "DummySourceFileName");
}

private void Setup()
{
this.mockRequestData = new Mock<IRequestData>();
Expand Down

0 comments on commit 5ef1f3b

Please sign in to comment.