Make Cancel Discovery faster and more reliable (#3527)

microsoft · Mar 29, 2022 · 7258347 · 7258347
1 parent 12ae76d
commit 7258347
Show file tree

Hide file tree

Showing 3 changed files with 159 additions and 7 deletions.
diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/Microsoft.TestPlatform.AcceptanceTests.csproj b/test/Microsoft.TestPlatform.AcceptanceTests/Microsoft.TestPlatform.AcceptanceTests.csproj
@@ -28,6 +28,7 @@
   </ItemGroup>
   <ItemGroup>
     <PackageReference Include="Chutzpah" Version="$(ChutzpahAdapterVersion)" />
+    <PackageReference Include="FluentAssertions" Version="5.10.3" />
     <PackageReference Include="Microsoft.CodeAnalysis.BannedApiAnalyzers" Version="3.3.3">
       <PrivateAssets>all</PrivateAssets>
       <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets>

diff --git a/test/Microsoft.TestPlatform.AcceptanceTests/TranslationLayerTests/DiscoverTests.cs b/test/Microsoft.TestPlatform.AcceptanceTests/TranslationLayerTests/DiscoverTests.cs
@@ -3,9 +3,13 @@
 
 using System;
 using System.Collections.Generic;
+using System.Diagnostics;
 using System.Linq;
 using System.Threading.Tasks;
 
+using FluentAssertions;
+using FluentAssertions.Extensions;
+
 using Microsoft.TestPlatform.TestUtilities;
 using Microsoft.TestPlatform.VsTestConsole.TranslationLayer.Interfaces;
 using Microsoft.VisualStudio.TestPlatform.Common.Telemetry;
@@ -200,6 +204,7 @@ public void DiscoverTestsUsingSourceNavigation(RunnerInfo runnerInfo)
     [NetCoreTargetFrameworkDataSource]
     public async Task CancelTestDiscovery(RunnerInfo runnerInfo)
     {
+        var sw = Stopwatch.StartNew();
         // Setup
         var testAssemblies = new List<string>
         {
@@ -212,11 +217,20 @@ public async Task CancelTestDiscovery(RunnerInfo runnerInfo)
 
         var discoveredTests = new List<TestCase>();
         var discoveryEvents = new Mock<ITestDiscoveryEventsHandler>();
+        var alreadyCancelled = false;
+        TimeSpan cancellationCalled = TimeSpan.Zero;
         discoveryEvents.Setup(events => events.HandleDiscoveredTests(It.IsAny<IEnumerable<TestCase>>()))
             .Callback((IEnumerable<TestCase> testcases) =>
             {
+                // As soon as we get first test call cancel. That way we know there is discovery in progress.
                 discoveredTests.AddRange(testcases);
-                _vstestConsoleWrapper.CancelDiscovery();
+                if (!alreadyCancelled)
+                {
+                    cancellationCalled = sw.Elapsed;
+                    // Calling cancel many times crashes. https://github.com/microsoft/vstest/issues/3526
+                    alreadyCancelled = true;
+                    _vstestConsoleWrapper.CancelDiscovery();
+                }
             });
         var isTestCancelled = false;
         discoveryEvents.Setup(events => events.HandleDiscoveryComplete(It.IsAny<long>(), It.IsAny<IEnumerable<TestCase>>(), It.IsAny<bool>()))
@@ -229,11 +243,23 @@ public async Task CancelTestDiscovery(RunnerInfo runnerInfo)
                 }
             });
 
+        string runSettingsXml =
+             $@"<?xml version=""1.0"" encoding=""utf-8""?>
+            <RunSettings>
+                <RunConfiguration>
+                    <TargetFrameworkVersion>{FrameworkArgValue}</TargetFrameworkVersion>
+                    <BatchSize>1</BatchSize>
+                </RunConfiguration>
+            </RunSettings>";
+
         // Act
-        await Task.Run(() => _vstestConsoleWrapper.DiscoverTests(testAssemblies, GetDefaultRunSettings(), discoveryEvents.Object));
+        await Task.Run(() => _vstestConsoleWrapper.DiscoverTests(testAssemblies, runSettingsXml, discoveryEvents.Object));
 
         // Assert.
         Assert.IsTrue(isTestCancelled);
+        var done = sw.Elapsed;
+        var timeTillCancelled = done - cancellationCalled;
+        timeTillCancelled.Should().BeLessThan(2.Seconds());
         int discoveredSourcesCount = discoveredTests.Select(testcase => testcase.Source).Distinct().Count();
         Assert.AreNotEqual(testAssemblies.Count, discoveredSourcesCount, "All test assemblies discovered");
     }

diff --git a/test/TestAssets/DiscoveryTestProject/LongDiscoveryTestClass.cs b/test/TestAssets/DiscoveryTestProject/LongDiscoveryTestClass.cs
@@ -10,18 +10,143 @@ namespace DiscoveryTestProject3
     [TestClass]
     public class LongDiscoveryTestClass
     {
-        [MyTestMethod]
-        public void CustomTestMethod()
+        // This is for discovery cancellation test.
+        // 20 tests below to be discovered until we reach the X_ Y_ Z_LongDiscoveryTestMethod which haver attribute that
+        // takes a very long time to create, which prolongs the discovery time and keeps us discovering while we
+        // are cancelling the discovery from the test.
+        #region 20 empty tests
+
+        [TestMethod]
+        public void TestMethod1()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod2()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod3()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod4()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod5()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod6()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod7()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod8()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod9()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod10()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod11()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod12()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod13()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod14()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod15()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod16()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod17()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod18()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod19()
+        {
+        }
+
+        [TestMethod]
+        public void TestMethod20()
+        {
+        }
+
+        #endregion
+
+        // X_ to make it discover last.
+        [TestMethodWithDelay]
+        public void X_LongDiscoveryTestMethod()
+        {
+
+        }
+
+        // Y_ to make it discover last.
+        [TestMethodWithDelay]
+        public void Y_LongDiscoveryTestMethod()
+        {
+
+        }
+
+        // Z_ to make it discover last.
+        [TestMethodWithDelay]
+        public void Z_LongDiscoveryTestMethod()
         {
 
         }
     }
 
-    internal class MyTestMethodAttribute : TestMethodAttribute
+    internal class TestMethodWithDelayAttribute : TestMethodAttribute
     {
-        public MyTestMethodAttribute()
+        public TestMethodWithDelayAttribute()
         {
-            Thread.Sleep(10000);
+            // This will be multiplied by 3 because the framework will internally create this
+            // attribute 3 times. And by another 3 because we have 3 slow tests.
+            Thread.Sleep(100);
         }
     }
 }