Pdb Lock Issue (microsoft#2029)

* Pdb lock issue * Added comments
Cosifne · May 30, 2019 · ac001e6 · ac001e6
1 parent bcbcf77
commit ac001e6
Show file tree

Hide file tree

Showing 2 changed files with 22 additions and 23 deletions.
diff --git a/src/Microsoft.TestPlatform.VsTestConsole.TranslationLayer/VsTestConsoleRequestSender.cs b/src/Microsoft.TestPlatform.VsTestConsole.TranslationLayer/VsTestConsoleRequestSender.cs
@@ -417,13 +417,14 @@ private void SendMessageAndListenAndReportTestCases(IEnumerable<string> sources,
             catch (Exception exception)
             {
                 EqtTrace.Error("Aborting Test Discovery Operation: {0}", exception);
-
                 eventHandler.HandleLogMessage(TestMessageLevel.Error, TranslationLayerResources.AbortedTestsDiscovery);
-
                 var discoveryCompleteEventArgs = new DiscoveryCompleteEventArgs(-1, true);
                 eventHandler.HandleDiscoveryComplete(discoveryCompleteEventArgs, null);
 
-                CleanupCommunicationIfProcessExit();
+                // Earlier we were closing the connection with vstest.console in case of exceptions
+                // Removing that code because vstest.console might be in a healthy state and letting the client
+                // know of the error, so that the TL can wait for the next instruction from the client itself.
+                // Also, connection termination might not kill the process which could result in files being locked by testhost.
             }
 
             this.testPlatformEventSource.TranslationLayerDiscoveryStop();
@@ -483,7 +484,10 @@ private async Task SendMessageAndListenAndReportTestCasesAsync(IEnumerable<strin
                 var discoveryCompleteEventArgs = new DiscoveryCompleteEventArgs(-1, true);
                 eventHandler.HandleDiscoveryComplete(discoveryCompleteEventArgs, null);
 
-                CleanupCommunicationIfProcessExit();
+                // Earlier we were closing the connection with vstest.console in case of exceptions
+                // Removing that code because vstest.console might be in a healthy state and letting the client
+                // know of the error, so that the TL can wait for the next instruction from the client itself.
+                // Also, connection termination might not kill the process which could result in files being locked by testhost.
             }
 
             this.testPlatformEventSource.TranslationLayerDiscoveryStop();
@@ -537,7 +541,11 @@ private void SendMessageAndListenAndReportTestResults(string messageType, object
                 eventHandler.HandleLogMessage(TestMessageLevel.Error, TranslationLayerResources.AbortedTestsRun);
                 var completeArgs = new TestRunCompleteEventArgs(null, false, true, exception, null, TimeSpan.Zero);
                 eventHandler.HandleTestRunComplete(completeArgs, null, null, null);
-                this.CleanupCommunicationIfProcessExit();
+
+                // Earlier we were closing the connection with vstest.console in case of exceptions
+                // Removing that code because vstest.console might be in a healthy state and letting the client
+                // know of the error, so that the TL can wait for the next instruction from the client itself.
+                // Also, connection termination might not kill the process which could result in files being locked by testhost.
             }
 
             this.testPlatformEventSource.TranslationLayerExecutionStop();
@@ -591,21 +599,16 @@ private async Task SendMessageAndListenAndReportTestResultsAsync(string messageT
                 eventHandler.HandleLogMessage(TestMessageLevel.Error, TranslationLayerResources.AbortedTestsRun);
                 var completeArgs = new TestRunCompleteEventArgs(null, false, true, exception, null, TimeSpan.Zero);
                 eventHandler.HandleTestRunComplete(completeArgs, null, null, null);
-                this.CleanupCommunicationIfProcessExit();
+
+                // Earlier we were closing the connection with vstest.console in case of exceptions
+                // Removing that code because vstest.console might be in a healthy state and letting the client
+                // know of the error, so that the TL can wait for the next instruction from the client itself.
+                // Also, connection termination might not kill the process which could result in files being locked by testhost.
             }
 
             this.testPlatformEventSource.TranslationLayerExecutionStop();
         }
 
-        private void CleanupCommunicationIfProcessExit()
-        {
-            if (this.processExitCancellationTokenSource != null
-                && this.processExitCancellationTokenSource.IsCancellationRequested)
-            {
-                this.communicationManager.StopServer();
-            }
-        }
-
         private Message TryReceiveMessage()
         {
             Message message = null;

diff --git a/test/TranslationLayer.UnitTests/VsTestConsoleRequestSenderTests.cs b/test/TranslationLayer.UnitTests/VsTestConsoleRequestSenderTests.cs
@@ -673,7 +673,7 @@ public async Task DiscoverTestsAsyncShouldAbortOnExceptionInSendMessage()
         }
 
         [TestMethod]
-        public void DiscoverTestsShouldAbortWhenProcessExited()
+        public void DiscoverTestsShouldLogErrorWhenProcessExited()
         {
             var mockHandler = new Mock<ITestDiscoveryEventsHandler2>();
             var sources = new List<string> { "1.dll" };
@@ -698,11 +698,10 @@ public void DiscoverTestsShouldAbortWhenProcessExited()
 
             manualEvent.WaitOne();
             mockHandler.Verify(mh => mh.HandleLogMessage(TestMessageLevel.Error, It.IsAny<string>()), Times.Once);
-            this.mockCommunicationManager.Verify(cm => cm.StopServer(), Times.Once);
         }
 
         [TestMethod]
-        public async Task DiscoverTestsAsyncShouldAbortWhenProcessExited()
+        public async Task DiscoverTestsAsyncShouldLogErrorWhenProcessExited()
         {
             var mockHandler = new Mock<ITestDiscoveryEventsHandler2>();
             var sources = new List<string> { "1.dll" };
@@ -723,7 +722,6 @@ public async Task DiscoverTestsAsyncShouldAbortWhenProcessExited()
             await this.requestSenderAsync.DiscoverTestsAsync(sources, null, new TestPlatformOptions(), mockHandler.Object);
 
             mockHandler.Verify(mh => mh.HandleLogMessage(TestMessageLevel.Error, It.IsAny<string>()), Times.Once);
-            this.mockCommunicationManager.Verify(cm => cm.StopServer(), Times.Once);
         }
 
         #endregion
@@ -1840,7 +1838,7 @@ public async Task StartTestRunAsyncShouldAbortOnExceptionInSendMessage()
         }
 
         [TestMethod]
