Skip to content
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

Fix Few UWP VC++ unit tests are not executing #1649

Merged
merged 6 commits into from
Jun 18, 2018
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -46,5 +46,13 @@ public interface IDataSerializer
/// <param name="version">version to be sent</param>
/// <returns>Raw Serialized message</returns>
string SerializePayload(string messageType, object payload, int version);

/// <summary>
/// Creates cloned object for given object.
/// </summary>
/// <typeparam name="T"> The type of object to be cloned. </typeparam>
/// <param name="obj">Object to be cloned.</param>
/// <returns>Newly cloned object.</returns>
T Clone<T>(T obj);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -152,6 +152,18 @@ public string SerializePayload(string messageType, object payload, int version)
return JsonConvert.SerializeObject(message);
}

/// <inheritdoc/>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please write a comment on why we chose this approach, just for reference.
Also consider making it an extension method.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are lot of discussion over stackoverflow, how to do deep copy.

  1. BinaryFormatter
    This very generic and easy to implement, But it is not available in NS14.
  2. ICloneable
    This required changes in all the class which are involved and difficult to maintain(complicated code).

As we are already using JsonDataSeializer to transfer the object over network. We can use the same functionality to do clone Which is well tested.

public T Clone<T>(T obj)
{
if (obj == null)
{
return default(T);
}

var stringObj = this.Serialize<T>(obj, 2);
return this.Deserialize<T>(stringObj, 2);
}

