From 84c52d9be746b56ac070d2644517feae77985468 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 6 Apr 2023 17:30:20 +0200 Subject: [PATCH 01/10] Fix hangdump running into crashdump --- .../BlameCollector.cs | 7 +- .../ProcDumpDumper.cs | 118 +++++++++++++++--- 2 files changed, 105 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index 02c9e97215..a85fe388d2 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs @@ -245,9 +245,10 @@ private void CollectDumpAndAbortTesthost() if (_collectProcessDumpOnCrash) { - // Detach procdump from the testhost process to prevent testhost process from crashing - // if/when we try to kill the existing proc dump process. - // And also prevent collecting dump on exit of the process. + // Detach the dumper from the testhost process to prevent testhost process. When the dumper is procdump.exe + // it must be detached before we try to dump the process, and simply killing it would take down the testhost process. + // + // Detaching also prevents creating an extra dump at the exit of the testhost process. _processDumpUtility.DetachFromTargetProcess(_testHostProcessId); } diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs index 4436f79ff1..78a2778920 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs @@ -4,10 +4,11 @@ using System; using System.Collections.Generic; using System.Diagnostics; -using System.Diagnostics.CodeAnalysis; using System.Globalization; using System.IO; using System.Linq; +using System.Threading; +using System.Threading.Tasks; using Microsoft.VisualStudio.TestPlatform.CoreUtilities.Helpers; using Microsoft.VisualStudio.TestPlatform.Execution; @@ -32,6 +33,7 @@ public class ProcDumpDumper : ICrashDumper, IHangDumper private readonly IFileHelper _fileHelper; private readonly IEnvironment _environment; private readonly IEnvironmentVariableHelper _environmentVariableHelper; + private string? _procDumpPath; private Process? _procDumpProcess; private string? _tempDirectory; private string? _dumpFileName; @@ -39,6 +41,9 @@ public class ProcDumpDumper : ICrashDumper, IHangDumper private string? _outputDirectory; private Process? _process; private string? _outputFilePrefix; + private bool _isCrashDumpInProgress; + private bool _crashDumpDetected; + private readonly int _timeout = EnvironmentHelper.GetConnectionTimeout() * 1000; private readonly ProcDumpExecutableHelper _procDumpExecutableHelper; public ProcDumpDumper() @@ -63,25 +68,49 @@ internal ProcDumpDumper(IProcessHelper processHelper, IFileHelper fileHelper, IE _procDumpExecutableHelper = new ProcDumpExecutableHelper(processHelper, fileHelper, environment, environmentVariableHelper); } - [SuppressMessage("Performance", "CA1822:Mark members as static", Justification = "Part of the public API")] protected Action OutputReceivedCallback => (process, data) => - // useful for visibility when debugging this tool - // Console.ForegroundColor = ConsoleColor.Cyan; - // Console.WriteLine(data); - // Console.ForegroundColor = ConsoleColor.White; - // Log all standard output message of procdump in diag files. - // Otherwise they end up coming on console in pipleine. + { EqtTrace.Info($"ProcDumpDumper.OutputReceivedCallback: Output received from procdump process: {data ?? ""}"); + // This is what procdump writes to the output when it is creating a crash dump. When hangdump triggers while we are writing a crash dump + // we probably don't want to cancel, because that crashdump probably has the more interesting info. + // [16:06:59] Dump 1 initiated: + // [16:07:00] Dump 1 writing: Estimated dump file size is 11034 MB. + // [16:07:09] Dump 1 complete: 11034 MB written in 10.1 seconds + // We also want to know when we completed writing a dump (and not just set _isCrashDumpInProgress once), because dumpcount larger than 1 + // can be provided externally and then the first dump would prevent hangdump forever from stopping the process, but the not every dump is crashing the process + // so we would run forever. + // + // Yes the two ifs below depend on the content being in english, and containg those words (which is the case for procdump from 2017 till 2023 at least), + // if we get different language it should not break us, we will just cancel more aggressively (unfortunately). + if (data != null && data.Contains("initiated")) + { + EqtTrace.Info($"ProcDumpDumper.OutputReceivedCallback: Output received from procdump process contains 'initiated', crashdump is being written. Don't cancel procdump right now."); + _isCrashDumpInProgress = true; + _crashDumpDetected = true; + } + + if (data != null && data.Contains("complete")) + { + EqtTrace.Info($"ProcDumpDumper.OutputReceivedCallback: Output received from procdump process contains 'complete' dump is finished, you can cancel procdump if you need."); + _isCrashDumpInProgress = false; + // Do not reset _crashDumpDetected back to false here. Any detected crash dumping should keep it true, so we don't throw away the dump. + } + }; + + internal static Action ErrorReceivedCallback => (process, data) => + EqtTrace.Error($"ProcDumpDumper.ErrorReceivedCallback: Error received from procdump process: {data ?? ""}"); + /// public void WaitForDumpToFinish() { - if (_processHelper == null) + if (_procDumpProcess == null) { EqtTrace.Info($"ProcDumpDumper.WaitForDumpToFinish: ProcDump was not previously attached, this might indicate error during setup, look for ProcDumpDumper.AttachToTargetProcess."); + return; } - _processHelper?.WaitForProcessExit(_procDumpProcess); + _processHelper.WaitForProcessExit(_procDumpProcess); } /// @@ -118,12 +147,13 @@ public void AttachToTargetProcess(int processId, string outputDirectory, DumpTyp isFullDump: dumpType == DumpTypeOption.Full); EqtTrace.Info($"ProcDumpDumper.AttachToTargetProcess: Running ProcDump with arguments: '{procDumpArgs}'."); + _procDumpPath = procDumpPath; _procDumpProcess = (Process)_processHelper.LaunchProcess( procDumpPath, procDumpArgs, _tempDirectory, null, - null, + ErrorReceivedCallback, null, OutputReceivedCallback); @@ -133,27 +163,79 @@ public void AttachToTargetProcess(int processId, string outputDirectory, DumpTyp /// public void DetachFromTargetProcess(int targetProcessId) { - if (_procDumpProcess == null) + if (_procDumpProcess == null || _procDumpPath == null) { EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump was not previously attached, this might indicate error during setup, look for ProcDumpDumper.AttachToTargetProcess."); return; } + if (_procDumpProcess.HasExited) + { + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump already exited, returning."); + return; + } + + // Process? procDumpCancelProcess = null; try { + if (_isCrashDumpInProgress) + { + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump is currently dumping process '{targetProcessId}', wait at most {_timeout} ms for it to finish so we get the crashdump."); + var procDumpExit = Task.Run(() => _procDumpProcess.WaitForExit(_timeout)); + // Could do this better with completion source, but we have nothing better to do in this process anyway, + // than wait for the crashdump to finish. + while (_isCrashDumpInProgress && !procDumpExit.IsCompleted) + { + // The timeout is driven by VSTEST_CONNECTION_TIMEOUT which is specified in seconds so it cannot be less than 1000ms. + // (Technically it can be 0, but that will fail way before we ever reach here.) + Thread.Sleep(500); + EqtTrace.Verbose($"ProcDumpDumper.DetachFromTargetProcess: Waiting for procdump to finish dumping the process."); + } + + if (procDumpExit.IsCompleted && procDumpExit.Result == false) + { + EqtTrace.Verbose($"ProcDumpDumper.DetachFromTargetProcess: Procdump did not exit after {_timeout} ms."); + } + } + + if (_procDumpProcess.HasExited) + { + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump already exited, returning."); + return; + } + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump detaching from target process '{targetProcessId}'."); + // Alternative to sending this event is calling Procdump -cancel (the dumped process id, not the existing Procdump.exe process id). + // But not all versions of procdump have that parameter (definitely not the one we are getting from the Procdump 0.0.1 nuget package), and it works reliably. + // What was not reliable before was that we sent the message and immediately killed procdump, that caused testhost to crash occasionally, because procdump was not detached, + // and killing the process when it is not detached takes the observed process with it. new Win32NamedEvent($"Procdump-{targetProcessId}").Set(); + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: Cancel event was sent to Procdump."); + + var sw = Stopwatch.StartNew(); + var exited = _procDumpProcess.WaitForExit(_timeout); + if (exited) + { + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump cancelled after {sw.ElapsedMilliseconds} ms."); + } + else + { + EqtTrace.Info($"ProcDumpDumper.DetachFromTargetProcess: ProcDump cancellation timed out, after {sw.ElapsedMilliseconds} ms."); + } } finally { try { - EqtTrace.Info("ProcDumpDumper.DetachFromTargetProcess: Attempting to kill proc dump process."); - _processHelper.TerminateProcess(_procDumpProcess); + if (!_procDumpProcess.HasExited) + { + EqtTrace.Info("ProcDumpDumper.DetachFromTargetProcess: Procdump process is still running after cancellation, force killing it. This will probably take down the process it is attached to as well."); + _processHelper.TerminateProcess(_procDumpProcess); + } } catch (Exception e) { - EqtTrace.Warning($"ProcDumpDumper.DetachFromTargetProcess: Failed to kill proc dump process with exception {e}"); + EqtTrace.Warning($"ProcDumpDumper.DetachFromTargetProcess: Failed to kill procdump process with exception {e}"); } } } @@ -174,7 +256,9 @@ public IEnumerable GetDumpFiles(bool processCrashed) return allDumps; } - if (processCrashed) + // When we know there was a crash dump collected, either because we detected it from the procdump output, or because + // we got that info from exit code, don't try to remove the extra crash dump that we generate on process exit. + if (_crashDumpDetected || processCrashed) { return allDumps; } @@ -241,7 +325,7 @@ public void Dump(int processId, string outputDirectory, DumpTypeOption dumpType) procDumpArgs, tempDirectory, null, - null, + ErrorReceivedCallback, null, OutputReceivedCallback); From dcdcc4654918385d6dab9a590ed23c13a7bd13ad Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Tue, 11 Apr 2023 12:57:41 +0200 Subject: [PATCH 02/10] Figure out crash dump --- .../BlameCollector.cs | 8 +++++-- .../ICrashDumper.cs | 2 +- .../Interfaces/IProcessDumpUtility.cs | 2 +- .../NetClientCrashDumper.cs | 2 +- .../ProcDumpDumper.cs | 22 ++++++++++++++----- .../ProcessDumpUtility.cs | 4 ++-- .../PublicAPI/PublicAPI.Shipped.txt | 6 ++--- .../BlameCollectorTests.cs | 22 +++++++++---------- .../ProcessDumpUtilityTests.cs | 3 ++- 9 files changed, 44 insertions(+), 27 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index a85fe388d2..53b9a2d1a0 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs @@ -66,6 +66,7 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier private bool _uploadDumpFiles; private string? _tempDirectory; private string? _monitorPostmortemDumpFolderPath; + private DateTime? _testSessionEndedTimestamp; /// /// Initializes a new instance of the class. @@ -272,7 +273,9 @@ private void CollectDumpAndAbortTesthost() { try { - var dumpFiles = _processDumpUtility.GetDumpFiles(true, /* if we killed it by hang dumper, we already have our dump, otherwise it might have crashed, and we want all dumps */ !hangDumpSuccess); + var dumpFiles = _processDumpUtility.GetDumpFiles(true, + /* if we killed it by hang dumper, we already have our dump, otherwise it might have crashed, and we want all dumps */ !hangDumpSuccess, + _testSessionEndedTimestamp); foreach (var dumpFile in dumpFiles) { try @@ -500,6 +503,7 @@ private void EventsTestCaseEnd(object? sender, TestCaseEndEventArgs e) /// SessionEndEventArgs private void SessionEndedHandler(object? sender, SessionEndEventArgs args) { + _testSessionEndedTimestamp = DateTime.Now; TPDebug.Assert(_testSequence != null && _testObjectDictionary != null && _context != null && _dataCollectionSink != null && _logger != null, "Initialize must be called before calling this method"); ResetInactivityTimer(); @@ -531,7 +535,7 @@ private void SessionEndedHandler(object? sender, SessionEndEventArgs args) { try { - var dumpFiles = _processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: _collectDumpAlways, processCrashedWhenRunningTests); + var dumpFiles = _processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: _collectDumpAlways, processCrashedWhenRunningTests, _testSessionEndedTimestamp); foreach (var dumpFile in dumpFiles) { if (!dumpFile.IsNullOrEmpty()) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs index 6f74ed7edc..86a1a86b64 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs @@ -14,5 +14,5 @@ public interface ICrashDumper void DetachFromTargetProcess(int processId); - IEnumerable GetDumpFiles(bool processCrashed); + IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessionEndTimestamp); } diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs index a5a559197d..fd0d8a59ed 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs @@ -16,7 +16,7 @@ public interface IProcessDumpUtility /// /// Path of dump file /// - IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed); + IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed, DateTime? testSessionEndedTimestamp); /// /// Launch proc dump process diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs index 0fe0358e29..a8ce171337 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs @@ -31,7 +31,7 @@ public void DetachFromTargetProcess(int processId) // here we might consider renaming the files to have timestamp } - public IEnumerable GetDumpFiles(bool processCrashed) + public IEnumerable GetDumpFiles(bool _, DateTime? _2) { return _fileHelper.DirectoryExists(_outputDirectory) ? _fileHelper.GetFiles(_outputDirectory, "*_crashdump*.dmp", SearchOption.AllDirectories) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs index 78a2778920..c49b0feb4b 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs @@ -42,7 +42,7 @@ public class ProcDumpDumper : ICrashDumper, IHangDumper private Process? _process; private string? _outputFilePrefix; private bool _isCrashDumpInProgress; - private bool _crashDumpDetected; + private readonly List _crashDumpTimestamps = new(); private readonly int _timeout = EnvironmentHelper.GetConnectionTimeout() * 1000; private readonly ProcDumpExecutableHelper _procDumpExecutableHelper; @@ -87,14 +87,14 @@ internal ProcDumpDumper(IProcessHelper processHelper, IFileHelper fileHelper, IE { EqtTrace.Info($"ProcDumpDumper.OutputReceivedCallback: Output received from procdump process contains 'initiated', crashdump is being written. Don't cancel procdump right now."); _isCrashDumpInProgress = true; - _crashDumpDetected = true; } if (data != null && data.Contains("complete")) { EqtTrace.Info($"ProcDumpDumper.OutputReceivedCallback: Output received from procdump process contains 'complete' dump is finished, you can cancel procdump if you need."); _isCrashDumpInProgress = false; - // Do not reset _crashDumpDetected back to false here. Any detected crash dumping should keep it true, so we don't throw away the dump. + // Do not reset _crashDumpDetected back to false here. Any detected crash dumping should keep it true, so we don't throw away the dump. + _crashDumpTimestamps.Add(DateTime.Now); } }; @@ -240,7 +240,7 @@ public void DetachFromTargetProcess(int targetProcessId) } } - public IEnumerable GetDumpFiles(bool processCrashed) + public IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessionEndedTimestamp) { var allDumps = _fileHelper.DirectoryExists(_outputDirectory) ? _fileHelper.GetFiles(_outputDirectory, "*_crashdump*.dmp", SearchOption.AllDirectories) @@ -258,7 +258,19 @@ public IEnumerable GetDumpFiles(bool processCrashed) // When we know there was a crash dump collected, either because we detected it from the procdump output, or because // we got that info from exit code, don't try to remove the extra crash dump that we generate on process exit. - if (_crashDumpDetected || processCrashed) + var crashDumpDetected = _crashDumpTimestamps.Count == 0 + // When there are 0 crashdumps detected, we know the detection was false. + ? false + : _crashDumpTimestamps.Count > 1 + // When there are more than 1 crashdumps detected, we know it was true, and want to keep all dumps. + ? true + : testSessionEndedTimestamp == null + // When test session did not end and we have 1 crash dump it was a crash for sure. + ? true + // When session ended, and we created a dump within 100ms around it, it was most likely an + // on process exit dump. + : Math.Abs((_crashDumpTimestamps.Last() - (DateTime)testSessionEndedTimestamp!).TotalMilliseconds) > 100; + if (crashDumpDetected || processCrashed) { return allDumps; } diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs index fc42574303..003895d619 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs @@ -46,7 +46,7 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper, EqtTrace.Info($"ProcessDumpUtility.OutputReceivedCallback: Output received from procdump process: {data ?? ""}"); /// - public IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed) + public IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed, DateTime? testSessionEndedTimestamp) { if (!_wasHangDumped) { @@ -56,7 +56,7 @@ public IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCras // If the process was hang dumped we killed it ourselves, so it crashed when executing tests, // but we already have the hang dump, and should not also collect the exit dump that we got // from killing the process by the hang dumper. - IEnumerable crashDumps = _crashDumper?.GetDumpFiles(processCrashed) ?? new List(); + IEnumerable crashDumps = _crashDumper?.GetDumpFiles(processCrashed, testSessionEndedTimestamp) ?? new List(); IEnumerable hangDumps = _fileHelper.DirectoryExists(_hangDumpDirectory) ? _fileHelper.GetFiles(_hangDumpDirectory, "*_hangdump*.dmp", SearchOption.TopDirectoryOnly) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt index a5ac44b058..9e54bdcff4 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt @@ -34,7 +34,6 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.IBlameReaderWriter.WriteTes Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.AttachToTargetProcess(int processId, string! outputDirectory, Microsoft.TestPlatform.Extensions.BlameDataCollector.DumpTypeOption dumpType, bool collectAlways, System.Action! logWarning) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.DetachFromTargetProcess(int processId) -> void -Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.WaitForDumpToFinish() -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumperFactory Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumperFactory.Create(string! targetFramework) -> Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper! @@ -57,7 +56,6 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcDumpArgsBuilder.BuildH Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(int processId, string! filename, System.Collections.Generic.IEnumerable! procDumpExceptionsList, bool isFullDump) -> string! Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.DetachFromTargetProcess(int targetProcessId) -> void -Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed) -> System.Collections.Generic.IEnumerable! Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.StartHangBasedProcessDump(int processId, string! testResultsDirectory, bool isFullDump, string! targetFramework, System.Action? logWarning = null) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.StartTriggerBasedProcessDump(int processId, string! testResultsDirectory, bool isFullDump, string! targetFramework, bool collectAlways, System.Action! logWarning) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpArgsBuilder @@ -68,7 +66,6 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.AttachToTargetProcess(int processId, string! outputDirectory, Microsoft.TestPlatform.Extensions.BlameDataCollector.DumpTypeOption dumpType, bool collectAlways, System.Action! logWarning) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.DetachFromTargetProcess(int targetProcessId) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.Dump(int processId, string! outputDirectory, Microsoft.TestPlatform.Extensions.BlameDataCollector.DumpTypeOption dumpType) -> void -Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.OutputReceivedCallback.get -> System.Action! Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.ProcDumpDumper() -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.ProcDumpDumper(Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces.IProcessHelper! processHelper, Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper! fileHelper, Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces.IEnvironment! environment) -> void @@ -81,3 +78,6 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.ReadTestSeq Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.WriteTestSequence(System.Collections.Generic.List! testSequence, System.Collections.Generic.Dictionary! testObjectDictionary, string! filePath) -> string! Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.XmlReaderWriter(Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper! fileHelper) -> void override Microsoft.TestPlatform.Extensions.BlameDataCollector.BlameCollector.Initialize(System.Xml.XmlElement? configurationElement, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionEvents! events, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionSink! dataSink, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionLogger! logger, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionEnvironmentContext? environmentContext) -> void +Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.GetDumpFiles(bool processCrashed, System.DateTime? testSessionEndTimestamp) -> System.Collections.Generic.IEnumerable! +Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed, System.DateTime? testSessionEndedTimestamp) -> System.Collections.Generic.IEnumerable! +Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.GetDumpFiles(bool processCrashed, System.DateTime? testSessionEndedTimestamp) -> System.Collections.Generic.IEnumerable! diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index 3e7b6d8003..678900d35e 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -175,7 +175,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { dumpFile }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, It.IsAny())).Returns(new[] { dumpFile }); _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()); _blameDataCollector.Initialize( @@ -187,7 +187,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); } @@ -210,7 +210,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, DateTime.Now)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); _blameDataCollector.Initialize( GetDumpConfigurationElement(false, false, true, 0), @@ -221,7 +221,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); } /// @@ -244,7 +244,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { dumpFile }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, It.IsAny())).Returns(new[] { dumpFile }); _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception")); _blameDataCollector.Initialize( @@ -256,7 +256,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); } @@ -371,7 +371,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled() _context); // Setup - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny(), It.IsAny())).Returns(new[] { _filepath }); + _ = _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny(), It.IsAny(), It.IsAny())).Returns(new[] { _filepath }); _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); @@ -381,7 +381,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled() _mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(_dataCollectionContext)); // Verify GetDumpFiles Call - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny(), It.IsAny()), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); } /// @@ -423,7 +423,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab _context); // Setup - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { _filepath }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, It.IsAny())).Returns(new[] { _filepath }); _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); @@ -432,7 +432,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab _mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(_dataCollectionContext)); // Verify GetDumpFiles Call - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); } /// @@ -454,7 +454,7 @@ public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNot // Setup and raise events _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny())).Throws(new FileNotFoundException()); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny(), It.IsAny())).Throws(new FileNotFoundException()); _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); _mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(new TestCase())); _mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(_dataCollectionContext)); diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs index 629b94f369..edce104636 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. +using System; using System.IO; using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces; @@ -62,7 +63,7 @@ public void GetDumpFileWillThrowExceptionIfNoDumpfile() processDumpUtility.StartTriggerBasedProcessDump(processId, testResultsDirectory, false, ".NETCoreApp,Version=v5.0", false, _ => { }); - var ex = Assert.ThrowsException(() => processDumpUtility.GetDumpFiles(true, false)); + var ex = Assert.ThrowsException(() => processDumpUtility.GetDumpFiles(true, false, DateTime.Now)); Assert.AreEqual(ex.Message, Resources.Resources.DumpFileNotGeneratedErrorMessage); } } From 7f897ada7445a3aa26717bd189dde6ede81bbeff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Tue, 11 Apr 2023 13:36:10 +0200 Subject: [PATCH 03/10] Better logs --- .../ProcDumpDumper.cs | 42 ++++++++++++++----- 1 file changed, 31 insertions(+), 11 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs index c49b0feb4b..9d71211eb6 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs @@ -258,18 +258,38 @@ public IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessi // When we know there was a crash dump collected, either because we detected it from the procdump output, or because // we got that info from exit code, don't try to remove the extra crash dump that we generate on process exit. - var crashDumpDetected = _crashDumpTimestamps.Count == 0 + + bool crashDumpDetected = false; + if (_crashDumpTimestamps.Count == 0) + { // When there are 0 crashdumps detected, we know the detection was false. - ? false - : _crashDumpTimestamps.Count > 1 - // When there are more than 1 crashdumps detected, we know it was true, and want to keep all dumps. - ? true - : testSessionEndedTimestamp == null - // When test session did not end and we have 1 crash dump it was a crash for sure. - ? true - // When session ended, and we created a dump within 100ms around it, it was most likely an - // on process exit dump. - : Math.Abs((_crashDumpTimestamps.Last() - (DateTime)testSessionEndedTimestamp!).TotalMilliseconds) > 100; + EqtTrace.Verbose($"There were 0 {nameof(_crashDumpTimestamps)} crash dump was not detected."); + crashDumpDetected = false; + } + else if (_crashDumpTimestamps.Count > 1) + { + // When there are more than 1 crashdumps detected, we know it was true, and want to keep all dumps. + EqtTrace.Verbose($"There were more than 1 {nameof(_crashDumpTimestamps)} ({_crashDumpTimestamps.Count}), keeping all crashdumps."); + crashDumpDetected = true; + } + // From here downwards we know that we have 1 crash dump. + else if (testSessionEndedTimestamp == null) + { + EqtTrace.Verbose($"There was 1 {nameof(_crashDumpTimestamps)} and the test session did not end, keeping all crashdumps."); + // When test session did not end and we have 1 crash dump it was a crash for sure. + crashDumpDetected = true; + } + else + { + EqtTrace.Verbose($"There was 1 {nameof(_crashDumpTimestamps)} and the test session did end. Comparing timestamps to see if we should keep the crashdump."); + // When session ended, and we created a dump within 100ms around it, it was most likely an + // on process exit dump, and we don't need to keep that, unless the user overrides it (e.g. by specifying CollectAlways). + var timestampDiff = Math.Abs((_crashDumpTimestamps.Last() - (DateTime)testSessionEndedTimestamp!).TotalMilliseconds); + var isCrashDump = timestampDiff > 100; + EqtTrace.Verbose($"{nameof(_crashDumpTimestamps)}: {_crashDumpTimestamps.Last().ToString("o", CultureInfo.InvariantCulture)}, {nameof(testSessionEndedTimestamp)}: {((DateTime)testSessionEndedTimestamp!).ToString("o", CultureInfo.InvariantCulture)}. The difference between the events is {timestampDiff} ms."); + crashDumpDetected = isCrashDump; + } + if (crashDumpDetected || processCrashed) { return allDumps; From 442ba9308998c55a09b35ae56d922e2eb53ae6d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Tue, 11 Apr 2023 17:11:50 +0200 Subject: [PATCH 04/10] Review feedback --- .../ProcDumpDumper.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs index 9d71211eb6..8b24232e36 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs @@ -175,7 +175,6 @@ public void DetachFromTargetProcess(int targetProcessId) return; } - // Process? procDumpCancelProcess = null; try { if (_isCrashDumpInProgress) @@ -284,9 +283,10 @@ public IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessi EqtTrace.Verbose($"There was 1 {nameof(_crashDumpTimestamps)} and the test session did end. Comparing timestamps to see if we should keep the crashdump."); // When session ended, and we created a dump within 100ms around it, it was most likely an // on process exit dump, and we don't need to keep that, unless the user overrides it (e.g. by specifying CollectAlways). - var timestampDiff = Math.Abs((_crashDumpTimestamps.Last() - (DateTime)testSessionEndedTimestamp!).TotalMilliseconds); + var crashDumpTimestamp = _crashDumpTimestamps.Last(); + var timestampDiff = Math.Abs((crashDumpTimestamp - (DateTime)testSessionEndedTimestamp!).TotalMilliseconds); var isCrashDump = timestampDiff > 100; - EqtTrace.Verbose($"{nameof(_crashDumpTimestamps)}: {_crashDumpTimestamps.Last().ToString("o", CultureInfo.InvariantCulture)}, {nameof(testSessionEndedTimestamp)}: {((DateTime)testSessionEndedTimestamp!).ToString("o", CultureInfo.InvariantCulture)}. The difference between the events is {timestampDiff} ms."); + EqtTrace.Verbose($"{nameof(_crashDumpTimestamps)}: {crashDumpTimestamp.ToString("o", CultureInfo.InvariantCulture)}, {nameof(testSessionEndedTimestamp)}: {((DateTime)testSessionEndedTimestamp!).ToString("o", CultureInfo.InvariantCulture)}. The difference between the events is {timestampDiff} ms."); crashDumpDetected = isCrashDump; } From b7e5125029ea667d8d87c3ee33b397ac4ff79984 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 12 Apr 2023 09:29:35 +0200 Subject: [PATCH 05/10] Update src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jakub Jareš --- .../BlameCollector.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index 53b9a2d1a0..2466b4f3b1 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs @@ -246,7 +246,7 @@ private void CollectDumpAndAbortTesthost() if (_collectProcessDumpOnCrash) { - // Detach the dumper from the testhost process to prevent testhost process. When the dumper is procdump.exe + // Detach the dumper from the testhost process to prevent crashing testhost process. When the dumper is procdump.exe // it must be detached before we try to dump the process, and simply killing it would take down the testhost process. // // Detaching also prevents creating an extra dump at the exit of the testhost process. From 6a518bd58dca4100cb7659bd98ea76d4680421ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 20 Apr 2023 12:01:49 +0200 Subject: [PATCH 06/10] Fix --- .../BlameCollector.cs | 9 +--- .../ICrashDumper.cs | 2 +- .../Interfaces/IProcessDumpUtility.cs | 2 +- .../NetClientCrashDumper.cs | 2 +- .../ProcDumpDumper.cs | 42 +------------------ .../ProcessDumpUtility.cs | 4 +- .../PublicAPI/PublicAPI.Shipped.txt | 6 +-- .../BlameDataCollectorTests.cs | 2 +- .../BlameCollectorTests.cs | 22 +++++----- test/TestAssets/crash/UnitTest1.cs | 14 +++++++ 10 files changed, 38 insertions(+), 67 deletions(-) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs index 53b9a2d1a0..d314de9f9c 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/BlameCollector.cs @@ -66,7 +66,6 @@ public class BlameCollector : DataCollector, ITestExecutionEnvironmentSpecifier private bool _uploadDumpFiles; private string? _tempDirectory; private string? _monitorPostmortemDumpFolderPath; - private DateTime? _testSessionEndedTimestamp; /// /// Initializes a new instance of the class. @@ -256,13 +255,11 @@ private void CollectDumpAndAbortTesthost() // Skip creating the dump if the option is set to none, and just kill the process. if ((_hangDumpType ?? HangDumpType.Full) != HangDumpType.None) { - var hangDumpSuccess = false; try { Action logWarning = m => _logger.LogWarning(_context.SessionDataCollectionContext, m); var dumpDirectory = GetDumpDirectory(); _processDumpUtility.StartHangBasedProcessDump(_testHostProcessId, dumpDirectory, _hangDumpType == HangDumpType.Full, _targetFramework!, logWarning); - hangDumpSuccess = true; } catch (Exception ex) { @@ -274,8 +271,7 @@ private void CollectDumpAndAbortTesthost() try { var dumpFiles = _processDumpUtility.GetDumpFiles(true, - /* if we killed it by hang dumper, we already have our dump, otherwise it might have crashed, and we want all dumps */ !hangDumpSuccess, - _testSessionEndedTimestamp); + true /* Get all dumps that there are, if crashdump was created before we hangdumped, get it. It probably has interesting info. */); foreach (var dumpFile in dumpFiles) { try @@ -503,7 +499,6 @@ private void EventsTestCaseEnd(object? sender, TestCaseEndEventArgs e) /// SessionEndEventArgs private void SessionEndedHandler(object? sender, SessionEndEventArgs args) { - _testSessionEndedTimestamp = DateTime.Now; TPDebug.Assert(_testSequence != null && _testObjectDictionary != null && _context != null && _dataCollectionSink != null && _logger != null, "Initialize must be called before calling this method"); ResetInactivityTimer(); @@ -535,7 +530,7 @@ private void SessionEndedHandler(object? sender, SessionEndEventArgs args) { try { - var dumpFiles = _processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: _collectDumpAlways, processCrashedWhenRunningTests, _testSessionEndedTimestamp); + var dumpFiles = _processDumpUtility.GetDumpFiles(warnOnNoDumpFiles: _collectDumpAlways, processCrashedWhenRunningTests); foreach (var dumpFile in dumpFiles) { if (!dumpFile.IsNullOrEmpty()) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs index 86a1a86b64..6f74ed7edc 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ICrashDumper.cs @@ -14,5 +14,5 @@ public interface ICrashDumper void DetachFromTargetProcess(int processId); - IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessionEndTimestamp); + IEnumerable GetDumpFiles(bool processCrashed); } diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs index fd0d8a59ed..a5a559197d 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/Interfaces/IProcessDumpUtility.cs @@ -16,7 +16,7 @@ public interface IProcessDumpUtility /// /// Path of dump file /// - IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed, DateTime? testSessionEndedTimestamp); + IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed); /// /// Launch proc dump process diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs index a8ce171337..333a123a21 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/NetClientCrashDumper.cs @@ -31,7 +31,7 @@ public void DetachFromTargetProcess(int processId) // here we might consider renaming the files to have timestamp } - public IEnumerable GetDumpFiles(bool _, DateTime? _2) + public IEnumerable GetDumpFiles(bool _) { return _fileHelper.DirectoryExists(_outputDirectory) ? _fileHelper.GetFiles(_outputDirectory, "*_crashdump*.dmp", SearchOption.AllDirectories) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs index 8b24232e36..8aef1bdd46 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcDumpDumper.cs @@ -42,7 +42,6 @@ public class ProcDumpDumper : ICrashDumper, IHangDumper private Process? _process; private string? _outputFilePrefix; private bool _isCrashDumpInProgress; - private readonly List _crashDumpTimestamps = new(); private readonly int _timeout = EnvironmentHelper.GetConnectionTimeout() * 1000; private readonly ProcDumpExecutableHelper _procDumpExecutableHelper; @@ -93,8 +92,6 @@ internal ProcDumpDumper(IProcessHelper processHelper, IFileHelper fileHelper, IE { EqtTrace.Info($"ProcDumpDumper.OutputReceivedCallback: Output received from procdump process contains 'complete' dump is finished, you can cancel procdump if you need."); _isCrashDumpInProgress = false; - // Do not reset _crashDumpDetected back to false here. Any detected crash dumping should keep it true, so we don't throw away the dump. - _crashDumpTimestamps.Add(DateTime.Now); } }; @@ -239,7 +236,7 @@ public void DetachFromTargetProcess(int targetProcessId) } } - public IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessionEndedTimestamp) + public IEnumerable GetDumpFiles(bool processCrashed) { var allDumps = _fileHelper.DirectoryExists(_outputDirectory) ? _fileHelper.GetFiles(_outputDirectory, "*_crashdump*.dmp", SearchOption.AllDirectories) @@ -255,42 +252,7 @@ public IEnumerable GetDumpFiles(bool processCrashed, DateTime? testSessi return allDumps; } - // When we know there was a crash dump collected, either because we detected it from the procdump output, or because - // we got that info from exit code, don't try to remove the extra crash dump that we generate on process exit. - - bool crashDumpDetected = false; - if (_crashDumpTimestamps.Count == 0) - { - // When there are 0 crashdumps detected, we know the detection was false. - EqtTrace.Verbose($"There were 0 {nameof(_crashDumpTimestamps)} crash dump was not detected."); - crashDumpDetected = false; - } - else if (_crashDumpTimestamps.Count > 1) - { - // When there are more than 1 crashdumps detected, we know it was true, and want to keep all dumps. - EqtTrace.Verbose($"There were more than 1 {nameof(_crashDumpTimestamps)} ({_crashDumpTimestamps.Count}), keeping all crashdumps."); - crashDumpDetected = true; - } - // From here downwards we know that we have 1 crash dump. - else if (testSessionEndedTimestamp == null) - { - EqtTrace.Verbose($"There was 1 {nameof(_crashDumpTimestamps)} and the test session did not end, keeping all crashdumps."); - // When test session did not end and we have 1 crash dump it was a crash for sure. - crashDumpDetected = true; - } - else - { - EqtTrace.Verbose($"There was 1 {nameof(_crashDumpTimestamps)} and the test session did end. Comparing timestamps to see if we should keep the crashdump."); - // When session ended, and we created a dump within 100ms around it, it was most likely an - // on process exit dump, and we don't need to keep that, unless the user overrides it (e.g. by specifying CollectAlways). - var crashDumpTimestamp = _crashDumpTimestamps.Last(); - var timestampDiff = Math.Abs((crashDumpTimestamp - (DateTime)testSessionEndedTimestamp!).TotalMilliseconds); - var isCrashDump = timestampDiff > 100; - EqtTrace.Verbose($"{nameof(_crashDumpTimestamps)}: {crashDumpTimestamp.ToString("o", CultureInfo.InvariantCulture)}, {nameof(testSessionEndedTimestamp)}: {((DateTime)testSessionEndedTimestamp!).ToString("o", CultureInfo.InvariantCulture)}. The difference between the events is {timestampDiff} ms."); - crashDumpDetected = isCrashDump; - } - - if (crashDumpDetected || processCrashed) + if (processCrashed) { return allDumps; } diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs index 003895d619..fc42574303 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/ProcessDumpUtility.cs @@ -46,7 +46,7 @@ public ProcessDumpUtility(IProcessHelper processHelper, IFileHelper fileHelper, EqtTrace.Info($"ProcessDumpUtility.OutputReceivedCallback: Output received from procdump process: {data ?? ""}"); /// - public IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed, DateTime? testSessionEndedTimestamp) + public IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed) { if (!_wasHangDumped) { @@ -56,7 +56,7 @@ public IEnumerable GetDumpFiles(bool warnOnNoDumpFiles, bool processCras // If the process was hang dumped we killed it ourselves, so it crashed when executing tests, // but we already have the hang dump, and should not also collect the exit dump that we got // from killing the process by the hang dumper. - IEnumerable crashDumps = _crashDumper?.GetDumpFiles(processCrashed, testSessionEndedTimestamp) ?? new List(); + IEnumerable crashDumps = _crashDumper?.GetDumpFiles(processCrashed) ?? new List(); IEnumerable hangDumps = _fileHelper.DirectoryExists(_hangDumpDirectory) ? _fileHelper.GetFiles(_hangDumpDirectory, "*_hangdump*.dmp", SearchOption.TopDirectoryOnly) diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt index 9e54bdcff4..ff0c2cb5bc 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt @@ -78,6 +78,6 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.ReadTestSeq Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.WriteTestSequence(System.Collections.Generic.List! testSequence, System.Collections.Generic.Dictionary! testObjectDictionary, string! filePath) -> string! Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.XmlReaderWriter(Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper! fileHelper) -> void override Microsoft.TestPlatform.Extensions.BlameDataCollector.BlameCollector.Initialize(System.Xml.XmlElement? configurationElement, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionEvents! events, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionSink! dataSink, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionLogger! logger, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionEnvironmentContext? environmentContext) -> void -Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.GetDumpFiles(bool processCrashed, System.DateTime? testSessionEndTimestamp) -> System.Collections.Generic.IEnumerable! -Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed, System.DateTime? testSessionEndedTimestamp) -> System.Collections.Generic.IEnumerable! -Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.GetDumpFiles(bool processCrashed, System.DateTime? testSessionEndedTimestamp) -> System.Collections.Generic.IEnumerable! +Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! +Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed) -> System.Collections.Generic.IEnumerable! +Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs index 40636532b9..53d46834a0 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs @@ -209,7 +209,7 @@ public void CrashDumpOnStackOverflow(RunnerInfo runnerInfo) var assemblyPaths = GetAssetFullPath("crash.dll"); var arguments = PrepareArguments(assemblyPaths, GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue); arguments = string.Concat(arguments, $" /ResultsDirectory:{TempDirectory.Path}"); - arguments = string.Concat(arguments, $@" /Blame:""CollectDump;DumpType=full"""); + arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;TestTimeout=10s;CollectDump;DumpType=full"" /diag:S:\t\logs\l1\"); var env = new Dictionary { diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index 678900d35e..e0edc1b8b3 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -175,7 +175,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, It.IsAny())).Returns(new[] { dumpFile }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { dumpFile }); _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()); _blameDataCollector.Initialize( @@ -187,7 +187,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); } @@ -210,7 +210,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, DateTime.Now)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); _blameDataCollector.Initialize( GetDumpConfigurationElement(false, false, true, 0), @@ -221,7 +221,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); } /// @@ -244,7 +244,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, It.IsAny())).Returns(new[] { dumpFile }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { dumpFile }); _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception")); _blameDataCollector.Initialize( @@ -256,7 +256,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); } @@ -371,7 +371,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled() _context); // Setup - _ = _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny(), It.IsAny(), It.IsAny())).Returns(new[] { _filepath }); + _ = _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny(), It.IsAny())).Returns(new[] { _filepath }); _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); @@ -381,7 +381,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled() _mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(_dataCollectionContext)); // Verify GetDumpFiles Call - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny(), It.IsAny(), It.IsAny()), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(It.IsAny(), It.IsAny()), Times.Once); } /// @@ -423,7 +423,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab _context); // Setup - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false, It.IsAny())).Returns(new[] { _filepath }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { _filepath }); _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); @@ -432,7 +432,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfCollectDumpOnExitIsEnab _mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(_dataCollectionContext)); // Verify GetDumpFiles Call - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false, It.IsAny()), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); } /// @@ -454,7 +454,7 @@ public void TriggerSessionEndedHandlerShouldLogWarningIfGetDumpFileThrowsFileNot // Setup and raise events _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny(), It.IsAny())).Throws(new FileNotFoundException()); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny())).Throws(new FileNotFoundException()); _mockDataColectionEvents.Raise(x => x.TestHostLaunched += null, new TestHostLaunchedEventArgs(_dataCollectionContext, 1234)); _mockDataColectionEvents.Raise(x => x.TestCaseStart += null, new TestCaseStartEventArgs(new TestCase())); _mockDataColectionEvents.Raise(x => x.SessionEnd += null, new SessionEndEventArgs(_dataCollectionContext)); diff --git a/test/TestAssets/crash/UnitTest1.cs b/test/TestAssets/crash/UnitTest1.cs index cfcb1181c3..92b391319b 100644 --- a/test/TestAssets/crash/UnitTest1.cs +++ b/test/TestAssets/crash/UnitTest1.cs @@ -4,6 +4,8 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using System; +using System.Collections.Generic; +using System.Linq; #pragma warning disable IDE1006 // Naming Styles namespace timeout @@ -15,8 +17,20 @@ public class UnitTest1 [TestMethod] public void TestMethod1() { + var list = new List(); + list.AddRange(Enumerable.Range(0, 100000).Select(a => new a())); // stack overflow Span s = stackalloc byte[int.MaxValue]; } } + + public class a + { + public static Random random = new Random(); + public a() + { + abc = new string(((char)(byte)random.Next(0, 255)), random.Next(10000, 100_000)); + } + public string abc; + } } From 78d856b30da58c3a4c147d7c3a8752e74bfa38e2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 20 Apr 2023 12:33:31 +0200 Subject: [PATCH 07/10] Fix --- .../Resources/Resources.Designer.cs | 2 +- .../BlameDataCollectorTests.cs | 2 +- .../ProcessDumpUtilityTests.cs | 3 +-- 3 files changed, 3 insertions(+), 4 deletions(-) diff --git a/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs b/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs index 689e046619..f50f5a5142 100644 --- a/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs +++ b/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs @@ -21,7 +21,7 @@ namespace Microsoft.TestPlatform.Build.Resources { // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "15.0.0.0")] - [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] + [global::System.Diagnostics. DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs index 53d46834a0..ad21373cb0 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs @@ -209,7 +209,7 @@ public void CrashDumpOnStackOverflow(RunnerInfo runnerInfo) var assemblyPaths = GetAssetFullPath("crash.dll"); var arguments = PrepareArguments(assemblyPaths, GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue); arguments = string.Concat(arguments, $" /ResultsDirectory:{TempDirectory.Path}"); - arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;TestTimeout=10s;CollectDump;DumpType=full"" /diag:S:\t\logs\l1\"); + arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;TestTimeout=15s;CollectDump;DumpType=full"" /diag:S:\t\logs\l1\"); var env = new Dictionary { diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs index edce104636..629b94f369 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/ProcessDumpUtilityTests.cs @@ -1,7 +1,6 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT license. See LICENSE file in the project root for full license information. -using System; using System.IO; using Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces; @@ -63,7 +62,7 @@ public void GetDumpFileWillThrowExceptionIfNoDumpfile() processDumpUtility.StartTriggerBasedProcessDump(processId, testResultsDirectory, false, ".NETCoreApp,Version=v5.0", false, _ => { }); - var ex = Assert.ThrowsException(() => processDumpUtility.GetDumpFiles(true, false, DateTime.Now)); + var ex = Assert.ThrowsException(() => processDumpUtility.GetDumpFiles(true, false)); Assert.AreEqual(ex.Message, Resources.Resources.DumpFileNotGeneratedErrorMessage); } } From e123d0ddca056a4c61cd924e2b5782f08b80651f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 20 Apr 2023 12:37:17 +0200 Subject: [PATCH 08/10] Revert --- .../Resources/Resources.Designer.cs | 2 +- .../PublicAPI/PublicAPI.Shipped.txt | 6 +++--- .../BlameDataCollectorTests.cs | 2 +- .../BlameCollectorTests.cs | 2 +- test/TestAssets/crash/UnitTest1.cs | 14 -------------- 5 files changed, 6 insertions(+), 20 deletions(-) diff --git a/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs b/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs index f50f5a5142..689e046619 100644 --- a/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs +++ b/src/Microsoft.TestPlatform.Build/Resources/Resources.Designer.cs @@ -21,7 +21,7 @@ namespace Microsoft.TestPlatform.Build.Resources { // To add or remove a member, edit your .ResX file then rerun ResGen // with the /str option, or rebuild your VS project. [global::System.CodeDom.Compiler.GeneratedCodeAttribute("System.Resources.Tools.StronglyTypedResourceBuilder", "15.0.0.0")] - [global::System.Diagnostics. DebuggerNonUserCodeAttribute()] + [global::System.Diagnostics.DebuggerNonUserCodeAttribute()] [global::System.Runtime.CompilerServices.CompilerGeneratedAttribute()] internal class Resources { diff --git a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt index ff0c2cb5bc..a5ac44b058 100644 --- a/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt +++ b/src/Microsoft.TestPlatform.Extensions.BlameDataCollector/PublicAPI/PublicAPI.Shipped.txt @@ -34,6 +34,7 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.IBlameReaderWriter.WriteTes Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.AttachToTargetProcess(int processId, string! outputDirectory, Microsoft.TestPlatform.Extensions.BlameDataCollector.DumpTypeOption dumpType, bool collectAlways, System.Action! logWarning) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.DetachFromTargetProcess(int processId) -> void +Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.WaitForDumpToFinish() -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumperFactory Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumperFactory.Create(string! targetFramework) -> Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper! @@ -56,6 +57,7 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcDumpArgsBuilder.BuildH Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcDumpArgsBuilder.BuildTriggerBasedProcDumpArgs(int processId, string! filename, System.Collections.Generic.IEnumerable! procDumpExceptionsList, bool isFullDump) -> string! Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.DetachFromTargetProcess(int targetProcessId) -> void +Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed) -> System.Collections.Generic.IEnumerable! Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.StartHangBasedProcessDump(int processId, string! testResultsDirectory, bool isFullDump, string! targetFramework, System.Action? logWarning = null) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.StartTriggerBasedProcessDump(int processId, string! testResultsDirectory, bool isFullDump, string! targetFramework, bool collectAlways, System.Action! logWarning) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpArgsBuilder @@ -66,6 +68,7 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.AttachToTargetProcess(int processId, string! outputDirectory, Microsoft.TestPlatform.Extensions.BlameDataCollector.DumpTypeOption dumpType, bool collectAlways, System.Action! logWarning) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.DetachFromTargetProcess(int targetProcessId) -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.Dump(int processId, string! outputDirectory, Microsoft.TestPlatform.Extensions.BlameDataCollector.DumpTypeOption dumpType) -> void +Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.OutputReceivedCallback.get -> System.Action! Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.ProcDumpDumper() -> void Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.ProcDumpDumper(Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces.IProcessHelper! processHelper, Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper! fileHelper, Microsoft.VisualStudio.TestPlatform.PlatformAbstractions.Interfaces.IEnvironment! environment) -> void @@ -78,6 +81,3 @@ Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.ReadTestSeq Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.WriteTestSequence(System.Collections.Generic.List! testSequence, System.Collections.Generic.Dictionary! testObjectDictionary, string! filePath) -> string! Microsoft.TestPlatform.Extensions.BlameDataCollector.XmlReaderWriter.XmlReaderWriter(Microsoft.VisualStudio.TestPlatform.Utilities.Helpers.Interfaces.IFileHelper! fileHelper) -> void override Microsoft.TestPlatform.Extensions.BlameDataCollector.BlameCollector.Initialize(System.Xml.XmlElement? configurationElement, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionEvents! events, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionSink! dataSink, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionLogger! logger, Microsoft.VisualStudio.TestPlatform.ObjectModel.DataCollection.DataCollectionEnvironmentContext? environmentContext) -> void -Microsoft.TestPlatform.Extensions.BlameDataCollector.ICrashDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! -Microsoft.TestPlatform.Extensions.BlameDataCollector.IProcessDumpUtility.GetDumpFiles(bool warnOnNoDumpFiles, bool processCrashed) -> System.Collections.Generic.IEnumerable! -Microsoft.TestPlatform.Extensions.BlameDataCollector.ProcDumpDumper.GetDumpFiles(bool processCrashed) -> System.Collections.Generic.IEnumerable! diff --git a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs index ad21373cb0..40636532b9 100644 --- a/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Acceptance.IntegrationTests/BlameDataCollectorTests.cs @@ -209,7 +209,7 @@ public void CrashDumpOnStackOverflow(RunnerInfo runnerInfo) var assemblyPaths = GetAssetFullPath("crash.dll"); var arguments = PrepareArguments(assemblyPaths, GetTestAdapterPath(), string.Empty, string.Empty, runnerInfo.InIsolationValue); arguments = string.Concat(arguments, $" /ResultsDirectory:{TempDirectory.Path}"); - arguments = string.Concat(arguments, $@" /Blame:""CollectHangDump;TestTimeout=15s;CollectDump;DumpType=full"" /diag:S:\t\logs\l1\"); + arguments = string.Concat(arguments, $@" /Blame:""CollectDump;DumpType=full"""); var env = new Dictionary { diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index e0edc1b8b3..3e7b6d8003 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -371,7 +371,7 @@ public void TriggerSessionEndedHandlerShouldGetDumpFileIfProcDumpEnabled() _context); // Setup - _ = _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny(), It.IsAny())).Returns(new[] { _filepath }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(It.IsAny(), It.IsAny())).Returns(new[] { _filepath }); _mockBlameReaderWriter.Setup(x => x.WriteTestSequence(It.IsAny>(), It.IsAny>(), It.IsAny())) .Returns(_filepath); diff --git a/test/TestAssets/crash/UnitTest1.cs b/test/TestAssets/crash/UnitTest1.cs index 92b391319b..cfcb1181c3 100644 --- a/test/TestAssets/crash/UnitTest1.cs +++ b/test/TestAssets/crash/UnitTest1.cs @@ -4,8 +4,6 @@ using Microsoft.VisualStudio.TestTools.UnitTesting; using System; -using System.Collections.Generic; -using System.Linq; #pragma warning disable IDE1006 // Naming Styles namespace timeout @@ -17,20 +15,8 @@ public class UnitTest1 [TestMethod] public void TestMethod1() { - var list = new List(); - list.AddRange(Enumerable.Range(0, 100000).Select(a => new a())); // stack overflow Span s = stackalloc byte[int.MaxValue]; } } - - public class a - { - public static Random random = new Random(); - public a() - { - abc = new string(((char)(byte)random.Next(0, 255)), random.Next(10000, 100_000)); - } - public string abc; - } } From 076c33644b4b6641da576a1d3a6decec1775d43e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 20 Apr 2023 14:08:23 +0200 Subject: [PATCH 09/10] Fix test --- .../BlameCollectorTests.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index 3e7b6d8003..e165fb06fc 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -175,7 +175,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { dumpFile }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny())).Returns(new[] { dumpFile }); _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()); _blameDataCollector.Initialize( @@ -187,7 +187,7 @@ public void InitializeWithDumpForHangShouldCaptureADumpOnTimeout() hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny()), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); } From a76cb1aa568946666138faf589fdc44f0f38c262 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20Jare=C5=A1?= Date: Thu, 20 Apr 2023 17:32:15 +0200 Subject: [PATCH 10/10] Fix tests --- .../BlameCollectorTests.cs | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs index e165fb06fc..753b705c13 100644 --- a/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs +++ b/test/Microsoft.TestPlatform.Extensions.BlameDataCollector.UnitTests/BlameCollectorTests.cs @@ -210,7 +210,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some exception")); _blameDataCollector.Initialize( GetDumpConfigurationElement(false, false, true, 0), @@ -221,7 +221,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfGet hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny()), Times.Once); } /// @@ -244,7 +244,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt _mockFileHelper.Setup(x => x.Exists(It.Is(y => y == "abc_hang.dmp"))).Returns(true); _mockFileHelper.Setup(x => x.GetFullPath(It.Is(y => y == "abc_hang.dmp"))).Returns("abc_hang.dmp"); _mockProcessDumpUtility.Setup(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>())); - _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, false)).Returns(new[] { dumpFile }); + _mockProcessDumpUtility.Setup(x => x.GetDumpFiles(true, It.IsAny())).Returns(new[] { dumpFile }); _mockDataCollectionSink.Setup(x => x.SendFileAsync(It.IsAny())).Callback(() => hangBasedDumpcollected.Set()).Throws(new Exception("Some other exception")); _blameDataCollector.Initialize( @@ -256,7 +256,7 @@ public void InitializeWithDumpForHangShouldCaptureKillTestHostOnTimeoutEvenIfAtt hangBasedDumpcollected.Wait(1000); _mockProcessDumpUtility.Verify(x => x.StartHangBasedProcessDump(It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny(), It.IsAny>()), Times.Once); - _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, false), Times.Once); + _mockProcessDumpUtility.Verify(x => x.GetDumpFiles(true, It.IsAny()), Times.Once); _mockDataCollectionSink.Verify(x => x.SendFileAsync(It.Is(y => y.Path == dumpFile)), Times.Once); }