From 829659f9ca95ccfb54480948d8e472651dd0ff5d Mon Sep 17 00:00:00 2001 From: Sarabjot Singh Date: Thu, 24 Oct 2019 14:58:25 +0530 Subject: [PATCH] Add missing discovery cancel implementation (#2227) * Revert "Implemented cancellation of individual source files discovery (#2134)" This reverts commit 8e6f70f444299a122e71f7924f3db3f6feb4adeb. * Calling close instead of endsession to make sure we kill the test host in case it does not exit --- .../TestRequestSender.cs | 2 +- .../Client/ProxyDiscoveryManager.cs | 4 +- .../Discovery/DiscovererEnumerator.cs | 52 ++++++++----------- .../Client/ProxyDiscoveryManagerTests.cs | 15 ++++++ .../Discovery/DiscovererEnumeratorTests.cs | 21 ++------ .../EventHandlers/TestRequestHandlerTests.cs | 4 -- 6 files changed, 45 insertions(+), 53 deletions(-) diff --git a/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs b/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs index 9cebf09b3d..755f39cd13 100644 --- a/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs +++ b/src/Microsoft.TestPlatform.CommunicationUtilities/TestRequestSender.cs @@ -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)); } } diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs index 8ba381f1ac..b20a8fcf4f 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs @@ -140,7 +140,9 @@ public void DiscoverTests(DiscoveryCriteria discoveryCriteria, ITestDiscoveryEve /// 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(); } /// diff --git a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs index 5e1948a3ae..48666d0c3d 100644 --- a/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs +++ b/src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs @@ -161,9 +161,7 @@ private void LoadTestsFromAnExtension(string extensionAssembly, IEnumerable discoverer, - IEnumerable sources, + Dictionary, IEnumerable> 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 @@ -219,19 +217,12 @@ 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; @@ -239,7 +230,8 @@ private void CollectTelemetryAtEnd(double totalTimeTakenByAdapters, double total 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); @@ -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 discoverer, IMessageLogger logger, out Type discovererType) @@ -499,11 +495,5 @@ private static bool IsAssembly(string filePath) } } - private class DiscoveryResult - { - public double TotalTimeSpentInAdapaters { get; set; } - public int TotalAdaptersUsed { get; set; } - } - } } diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs index 36659baa4a..e10c7c7c60 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs @@ -493,6 +493,21 @@ public void DiscoveryManagerShouldPassOnHandleLogMessage() mockTestDiscoveryEventsHandler.Verify(mtdeh => mtdeh.HandleLogMessage(It.IsAny(), It.IsAny()), Times.Once); } + [TestMethod] + public void AbortShouldSendTestDiscoveryCancelIfCommunicationSuccessful() + { + var mockTestDiscoveryEventsHandler = new Mock(); + this.mockRequestSender.Setup(s => s.WaitForRequestHandlerConnection(It.IsAny(), It.IsAny())).Returns(true); + + Mock mockTestRunEventsHandler = new Mock(); + + 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; diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscovererEnumeratorTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscovererEnumeratorTests.cs index d5dbab08f5..cb56326c87 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscovererEnumeratorTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscovererEnumeratorTests.cs @@ -637,18 +637,13 @@ public static void Reset() [Category("managed")] private class ManagedDllTestDiscoverer : DllTestDiscoverer { - static ManagedDllTestDiscoverer() - { - Sources = new List(); - } - public static bool IsManagedDiscoverTestCalled { get; private set; } - public static List Sources { get; private set; } + public static IEnumerable Sources { get; set; } public override void DiscoverTests(IEnumerable sources, IDiscoveryContext discoveryContext, IMessageLogger logger, ITestCaseDiscoverySink discoverySink) { - Sources.AddRange(sources); + Sources = sources; IsManagedDiscoverTestCalled = true; base.DiscoverTests(sources, discoveryContext, logger, discoverySink); } @@ -656,7 +651,7 @@ public override void DiscoverTests(IEnumerable sources, IDiscoveryContex public static void Reset() { IsManagedDiscoverTestCalled = false; - Sources = new List(); + Sources = null; } } @@ -726,14 +721,9 @@ private static bool ShouldTestDiscovered(IEnumerable sources) [DefaultExecutorUri("discoverer://jsondiscoverer")] private class JsonTestDiscoverer : ITestDiscoverer { - static JsonTestDiscoverer() - { - Sources = new List(); - } - public static bool IsDiscoverTestCalled { get; private set; } - public static List Sources { get; private set; } + public static IEnumerable Sources { get; private set; } public static IDiscoveryContext DiscoveryContext { get; private set; } @@ -744,7 +734,7 @@ static JsonTestDiscoverer() public void DiscoverTests(IEnumerable sources, IDiscoveryContext discoveryContext, IMessageLogger logger, ITestCaseDiscoverySink discoverySink) { IsDiscoverTestCalled = true; - Sources.AddRange(sources); + Sources = sources; DiscoveryContext = discoveryContext; MessageLogger = logger; DiscoverySink = discoverySink; @@ -753,7 +743,6 @@ public void DiscoverTests(IEnumerable sources, IDiscoveryContext discove public static void Reset() { IsDiscoverTestCalled = false; - Sources = new List(); } } diff --git a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs index 4b5d5a2d24..d90d5e4b2b 100644 --- a/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs +++ b/test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/EventHandlers/TestRequestHandlerTests.cs @@ -286,8 +286,6 @@ public void ProcessRequestsExecutionCancelShouldCancelTestRun() this.SendSessionEnd(); } - // ProcessRequestsExecutionCancelShouldStopRequestProcessing - [TestMethod] public void ProcessRequestsExecutionLaunchAdapterProcessWithDebuggerShouldSendAckMessage() { @@ -312,8 +310,6 @@ public void ProcessRequestsExecutionAbortShouldStopTestRun() this.SendSessionEnd(); } - // ProcessRequestsExecutionAbortShouldStopRequestProcessing - [TestMethod] public void SendExecutionCompleteShouldSendTestRunCompletePayloadOnChannel() {