diff --git a/.gitignore b/.gitignore index fd0b49f4b4..4a52aa889f 100644 --- a/.gitignore +++ b/.gitignore @@ -124,3 +124,8 @@ src/package/sign/sign.nuget.targets *.tr.resx *.zh-Hans.resx *.zh-Hant.resx + +# =========================== +# Vscode files +# =========================== +.vscode/ diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index b880a66fab..7674fe69af 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/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(); + } } /// diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs index 5aff12e962..e8b78e2000 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs @@ -3,8 +3,6 @@ namespace Microsoft.TestPlatform.Extensions.BlameDataCollector { - using System.Collections.Generic; - public interface IProcessDumpUtility { /// @@ -31,5 +29,10 @@ public interface IProcessDumpUtility /// Is full dump enabled /// void StartProcessDump(int processId, string dumpFileGuid, string testResultsDirectory, bool isFullDump = false); + + /// + /// Terminate the proc dump process + /// + void TerminateProcess(); } } diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs index 8fd3d88356..9cd039df81 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs @@ -44,8 +44,8 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper, }; } - /// - public string GetDumpFile() + /// + public string GetDumpFile() { if (this.procDumpProcess == null) { @@ -95,6 +95,20 @@ public void StartProcessDump(int processId, string dumpFileGuid, string testResu null) as Process; } + /// + 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}"); + } + } + /// /// Arguments for procdump.exe /// diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index 3f1da072fd..c7bbb7efd0 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -199,6 +199,30 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled() this.mockProcessDumpUtility.Verify(x => x.GetDumpFile(), Times.Once); } + /// + /// The trigger session ended handler should ensure proc dump process is terminated when no crash is detected + /// + [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); + } + /// /// The trigger session ended handler should not dump files if proc dump was enabled and test host did not crash /// diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs index 17a8277a62..cc7a37cdcd 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs +++ b/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(), It.IsAny(), null, null, null)); } + /// + /// Ensure terminate process calls terminate on proc dump process + /// + [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())); + + // Raise + processDumpUtility.TerminateProcess(); + + // Verify + this.mockProcessHelper.Verify(x => x.TerminateProcess(It.IsAny()), Times.Once); + } + [TestCleanup] public void TestCleanup() {