/// <summary>
/// Serialize an object to JSON using default serialization settings.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,7 @@ internal Collection<AttachmentSet> Attachments
/// <param name="testCase">test case which will be started.</param>
public void RecordStart(TestCase testCase)
{
EqtTrace.Verbose("TestExecutionRecorder.RecordStart: Starting test: {0}.", testCase?.FullyQualifiedName);
this.testRunCache.OnTestStarted(testCase);

if (this.testCaseEventsHandler != null)
Expand All @@ -85,6 +86,7 @@ public void RecordStart(TestCase testCase)
/// test result to the framework when the test(s) is canceled. </exception>
public void RecordResult(TestResult testResult)
{
EqtTrace.Verbose("TestExecutionRecorder.RecordResult: Received result for test: {0}.", testResult?.TestCase?.FullyQualifiedName);
if (this.testCaseEventsHandler != null)
{
// Send TestCaseEnd in case RecordEnd was not called.
Expand All @@ -104,6 +106,7 @@ public void RecordResult(TestResult testResult)
/// <param name="outcome">outcome of the test case.</param>
public void RecordEnd(TestCase testCase, TestOutcome outcome)
{
EqtTrace.Verbose("TestExecutionRecorder.RecordEnd: test: {0} execution completed.", testCase?.FullyQualifiedName);
this.testRunCache.OnTestCompletion(testCase);
this.SendTestCaseEnd(testCase, outcome);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution
{
using System;
using System.Collections;
using System.Collections.Generic;
using System.Collections.ObjectModel;
using System.Diagnostics;
Expand All @@ -13,7 +14,7 @@ namespace Microsoft.VisualStudio.TestPlatform.CrossPlatEngine.Execution
using System.Linq;
using System.Reflection;
using System.Threading.Tasks;

using CommunicationUtilities.Interfaces;
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework;
using Microsoft.VisualStudio.TestPlatform.Common.ExtensionFramework.Utilities;
using Microsoft.VisualStudio.TestPlatform.Common.Interfaces;
Expand Down Expand Up @@ -78,6 +79,11 @@ internal abstract class BaseRunTests
/// </summary>
private RunConfiguration runConfiguration;

/// <summary>
/// The Serializer to clone testcase object incase of user input test source is package. E.g UWP scenario(appx/build.appxrecipe).
/// </summary>
private IDataSerializer dataSerializer;

#endregion

#region Constructor
Expand Down Expand Up @@ -107,7 +113,8 @@ protected BaseRunTests(IRequestData requestData,
testRunEventsHandler,
testPlatformEventSource,
testCaseEventsHandler as ITestEventsPublisher,
new PlatformThread())
new PlatformThread(),
JsonDataSerializer.Instance)
{
}

Expand All @@ -131,7 +138,8 @@ protected BaseRunTests(IRequestData requestData,
ITestRunEventsHandler testRunEventsHandler,
ITestPlatformEventSource testPlatformEventSource,
ITestEventsPublisher testEventsPublisher,
IThread platformThread)
IThread platformThread,
IDataSerializer dataSerializer)
{
this.package = package;
this.runSettings = runSettings;
Expand All @@ -144,6 +152,7 @@ protected BaseRunTests(IRequestData requestData,
this.testPlatformEventSource = testPlatformEventSource;
this.testEventsPublisher = testEventsPublisher;
this.platformThread = platformThread;
this.dataSerializer = dataSerializer;
this.SetContext();
}

Expand Down Expand Up @@ -557,6 +566,12 @@ private void RaiseTestRunComplete(
// Collecting Number of Adapters Used to run tests.
this.requestData.MetricsCollection.Add(TelemetryDataConstants.NumberOfAdapterUsedToRunTests, this.ExecutorUrisThatRanTests.Count());

if (lastChunk.Any() && string.IsNullOrEmpty(package) == false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: inaddition to package being null should we also compare with source.. also depending on package being null for a business logic seems a little odd.. ideally there shold be a flag setup upstream (to indicate this behavior)

{
Tuple<ICollection<TestResult>, ICollection<TestCase>> updatedTestResultsAndInProgressTestCases = this.UpdateTestCaseSourceToPackage(lastChunk, null);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: can use var ? (also usage of Tuple indicates a missing struct)

lastChunk = updatedTestResultsAndInProgressTestCases.Item1;
}

var testRunChangedEventArgs = new TestRunChangedEventArgs(runStats, lastChunk, Enumerable.Empty<TestCase>());

// Adding Metrics along with Test Run Complete Event Args
Expand All @@ -569,10 +584,6 @@ private void RaiseTestRunComplete(
attachments,
elapsedTime);
testRunCompleteEventArgs.Metrics = this.requestData.MetricsCollection.Metrics;
if (lastChunk.Any())
{
UpdateTestResults(lastChunk, null, this.package);
}

this.testRunEventsHandler.HandleTestRunComplete(
testRunCompleteEventArgs,
Expand All @@ -586,13 +597,19 @@ private void RaiseTestRunComplete(
}
}

private void OnCacheHit(TestRunStatistics testRunStats, ICollection<TestResult> results, ICollection<TestCase> inProgressTests)
private void OnCacheHit(TestRunStatistics testRunStats, ICollection<TestResult> results, ICollection<TestCase> inProgressTestCases)
{
if (this.testRunEventsHandler != null)
{
UpdateTestResults(results, inProgressTests, this.package);
if (string.IsNullOrEmpty(package) == false)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i see this being repeated.. opportunity to have it in a meaningful method..

{
Tuple<ICollection<TestResult>, ICollection<TestCase>> updatedTestResultsAndInProgressTestCases
= this.UpdateTestCaseSourceToPackage(results, inProgressTestCases);
results = updatedTestResultsAndInProgressTestCases.Item1;
inProgressTestCases = updatedTestResultsAndInProgressTestCases.Item2;
}

var testRunChangedEventArgs = new TestRunChangedEventArgs(testRunStats, results, inProgressTests);
var testRunChangedEventArgs = new TestRunChangedEventArgs(testRunStats, results, inProgressTestCases);
this.testRunEventsHandler.HandleTestRunStatsChange(testRunChangedEventArgs);
}
else
Expand Down Expand Up @@ -624,22 +641,50 @@ private bool TryToRunInSTAThread(Action action, bool waitForCompletion)
}


private static void UpdateTestResults(IEnumerable<TestResult> testResults, IEnumerable<TestCase> testCases, string package)
private Tuple<ICollection<TestResult>, ICollection<TestCase>> UpdateTestCaseSourceToPackage(
ICollection<TestResult> testResults,
ICollection<TestCase> inProgressTestCases)
{
// Before sending the testresults back, update the test case objects with source provided by IDE/User.
if (!string.IsNullOrEmpty(package))

EqtTrace.Verbose("BaseRunTests.UpdateTestCaseSourceToPackage: Update source details for testResults and testCases.");

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

remove unnecessary empty lines

var updatedTestResults = UpdateTestResults(testResults, package);

var updatedInProgressTestCases = UpdateInProgressTests(inProgressTestCases, package);

return new Tuple<ICollection<TestResult>, ICollection<TestCase>>(updatedTestResults, updatedInProgressTestCases);
}

private ICollection<TestResult> UpdateTestResults(ICollection<TestResult> testResults, string package)
{
ICollection<TestResult> updatedTestResults = new List<TestResult>();

foreach (var testResult in testResults)
{
foreach (var tr in testResults)
{
tr.TestCase.Source = package;
}
var updatedTestResult = this.dataSerializer.Clone<TestResult>(testResult);
updatedTestResult.TestCase.Source = package;
updatedTestResults.Add(updatedTestResult);
}

// TestCases can be empty, enumerate on EmptyList then
foreach (var tc in testCases ?? Enumerable.Empty<TestCase>())
{
tc.Source = package;
}
return updatedTestResults;
}

private ICollection<TestCase> UpdateInProgressTests(ICollection<TestCase> inProgressTestCases, string package)
{
if (inProgressTestCases == null)
{
return null;
}

ICollection<TestCase> updatedTestCases = new List<TestCase>();
foreach (var inProgressTestCase in inProgressTestCases)
{
var updatedTestCase = this.dataSerializer.Clone<TestCase>(inProgressTestCase);
updatedTestCase.Source = package;
updatedTestCases.Add(updatedTestCase);
}

return updatedTestCases;
}

#endregion
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,23 +4,33 @@
namespace Microsoft.TestPlatform.CommunicationUtilities.UnitTests
{
using System;
using System.Collections.Generic;
using System.Linq;
using System.Runtime.Serialization;
using Microsoft.VisualStudio.TestPlatform.CommunicationUtilities;
using Microsoft.VisualStudio.TestTools.UnitTesting;
using VisualStudio.TestPlatform.ObjectModel;
using TestResult = VisualStudio.TestPlatform.ObjectModel.TestResult;

[TestClass]
public class JsonDataSerializerTests
{
private JsonDataSerializer jsonDataSerializer;

public JsonDataSerializerTests()
{
this.jsonDataSerializer = JsonDataSerializer.Instance;
}

[TestMethod]
public void SerializePayloadShouldSerializeAnObjectWithSelfReferencingLoop()
{
var classWithSelfReferencingLoop = new ClassWithSelfReferencingLoop(null);
classWithSelfReferencingLoop = new ClassWithSelfReferencingLoop(classWithSelfReferencingLoop);
classWithSelfReferencingLoop.InfiniteRefernce.InfiniteRefernce = classWithSelfReferencingLoop;

var sut = JsonDataSerializer.Instance;

// This line should not throw exception
sut.SerializePayload("dummy", classWithSelfReferencingLoop);
this.jsonDataSerializer.SerializePayload("dummy", classWithSelfReferencingLoop);
}

[TestMethod]
Expand All @@ -30,17 +40,94 @@ public void DeserializeShouldDeserializeAnObjectWhichHadSelfReferencingLoopBefor
classWithSelfReferencingLoop = new ClassWithSelfReferencingLoop(classWithSelfReferencingLoop);
classWithSelfReferencingLoop.InfiniteRefernce.InfiniteRefernce = classWithSelfReferencingLoop;

var sut = JsonDataSerializer.Instance;

var json = sut.SerializePayload("dummy", classWithSelfReferencingLoop);
var json = this.jsonDataSerializer.SerializePayload("dummy", classWithSelfReferencingLoop);

// This line should deserialize properly
var result = sut.Deserialize<ClassWithSelfReferencingLoop>(json, 1);
var result = this.jsonDataSerializer.Deserialize<ClassWithSelfReferencingLoop>(json, 1);

Assert.AreEqual(typeof(ClassWithSelfReferencingLoop), result.GetType());
Assert.IsNull(result.InfiniteRefernce);
}

[TestMethod]
public void CloneShouldReturnNullForNull()
{
var clonedTestCase = this.jsonDataSerializer.Clone<TestCase>(null);

Assert.IsNull(clonedTestCase);
}

[TestMethod]
public void CloneShouldWorkForValueType()
{
var i = 2;
var clonedI = this.jsonDataSerializer.Clone<int>(i);

Assert.AreEqual(clonedI, i);
}

[TestMethod]
public void CloneShouldCloneTestCaseObject()
{
var testCase = JsonDataSerializerTests.GetSampleTestCase(out var expectedTrait);

var clonedTestCase = this.jsonDataSerializer.Clone<TestCase>(testCase);

VerifyTestCaseClone(clonedTestCase, testCase, expectedTrait);
}

[TestMethod]
public void CloneShouldCloneTestResultsObject()
{
var testCase = JsonDataSerializerTests.GetSampleTestCase(out var expectedTrait);

var testResult = new TestResult(testCase);

var startTime = DateTimeOffset.UtcNow;
testResult.StartTime = startTime;

var clonedTestResult = this.jsonDataSerializer.Clone<TestResult>(testResult);

Assert.IsFalse(ReferenceEquals(testResult, clonedTestResult));

Assert.AreEqual(testResult.StartTime, clonedTestResult.StartTime);

VerifyTestCaseClone(testResult.TestCase, clonedTestResult.TestCase, expectedTrait);
}

private static TestCase GetSampleTestCase(out Trait expectedTrait)
{
var testCase = new TestCase("x.y.z", new Uri("uri://dummy"), "x.dll");

expectedTrait = new Trait("TraitName1", "TraitValue1");

testCase.Traits.Add(expectedTrait);
return testCase;
}

private static void VerifyTestCaseClone(TestCase clonedTestCase, TestCase testCase, Trait expectedTrait)
{
Assert.IsFalse(ReferenceEquals(clonedTestCase, testCase));

Assert.AreEqual(testCase.FullyQualifiedName, clonedTestCase.FullyQualifiedName);
Assert.IsFalse(ReferenceEquals(testCase.FullyQualifiedName, clonedTestCase.FullyQualifiedName));

Assert.AreEqual(testCase.ExecutorUri, clonedTestCase.ExecutorUri);
Assert.IsFalse(ReferenceEquals(testCase.ExecutorUri, clonedTestCase.ExecutorUri));

Assert.AreEqual(testCase.Source, clonedTestCase.Source);
Assert.IsFalse(ReferenceEquals(testCase.Source, clonedTestCase.Source));

Assert.AreEqual(1, clonedTestCase.Traits.Count());

foreach (var trait in clonedTestCase.Traits)
{
Assert.IsFalse(ReferenceEquals(expectedTrait, trait));
Assert.AreEqual(expectedTrait.Name, trait.Name);
Assert.AreEqual(expectedTrait.Value, trait.Value);
}
}

public class ClassWithSelfReferencingLoop
{
public ClassWithSelfReferencingLoop(ClassWithSelfReferencingLoop ir)
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT license. See LICENSE file in the project root for full license information.

namespace TestPlatform.CommunicationUtilities.UnitTests.ObjectModel
namespace Microsoft.TestPlatform.CrossPlatEngine.UnitTests.DataCollection
{
using System;
using System.Collections.ObjectModel;
Expand Down
Loading