-        public void StartTestRunShouldAbortOnProcessExited()
+        public void StartTestRunShouldLogErrorOnProcessExited()
         {
             var mockHandler = new Mock<ITestRunEventsHandler>();
             var manualEvent = new ManualResetEvent(false);
@@ -1869,11 +1867,10 @@ public void StartTestRunShouldAbortOnProcessExited()
 
             manualEvent.WaitOne();
             mockHandler.Verify(mh => mh.HandleLogMessage(TestMessageLevel.Error, It.IsAny<string>()), Times.Once);
-            this.mockCommunicationManager.Verify(cm => cm.StopServer(), Times.Once);
         }
 
         [TestMethod]
-        public async Task StartTestRunAsyncShouldAbortOnProcessExited()
+        public async Task StartTestRunAsyncShouldLogErrorOnProcessExited()
         {
             var mockHandler = new Mock<ITestRunEventsHandler>();
             var sources = new List<string> { "1.dll" };
@@ -1898,7 +1895,6 @@ public async Task StartTestRunAsyncShouldAbortOnProcessExited()
             await this.requestSenderAsync.StartTestRunAsync(sources, null, null, mockHandler.Object);
 
             mockHandler.Verify(mh => mh.HandleLogMessage(TestMessageLevel.Error, It.IsAny<string>()), Times.Once);
-            this.mockCommunicationManager.Verify(cm => cm.StopServer(), Times.Once);
         }
 
         #endregion