Skip to content

Commit

Permalink
Add missing discovery cancel implementation (#2227)
Browse files Browse the repository at this point in the history 
* Revert "Implemented cancellation of individual source files discovery (#2134)"
This reverts commit 8e6f70f.

* Calling close instead of endsession to make sure we kill the test host in case it does not exit
  • Loading branch information
@singhsarab
singhsarab committed Oct 24, 2019
1 parent 7ba379f commit 829659f
Show file tree
Hide file tree
Showing 6 changed files with 45 additions and 53 deletions.
2 changes: 1 addition & 1 deletion src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs
View file
Expand Up @@ -363,7 +363,7 @@ public void EndSession()
EqtTrace.Verbose("TestRequestSender.EndSession: Sending end session.");
}

this.channel.Send(this.dataSerializer.SerializeMessage(MessageType.SessionEnd));
this.channel?.Send(this.dataSerializer.SerializeMessage(MessageType.SessionEnd));
}
}

Expand Down
4 changes: 3 additions & 1 deletion src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs
View file
Expand Up @@ -140,7 +140,9 @@ public void DiscoverTests(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEve
/// <inheritdoc/>
public void Abort()
{
// This is no-op for the moment. There is no discovery abort message?
// Cancel fast, try to stop testhost deployment/launch
this.CancellationTokenSource.Cancel();
this.Close();
}

/// <inheritdoc/>
Expand Down
52 changes: 21 additions & 31 deletions src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs
View file
Expand Up @@ -161,9 +161,7 @@ private void LoadTestsFromAnExtension(string extensionAssembly, IEnumerable<stri
return;
}

var result = this.DiscoverTestsFromSingleDiscoverer(discoverer, discovererToSourcesMap[discoverer], context, discoverySink, logger, cancellationToken);
totalAdaptersUsed += result.TotalAdaptersUsed;
totalTimeTakenByAdapters += result.TotalTimeSpentInAdapaters;
this.DiscoverTestsFromSingleDiscoverer(discoverer, discovererToSourcesMap, context, discoverySink, logger, ref totalAdaptersUsed, ref totalTimeTakenByAdapters);
}

if (this.discoveryResultCache.TotalDiscoveredTests == 0)
Expand Down Expand Up @@ -193,19 +191,19 @@ private void CollectTelemetryAtEnd(double totalTimeTakenByAdapters, double total
totalAdaptersUsed);
}

private DiscoveryResult DiscoverTestsFromSingleDiscoverer(
private void DiscoverTestsFromSingleDiscoverer(
LazyExtension<ITestDiscoverer, ITestDiscovererCapabilities> discoverer,
IEnumerable<string> sources,
Dictionary<LazyExtension<ITestDiscoverer, ITestDiscovererCapabilities>, IEnumerable<string>> discovererToSourcesMap,
DiscoveryContext context,
TestCaseDiscoverySink discoverySink,
IMessageLogger logger,
CancellationToken cancellationToken)
ref double totalAdaptersUsed,
ref double totalTimeTakenByAdapters)
{
var result = new DiscoveryResult();
if (DiscovererEnumerator.TryToLoadDiscoverer(discoverer, logger, out var discovererType) == false)
{
// Fail to instantiate the discoverer type.
return result;
return;
}

// on instantiated successfully, get tests
Expand All @@ -219,27 +217,21 @@ private void CollectTelemetryAtEnd(double totalTimeTakenByAdapters, double total
var newTimeStart = DateTime.UtcNow;

this.testPlatformEventSource.AdapterDiscoveryStart(discoverer.Metadata.DefaultExecutorUri.AbsoluteUri);
foreach (var testSource in sources)
{
if (cancellationToken.IsCancellationRequested)
{
EqtTrace.Info("DiscovererEnumerator.DiscoverTestsFromSingleDiscoverer: Cancellation Requested. Aborting the discovery");
break;
}

discoverer.Value.DiscoverTests(new[] { testSource }, context, logger, discoverySink);
}
discoverer.Value.DiscoverTests(discovererToSourcesMap[discoverer], context, logger, discoverySink);

var totalAdapterRunTime = DateTime.UtcNow - newTimeStart;
this.testPlatformEventSource.AdapterDiscoveryStop(this.discoveryResultCache.TotalDiscoveredTests - currentTotalTests);

this.testPlatformEventSource.AdapterDiscoveryStop(this.discoveryResultCache.TotalDiscoveredTests -
currentTotalTests);

// Record Total Tests Discovered By each Discoverer.
var totalTestsDiscoveredByCurrentDiscoverer = this.discoveryResultCache.TotalDiscoveredTests - currentTotalTests;
this.requestData.MetricsCollection.Add(
string.Format("{0}.{1}", TelemetryDataConstants.TotalTestsByAdapter,
discoverer.Metadata.DefaultExecutorUri), totalTestsDiscoveredByCurrentDiscoverer);

result.TotalAdaptersUsed++;
totalAdaptersUsed++;


EqtTrace.Verbose("DiscovererEnumerator.DiscoverTestsFromSingleDiscoverer: Done loading tests for {0}",
discoverer.Value.GetType().FullName);
Expand All @@ -252,18 +244,22 @@ private void CollectTelemetryAtEnd(double totalTimeTakenByAdapters, double total
}

// Collecting Data Point for Time Taken to Discover Tests by each Adapter
this.requestData.MetricsCollection.Add($"{TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter}.{discoverer.Metadata.DefaultExecutorUri}", totalAdapterRunTime.TotalSeconds);
result.TotalTimeSpentInAdapaters += totalAdapterRunTime.TotalSeconds;
this.requestData.MetricsCollection.Add(
string.Format("{0}.{1}", TelemetryDataConstants.TimeTakenToDiscoverTestsByAnAdapter,
discoverer.Metadata.DefaultExecutorUri), totalAdapterRunTime.TotalSeconds);
totalTimeTakenByAdapters += totalAdapterRunTime.TotalSeconds;
}
catch (Exception e)
{
var message = string.Format(CultureInfo.CurrentUICulture, CrossPlatEngineResources.ExceptionFromLoadTests, discovererType.Name, e.Message);
var message = string.Format(
CultureInfo.CurrentUICulture,
CrossPlatEngineResources.ExceptionFromLoadTests,
discovererType.Name,
e.Message);

logger.SendMessage(TestMessageLevel.Error, message);
EqtTrace.Error("DiscovererEnumerator.DiscoverTestsFromSingleDiscoverer: {0} ", e);
}

return result;
}

