Ensure to send a session complete event (#4878)

* Ensure to send a session complete event * Add a test for completion event when session start fails * Update a comment
microsoft · Feb 23, 2024 · f232324 · f232324
1 parent 9a7d8cc
commit f232324
Show file tree

Hide file tree

Showing 4 changed files with 57 additions and 5 deletions.
diff --git a/src/Microsoft.TestPlatform.Client/TestPlatform.cs b/src/Microsoft.TestPlatform.Client/TestPlatform.cs
@@ -142,7 +142,6 @@ public bool StartTestSession(
             // sources tells us we should run in-process (i.e. in vstest.console). Because
             // of this no session will be created because there's no testhost to be launched.
             // Expecting a subsequent call to execute tests with the same set of parameters.
-            eventsHandler.HandleStartTestSessionComplete(new());
             return false;
         }
 

diff --git a/src/vstest.console/TestPlatformHelpers/TestRequestManager.cs b/src/vstest.console/TestPlatformHelpers/TestRequestManager.cs
@@ -525,6 +525,13 @@ public void StartTestSession(
                 var testSessionStarted = _testPlatform.StartTestSession(requestData, criteria, eventsHandler, sourceToSourceDetailMap, new NullWarningLogger());
                 if (!testSessionStarted)
                 {
+                    // Note: We need to send back a TestSessionComplete event, so that the caller
+                    // completes a session start request.
+                    // StartTestSession will invoke the HandleStartTestSessionComplete event
+                    // if the test session is started successfully. However, if it is not started,
+                    // HandleStartTestSessionComplete will not send an event. That's why we need
+                    // to do it here.
+                    eventsHandler.HandleStartTestSessionComplete(new());
                     EqtTrace.Warning("TestRequestManager.StartTestSession: Unable to start test session.");
                 }
             }

diff --git a/test/Microsoft.TestPlatform.Client.UnitTests/TestPlatformTests.cs b/test/Microsoft.TestPlatform.Client.UnitTests/TestPlatformTests.cs
@@ -494,10 +494,6 @@ public void StartTestSessionShouldReturnFalseIfTestSessionManagerIsNull()
                 testSessionCriteria,
                 mockEventsHandler.Object,
                 It.IsAny<Dictionary<string, SourceDetail>>(), It.IsAny<IWarningLogger>()));
-
-        mockEventsHandler.Verify(
-            eh => eh.HandleStartTestSessionComplete(It.IsAny<StartTestSessionCompleteEventArgs>()),
-            Times.Once);
     }
 
     [TestMethod]

diff --git a/test/vstest.console.UnitTests/TestPlatformHelpers/TestRequestManagerTests.cs b/test/vstest.console.UnitTests/TestPlatformHelpers/TestRequestManagerTests.cs
@@ -2317,6 +2317,56 @@ public void StartTestSessionShouldUpdateSettings()
         _mockAssemblyMetadataProvider.Verify(a => a.GetFrameworkName(It.IsAny<string>()));
     }
 
+    [TestMethod]
+    public void StartTestSessionShouldSendCompletedEventIfTestPlatformReturnsFalse()
+    {
+        var payload = new StartTestSessionPayload()
+        {
+            Sources = new List<string>() { "a.dll" },
+            RunSettings =
+                @"<?xml version=""1.0"" encoding=""utf-8""?>
+                    <RunSettings>
+                        <RunConfiguration>
+                        </RunConfiguration>
+                    </RunSettings>"
+        };
+
+        var eventsHandler = new Mock<ITestSessionEventsHandler>();
+        _commandLineOptions.IsDesignMode = true;
+
+        _mockAssemblyMetadataProvider.Setup(
+                a => a.GetArchitecture(It.IsAny<string>()))
+            .Returns(Architecture.ARM);
+        _mockAssemblyMetadataProvider.Setup(
+                a => a.GetFrameworkName(It.IsAny<string>()))
+            .Returns(new FrameworkName(Constants.DotNetFramework46));
+
+        _mockTestPlatform.Setup(
+                tp => tp.StartTestSession(
+                    It.IsAny<IRequestData>(),
+                    It.IsAny<StartTestSessionCriteria>(),
+                    It.IsAny<ITestSessionEventsHandler>(),
+                    It.IsAny<Dictionary<string, SourceDetail>>(),
+                    It.IsAny<IWarningLogger>()))
+            .Returns(false)
+            .Callback(
+                (IRequestData _, StartTestSessionCriteria criteria, ITestSessionEventsHandler _, Dictionary<string, SourceDetail> _, IWarningLogger _) =>
+                {
+                    Assert.IsTrue(criteria.RunSettings!.Contains(Constants.DotNetFramework46));
+                    Assert.IsTrue(criteria.RunSettings.Contains(nameof(Architecture.ARM)));
+                });
+
+        _testRequestManager.StartTestSession(
+            payload,
+            new Mock<ITestHostLauncher3>().Object,
+            eventsHandler.Object,
+            _protocolConfig);
+
+        eventsHandler.Verify(eh => eh.HandleStartTestSessionComplete(It.IsAny<StartTestSessionCompleteEventArgs>()));
+        _mockAssemblyMetadataProvider.Verify(a => a.GetArchitecture(It.IsAny<string>()));
+        _mockAssemblyMetadataProvider.Verify(a => a.GetFrameworkName(It.IsAny<string>()));
+    }
+
     [TestMethod]
     public void StartTestSessionShouldThrowSettingsExceptionWhenFindingIncompatibleDataCollectorsInTestSettings()
     {