Add missing discovery cancel implementation (#2227)

* 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

Author: singhsarab

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));
             }
         }
 

src/Microsoft.TestPlatform.CrossPlatEngine/Client/ProxyDiscoveryManager.cs

@@ -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/>

src/Microsoft.TestPlatform.CrossPlatEngine/Discovery/DiscovererEnumerator.cs

@@ -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)
@@ -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
@@ -219,27 +217,21 @@ private DiscoveryResult DiscoverTestsFromSingleDiscoverer(
                 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);
@@ -252,18 +244,22 @@ private DiscoveryResult DiscoverTestsFromSingleDiscoverer(
                 }
 
                 // 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)
@@ -499,11 +495,5 @@ private static IEnumerable<LazyExtension<ITestDiscoverer, ITestDiscovererCapabil
             }
         }
 
-        private class DiscoveryResult
-        {
-            public double TotalTimeSpentInAdapaters { get; set; }
-            public int TotalAdaptersUsed { get; set; }
-        }
-
     }
 }

test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Client/ProxyDiscoveryManagerTests.cs

@@ -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;

test/Microsoft.TestPlatform.CrossPlatEngine.UnitTests/Discovery/DiscovererEnumeratorTests.cs

@@ -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;
             }
         }
 
@@ -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; }
 
@@ -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;
@@ -753,7 +743,6 @@ public void DiscoverTests(IEnumerable<string> sources, IDiscoveryContext discove
             public static void Reset()
             {
                 IsDiscoverTestCalled = false;
-                Sources = new List<string>();
             }
         }
 

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()
         {