private static bool TryToLoadDiscoverer(LazyExtension<ITestDiscoverer, ITestDiscovererCapabilities> discoverer, IMessageLogger logger, out Type discovererType)
Expand Down Expand Up @@ -499,11 +495,5 @@ private static bool IsAssembly(string filePath)
}
}

private class DiscoveryResult
{
public double TotalTimeSpentInAdapaters { get; set; }
public int TotalAdaptersUsed { get; set; }
}

}
}
15 changes: 15 additions & 0 deletions test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs
View file
Expand Up @@ -493,6 +493,21 @@ public void DiscoveryManagerShouldPassOnHandleLogMessage()
mockTestDiscoveryEventsHandler.Verify(mtdeh => mtdeh.HandleLogMessage(It.IsAny<TestMessageLevel>(), It.IsAny<string>()), Times.Once);
}

[TestMethod]
public void AbortShouldSendTestDiscoveryCancelIfCommunicationSuccessful()
{
var mockTestDiscoveryEventsHandler = new Mock<ITestDiscoveryEventsHandler2>();
this.mockRequestSender.Setup(s => s.WaitForRequestHandlerConnection(It.IsAny<int>(), It.IsAny<CancellationToken>())).Returns(true);

Mock<ITestRunEventsHandler> mockTestRunEventsHandler = new Mock<ITestRunEventsHandler>();

this.testDiscoveryManager.DiscoverTests(this.discoveryCriteria, mockTestDiscoveryEventsHandler.Object);

this.testDiscoveryManager.Abort();

this.mockRequestSender.Verify(s => s.EndSession(), Times.Once);
}

private void InvokeAndVerifyDiscoverTests(bool skipDefaultAdapters)
{
TestPluginCache.Instance = null;
Expand Down
21 changes: 5 additions & 16 deletions test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscovererEnumeratorTests.cs
View file
Expand Up @@ -637,26 +637,21 @@ public static void Reset()
[Category("managed")]
private class ManagedDllTestDiscoverer : DllTestDiscoverer
{
static ManagedDllTestDiscoverer()
{
Sources = new List<string>();
}

public static bool IsManagedDiscoverTestCalled { get; private set; }

public static List<string> Sources { get; private set; }
public static IEnumerable<string> Sources { get; set; }

public override void DiscoverTests(IEnumerable<string> sources, IDiscoveryContext discoveryContext, IMessageLogger logger, ITestCaseDiscoverySink discoverySink)
{
Sources.AddRange(sources);
Sources = sources;
IsManagedDiscoverTestCalled = true;
base.DiscoverTests(sources, discoveryContext, logger, discoverySink);
}

public static void Reset()
{
IsManagedDiscoverTestCalled = false;
Sources = new List<string>();
Sources = null;
}
}

Expand Down Expand Up @@ -726,14 +721,9 @@ private static bool ShouldTestDiscovered(IEnumerable<string> sources)
[DefaultExecutorUri("discoverer://jsondiscoverer")]
private class JsonTestDiscoverer : ITestDiscoverer
{
static JsonTestDiscoverer()
{
Sources = new List<string>();
}

public static bool IsDiscoverTestCalled { get; private set; }

public static List<string> Sources { get; private set; }
public static IEnumerable<string> Sources { get; private set; }

public static IDiscoveryContext DiscoveryContext { get; private set; }

Expand All @@ -744,7 +734,7 @@ static JsonTestDiscoverer()
public void DiscoverTests(IEnumerable<string> sources, IDiscoveryContext discoveryContext, IMessageLogger logger, ITestCaseDiscoverySink discoverySink)
{
IsDiscoverTestCalled = true;
Sources.AddRange(sources);
Sources = sources;
DiscoveryContext = discoveryContext;
MessageLogger = logger;
DiscoverySink = discoverySink;
Expand All @@ -753,7 +743,6 @@ public void DiscoverTests(IEnumerable<string> sources, IDiscoveryContext discove
public static void Reset()
{
IsDiscoverTestCalled = false;
Sources = new List<string>();
}
}

Expand Down
4 changes: 0 additions & 4 deletions ...Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs
View file
Expand Up @@ -286,8 +286,6 @@ public void ProcessRequestsExecutionCancelShouldCancelTestRun()
this.SendSessionEnd();
}

// ProcessRequestsExecutionCancelShouldStopRequestProcessing

[TestMethod]
public void ProcessRequestsExecutionLaunchAdapterProcessWithDebuggerShouldSendAckMessage()
{
Expand All @@ -312,8 +310,6 @@ public void ProcessRequestsExecutionAbortShouldStopTestRun()
this.SendSessionEnd();
}

// ProcessRequestsExecutionAbortShouldStopRequestProcessing

[TestMethod]
public void SendExecutionCompleteShouldSendTestRunCompletePayloadOnChannel()
{
Expand Down

0 comments on commit 829659f

Please sign in to comment.