Fixed issue where proc dump was not getting terminated on no crash

.gitignore

@@ -124,3 +124,8 @@ src/package/sign/sign.nuget.targets
 *.tr.resx
 *.zh-Hans.resx
 *.zh-Hant.resx
+
+# ===========================
+# Vscode files
+# ===========================
+.vscode/

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs

@@ -202,45 +202,56 @@ private void SessionEnded_Handler(object sender, SessionEndEventArgs args)
                 EqtTrace.Info("Blame Collector : Session End");
             }
 
-            // If the last test crashes, it will not invoke a test case end and therefore
-            // In case of crash testStartCount will be greater than testEndCount and we need to write the sequence
-            // And send the attachment
-            if (this.testStartCount > this.testEndCount)
+            try
             {
-                var filepath = Path.Combine(this.GetResultsDirectory(), Constants.AttachmentFileName + "_" + this.attachmentGuid);
-                filepath = this.blameReaderWriter.WriteTestSequence(this.testSequence, this.testObjectDictionary, filepath);
-                var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, filepath, true);
-                this.dataCollectionSink.SendFileAsync(fileTranferInformation);
-            }
+                // If the last test crashes, it will not invoke a test case end and therefore
+                // In case of crash testStartCount will be greater than testEndCount and we need to write the sequence
+                // And send the attachment
+                if (this.testStartCount > this.testEndCount)
+                {
+                    var filepath = Path.Combine(this.GetResultsDirectory(), Constants.AttachmentFileName + "_" + this.attachmentGuid);
+                    filepath = this.blameReaderWriter.WriteTestSequence(this.testSequence, this.testObjectDictionary, filepath);
+                    var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, filepath, true);
+                    this.dataCollectionSink.SendFileAsync(fileTranferInformation);
+                }
 
-            if (this.processDumpEnabled)
-            {
-                // If there was a test case crash or if we need to collect dump on process exit.
-                if (this.testStartCount > this.testEndCount || this.collectDumpAlways)
+                if (this.processDumpEnabled)
                 {
-                    try
+                    // If there was a test case crash or if we need to collect dump on process exit.
+                    if (this.testStartCount > this.testEndCount || this.collectDumpAlways)
                     {
-                        var dumpFile = this.processDumpUtility.GetDumpFile();
-                        if (!string.IsNullOrEmpty(dumpFile))
+                        try
                         {
-                            var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
-                            this.dataCollectionSink.SendFileAsync(fileTranferInformation);
+                            var dumpFile = this.processDumpUtility.GetDumpFile();
+                            if (!string.IsNullOrEmpty(dumpFile))
+                            {
+                                var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true);
+                                this.dataCollectionSink.SendFileAsync(fileTranferInformation);
+                            }
+                            else
+                            {
+                                EqtTrace.Warning("BlameCollector.SessionEnded_Handler: blame:CollectDump was enabled but dump file was not generated.");
+                                this.logger.LogWarning(args.Context, Resources.Resources.ProcDumpNotGenerated);
+                            }
                         }
-                        else
+                        catch (FileNotFoundException ex)
                         {
-                            EqtTrace.Warning("BlameCollector.SessionEnded_Handler: blame:CollectDump was enabled but dump file was not generated.");
-                            this.logger.LogWarning(args.Context, Resources.Resources.ProcDumpNotGenerated);
+                            EqtTrace.Warning(ex.Message);
+                            this.logger.LogWarning(args.Context, ex.Message);
                         }
                     }
-                    catch (FileNotFoundException ex)
-                    {
-                        EqtTrace.Warning(ex.Message);
-                        this.logger.LogWarning(args.Context, ex.Message);
-                    }
                 }
             }
+            finally
+            {
+                // Attempt to terminate the proc dump process if proc dump was enabled
+                if (this.processDumpEnabled)
+                {
+                    this.processDumpUtility.TerminateProcess();
+                }
 
-            this.DeregisterEvents();
+                this.DeregisterEvents();
+            }
         }
 
         /// <summary>

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs

