From 5dd8e9eb4b6e8bc5688aecb4f992f1618a635fcf Mon Sep 17 00:00:00 2001 From: nohwnd Date: Tue, 1 Sep 2020 14:57:18 +0200 Subject: [PATCH 1/3] Upload on session end when HangDump started In rare cases where we have a crashing tests and short hang timeout, we get into situation where hang dumper started dumping, but then session ends because of the crash, and dumps are not uploaded because the session end is only uploading when there is crash dump enabled. After this change we look for any dump in any case and upload all the dumps that hang dumper was able to create. --- .../BlameCollector.cs | 60 ++++++++----------- 1 file changed, 24 insertions(+), 36 deletions(-) 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 From bae96efb441db083d22d7e14414c0051a6aa0da7 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Tue, 1 Sep 2020 15:11:40 +0200 Subject: [PATCH 2/3] Fix procdump condition --- .../CrashDumperFactory.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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}."); From a7293ff023ecbbe6e07ed721398df87f6b229dc1 Mon Sep 17 00:00:00 2001 From: nohwnd Date: Wed, 2 Sep 2020 09:13:16 +0200 Subject: [PATCH 3/3] Remove test, because we want to upload all dumps on session end when hangdump and crash dump run into each other, otherwise hang dumps would not get uploaded and the process would just end, leaving them on the local disk. --- .../BlameCollectorTests.cs | 27 ------------------- 1 file changed, 27 deletions(-) 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 ///