diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index 834b410510..3f8ad9bcd3 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs @@ -46,7 +46,6 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier private IInactivityTimer inactivityTimer; private TimeSpan inactivityTimespan = TimeSpan.FromMinutes(DefaultInactivityTimeInMinutes); private int testHostProcessId; - private bool dumpWasCollectedByHangDumper; private string targetFramework; private List> environmentVariables = new List>(); private bool uploadDumpFiles; @@ -223,7 +222,7 @@ private void CollectDumpAndAbortTesthost() } catch (Exception ex) { - ConsoleOutput.Instance.Error(true, $"Blame: Creating hang dump failed with error {ex}."); + this.logger.LogError(this.context.SessionDataCollectionContext, $"Blame: Creating hang dump failed with error.", ex); } if (this.collectProcessDumpOnTrigger) @@ -244,7 +243,6 @@ private void CollectDumpAndAbortTesthost() { if (!string.IsNullOrEmpty(dumpFile)) { - this.dumpWasCollectedByHangDumper = true; var fileTransferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true, this.fileHelper); this.dataCollectionSink.SendFileAsync(fileTransferInformation); } @@ -263,7 +261,7 @@ private void CollectDumpAndAbortTesthost() } catch (Exception ex) { - ConsoleOutput.Instance.Error(true, $"Blame: Collecting hang dump failed with error {ex}."); + this.logger.LogError(this.context.SessionDataCollectionContext, $"Blame: Collecting hang dump failed with error.", ex); } } else @@ -452,47 +450,37 @@ private void SessionEndedHandler(object sender, SessionEndEventArgs args) this.logger.LogWarning(this.context.SessionDataCollectionContext, Resources.Resources.NotGeneratingSequenceFile); } - if (this.collectProcessDumpOnTrigger) + if (this.uploadDumpFiles) { - // If there was a test case crash or if we need to collect dump on process exit. - // - // Do not try to collect dump when we already collected one from the hang dump - // we won't dump the killed process again and that would just show a warning on the command line - if ((this.testStartCount > this.testEndCount || this.collectDumpAlways) && !this.dumpWasCollectedByHangDumper) + try { - if (this.uploadDumpFiles) + var dumpFiles = this.processDumpUtility.GetDumpFiles(); + foreach (var dumpFile in dumpFiles) { - try + if (!string.IsNullOrEmpty(dumpFile)) { - var dumpFiles = this.processDumpUtility.GetDumpFiles(); - foreach (var dumpFile in dumpFiles) + try { - if (!string.IsNullOrEmpty(dumpFile)) - { - try - { - var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true); - this.dataCollectionSink.SendFileAsync(fileTranferInformation); - } - catch (FileNotFoundException ex) - { - EqtTrace.Warning(ex.ToString()); - this.logger.LogWarning(args.Context, ex.ToString()); - } - } + var fileTranferInformation = new FileTransferInformation(this.context.SessionDataCollectionContext, dumpFile, true); + this.dataCollectionSink.SendFileAsync(fileTranferInformation); + } + catch (FileNotFoundException ex) + { + EqtTrace.Warning(ex.ToString()); + this.logger.LogWarning(args.Context, ex.ToString()); } } - catch (FileNotFoundException ex) - { - EqtTrace.Warning(ex.ToString()); - this.logger.LogWarning(args.Context, ex.ToString()); - } - } - else - { - EqtTrace.Info("BlameCollector.CollectDumpAndAbortTesthost: Custom path to dump directory was provided via VSTEST_DUMP_PATH. Skipping attachment upload, the caller is responsible for collecting and uploading the dumps themselves."); } } + catch (FileNotFoundException ex) + { + EqtTrace.Warning(ex.ToString()); + this.logger.LogWarning(args.Context, ex.ToString()); + } + } + else + { + EqtTrace.Info("BlameCollector.CollectDumpAndAbortTesthost: Custom path to dump directory was provided via VSTEST_DUMP_PATH. Skipping attachment upload, the caller is responsible for collecting and uploading the dumps themselves."); } } finally diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs index 2adb34df10..469885579b 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/CrashDumperFactory.cs @@ -44,7 +44,7 @@ public ICrashDumper Create(string targetFramework) // This proven to be working okay while net5.0 could not create dumps from Task.Run, and I was using this same technique // to get dump of testhost. This needs PROCDUMP_PATH set to directory with procdump.exe, or having it in path. var procdumpOverride = Environment.GetEnvironmentVariable("VSTEST_DUMP_FORCEPROCDUMP")?.Trim(); - var forceUsingProcdump = string.IsNullOrWhiteSpace(procdumpOverride) && procdumpOverride != "0"; + var forceUsingProcdump = !string.IsNullOrWhiteSpace(procdumpOverride) && procdumpOverride != "0"; if (forceUsingProcdump) { EqtTrace.Info($"CrashDumperFactory: This is Windows on {targetFramework}. Forcing the use of ProcDumpCrashDumper that uses ProcDump utility, via VSTEST_DUMP_FORCEPROCDUMP={procdumpOverride}."); diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index fffd563384..880e742290 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -403,33 +403,6 @@ public void TriggerSessionEndedHandlerShouldEnsureProcDumpProcessIsTerminated() this.mockProcessDumpUtility.Verify(x => x.DetachFromTargetProcess(It.IsAny()), Times.Once); } - /// - /// The trigger session ended handler should not dump files if proc dump was enabled and test host did not crash - /// - [TestMethod] - public void TriggerSessionEndedHandlerShouldNotGetDumpFileIfNoCrash() - { - // Initializing Blame Data Collector - this.blameDataCollector.Initialize( - this.GetDumpConfigurationElement(), - this.mockDataColectionEvents.Object, - this.mockDataCollectionSink.Object, - this.mockLogger.Object, - this.context); - - // Setup - this.mockProcessDumpUtility.Setup(x => x.GetDumpFiles()).Returns(new[] { this.filepath }); - this.mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) - .Returns(this.filepath); - - // Raise - this.mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(this.dataCollectionContext, 1234)); - this.mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(this.dataCollectionContext)); - - // Verify GetDumpFiles Call - this.mockProcessDumpUtility.Verify(x => x.GetDumpFiles(), Times.Never); - } - /// /// The trigger session ended handler should get dump files if collect dump on exit was enabled irrespective of completed test case count ///