@@ -3,8 +3,6 @@
 
 namespace Microsoft.TestPlatform.Extensions.BlameDataCollector
 {
-    using System.Collections.Generic;
-
     public interface IProcessDumpUtility
     {
         /// <summary>
@@ -31,5 +29,10 @@ public interface IProcessDumpUtility
         /// Is full dump enabled
         /// </param>
         void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump = false);
+
+        /// <summary>
+        /// Terminate the proc dump process
+        /// </summary>
+        void TerminateProcess();
     }
 }

src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs

@@ -44,8 +44,8 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper,
             };
         }
 
-    /// <inheritdoc/>
-     public string GetDumpFile()
+        /// <inheritdoc/>
+        public string GetDumpFile()
         {
             if (this.procDumpProcess == null)
             {
@@ -95,6 +95,20 @@ public void StartProcessDump(int processId, string dumpFileGuid, string testResu
                                             null) as Process;
         }
 
+        /// <inheritdoc/>
+        public void TerminateProcess()
+        {
+            try
+            {
+                EqtTrace.Info("ProcessDumpUtility : Attempting to kill proc dump process.");
+                this.processHelper.TerminateProcess(this.procDumpProcess);
+            }
+            catch (Exception e)
+            {
+                EqtTrace.Warning($"ProcessDumpUtility : Failed to kill proc dump process with exception {e}");
+            }
+        }
+
         /// <summary>
         /// Arguments for procdump.exe
         /// </summary>

test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs

@@ -199,6 +199,30 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled()
             this.mockProcessDumpUtility.Verify(x => x.GetDumpFile(), Times.Once);
         }
 
+        /// <summary>
+        /// The trigger session ended handler should ensure proc dump process is terminated when no crash is detected
+        /// </summary>
+        [TestMethod]
+        public void TriggerSessionEndedHandlerShouldEnsureProcDumpProcessIsTerminated()
+        {
+            // Initializing Blame Data Collector
+            this.blameDataCollector.Initialize(
+                this.GetDumpConfigurationElement(),
+                this.mockDataColectionEvents.Object,
+                this.mockDataCollectionSink.Object,
+                this.mockLogger.Object,
+                this.context);
+
+            // Mock proc dump utility terminate process call
+            this.mockProcessDumpUtility.Setup(x => x.TerminateProcess());
+
+            // Raise
+            this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext));
+
+            // Verify GetDumpFiles Call
+            this.mockProcessDumpUtility.Verify(x => x.TerminateProcess(), Times.Once);
+        }
+
         /// <summary>
         /// The trigger session ended handler should not dump files if proc dump was enabled and test host did not crash
         /// </summary>

test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs

@@ -292,6 +292,28 @@ public void StartProcessDumpWillStartExeCorrespondingTo32BitTestHostProcessIn64B
             this.mockProcessHelper.Verify(x => x.LaunchProcess(Path.Combine("D:\\procdump", "procdump.exe"), It.IsAny<string>(), It.IsAny<string>(), null, null, null));
         }
 
+        /// <summary>
+        /// Ensure terminate process calls terminate on proc dump process
+        /// </summary>
+        [TestMethod]
+        public void TerminateProcessDumpShouldCallTerminateOnProcDumpProcess()
+        {
+            var processDumpUtility = new ProcessDumpUtility(
+            this.mockProcessHelper.Object,
+            this.mockFileHelper.Object,
+            this.mockPlatformEnvironment.Object,
+            this.mockNativeMethodsHelper.Object);
+
+            // Mock process helper
+            this.mockProcessHelper.Setup(x => x.TerminateProcess(It.IsAny<object>()));
+
+            // Raise
+            processDumpUtility.TerminateProcess();
+
+            // Verify
+            this.mockProcessHelper.Verify(x => x.TerminateProcess(It.IsAny<object>()), Times.Once);
+        }
+
         [TestCleanup]
         public void TestCleanup()
         {