From 20db37211b8dcfa79ef47fd09d59b8a60422c4ac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 15 May 2026 18:44:55 +0200 Subject: [PATCH 1/4] Collect all crash dumps in dump directory (#4186) When the testhost crashes alongside one or more child processes, the .NET runtime writes a separate dump for each crashing process using the configured dump file name pattern. Previously the crash-dump extension only reported the testhost's own dump (matched by PID), losing the child-process dumps. Convert the dump file name pattern to a wildcard search pattern by replacing every '%X' placeholder with '*' and enumerate every matching file in the dump directory, publishing each one as a FileArtifact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CrashDumpProcessLifetimeHandler.cs | 57 ++++++++++++++++--- .../CrashDumpTests.cs | 13 +++++ 2 files changed, 62 insertions(+), 8 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs index dce2b1b5da..c0ee97c904 100644 --- a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs +++ b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs @@ -65,21 +65,62 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH ApplicationStateGuard.Ensure(_netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern is not null); await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CrashDumpProcessCrashedDumpFileCreated, testHostProcessInformation.PID)), cancellationToken).ConfigureAwait(false); - // TODO: Crash dump supports more placeholders that we don't handle here. + // The crash dump file name pattern can contain placeholders such as %p (PID), %e (process exe name), + // %h (hostname), %t (timestamp), etc. that are expanded by the .NET runtime when it writes the dump. // See "Dump name formatting" in: - // https://github.com/dotnet/runtime/blob/82742628310076fff22d7e7ee216a74384352056/docs/design/coreclr/botr/xplat-minidump-generation.md - string expectedDumpFile = _netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern.Replace("%p", testHostProcessInformation.PID.ToString(CultureInfo.InvariantCulture)); - if (File.Exists(expectedDumpFile)) + // https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/xplat-minidump-generation.md + // We replace every placeholder with a wildcard so we can collect not just the testhost dump but also + // dumps produced by any of its child processes that may have crashed alongside it. + string dumpFileNamePattern = _netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern; + string? dumpDirectory = Path.GetDirectoryName(dumpFileNamePattern); + string searchPattern = ReplaceCrashDumpPlaceholdersWithWildcard(Path.GetFileName(dumpFileNamePattern)); + + bool publishedAny = false; + if (dumpDirectory is not null && Directory.Exists(dumpDirectory)) { - await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(expectedDumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory, searchPattern)) + { + await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + publishedAny = true; + } } - else + + if (!publishedAny) { + string expectedDumpFile = dumpFileNamePattern.Replace("%p", testHostProcessInformation.PID.ToString(CultureInfo.InvariantCulture)); await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CannotFindExpectedCrashDumpFile, expectedDumpFile)), cancellationToken).ConfigureAwait(false); - foreach (string dumpFile in Directory.GetFiles(Path.GetDirectoryName(expectedDumpFile)!, "*.dmp")) + if (dumpDirectory is not null && Directory.Exists(dumpDirectory)) { - await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory, "*.dmp")) + { + await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + } } } } + + internal static string ReplaceCrashDumpPlaceholdersWithWildcard(string fileName) + { + var sb = new StringBuilder(fileName.Length); + for (int i = 0; i < fileName.Length; i++) + { + if (fileName[i] == '%' && i + 1 < fileName.Length) + { + // Replace any %X placeholder with '*'. Collapse consecutive wildcards to keep the search + // pattern minimal and avoid confusing search engines with redundant '**' sequences. + if (sb.Length == 0 || sb[sb.Length - 1] != '*') + { + sb.Append('*'); + } + + i++; + } + else + { + sb.Append(fileName[i]); + } + } + + return sb.ToString(); + } } diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs index 4f2b1640f2..52af939c2f 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs @@ -46,4 +46,17 @@ public async Task CrashDump_CommandLineOptions_Are_AlwaysValid() Assert.IsTrue(validateOptionsResult.IsValid); Assert.IsTrue(string.IsNullOrEmpty(validateOptionsResult.ErrorMessage)); } + + [TestMethod] + [DataRow("MyApp_%p_crash.dmp", "MyApp_*_crash.dmp")] + [DataRow("%e_%p_crash.dmp", "*_*_crash.dmp")] + [DataRow("%p%t_crash.dmp", "*_crash.dmp")] + [DataRow("customdumpname.dmp", "customdumpname.dmp")] + [DataRow("dump_%p_%t_%h.dmp", "dump_*_*_*.dmp")] + [DataRow("trailing%", "trailing%")] + public void ReplaceCrashDumpPlaceholdersWithWildcard_ConvertsPlaceholdersToWildcards(string fileName, string expected) + { + string actual = CrashDumpProcessLifetimeHandler.ReplaceCrashDumpPlaceholdersWithWildcard(fileName); + Assert.AreEqual(expected, actual); + } } From 92eb8613dd9928b8c68a90fb3bfd39bcf0cb0fac Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 15 May 2026 19:45:27 +0200 Subject: [PATCH 2/4] Add acceptance test for collecting all crash dumps (#4186) Adds CrashDump_TesthostAndChildBothCrash_CollectsAllDumps which spawns a child process from the testhost that also crashes via FailFast, then asserts both dumps end up on disk AND are reported as out-of-process file artifacts. The artifact-output assertion is what guards the #4186 fix: prior to the fix, only the testhost's own PID-matching dump was published as an artifact. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CrashDumpTests.cs | 58 +++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs index cf7e844be3..48ee07e510 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs @@ -18,6 +18,35 @@ public async Task CrashDump_DefaultSetting_CreateDump(string tfm) Assert.IsNotNull(dumpFile, $"Dump file not found '{tfm}'\n{testHostResult}'"); } + [DynamicData(nameof(TargetFrameworks.NetForDynamicData), typeof(TargetFrameworks))] + [TestMethod] + public async Task CrashDump_TesthostAndChildBothCrash_CollectsAllDumps(string tfm) + { + string resultDirectory = Path.Combine(AssetFixture.TargetAssetPath, Guid.NewGuid().ToString("N")); + var testHost = TestInfrastructure.TestHost.LocateFrom(AssetFixture.TargetAssetPath, "CrashDump", tfm); + TestHostResult testHostResult = await testHost.ExecuteAsync( + $"--crashdump --results-directory {resultDirectory}", + new Dictionary + { + { "CRASHDUMP_SPAWN_CHILD_THAT_CRASHES", "1" }, + }, + cancellationToken: TestContext.CancellationToken); + testHostResult.AssertExitCodeIs(ExitCode.TestHostProcessExitedNonGracefully); + + // Both the testhost and its child process crash with FailFast and must produce a dump each. + // Without the fix for https://github.com/microsoft/testfx/issues/4186, only the dump matching + // the testhost's PID was reported as an artifact and the child dump was silently dropped. + string[] dumpFiles = Directory.GetFiles(resultDirectory, "CrashDump_*.dmp", SearchOption.AllDirectories); + Assert.HasCount(2, dumpFiles, $"Expected dumps for both the testhost and the child process '{tfm}'.\n{testHostResult}"); + + // Both dumps must also be reported as out-of-process file artifacts so they show up to the user. + testHostResult.AssertOutputContains("Out of process file artifacts produced:"); + foreach (string dumpFile in dumpFiles) + { + testHostResult.AssertOutputContains(Path.GetFileName(dumpFile)); + } + } + [TestMethod] public async Task CrashDump_CustomDumpName_CreateDump() { @@ -81,6 +110,8 @@ public override (string ID, string Name, string Code) GetAssetsToGenerate() => ( #file Program.cs using System; +using System.Diagnostics; +using System.Reflection; using System.Threading; using System.Threading.Tasks; using System.Globalization; @@ -97,6 +128,14 @@ public class Startup { public static async Task Main(string[] args) { + // When invoked as a child process spawned by the testhost, just crash so we produce + // an additional dump in the same directory using the dump env vars inherited from the parent. + if (args.Length > 0 && args[0] == "--child-crash") + { + Environment.FailFast("ChildCrashDump"); + return 0; // unreachable + } + ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_,__) => new DummyTestFramework()); builder.AddCrashDumpProvider(); @@ -125,6 +164,25 @@ public Task CloseTestSessionAsync(CloseTestSessionContex public Task ExecuteRequestAsync(ExecuteRequestContext context) { + // Optionally spawn a child process that also crashes (and produces its own dump) so we can + // exercise the crashdump extension's ability to collect dumps from child processes. + if (Environment.GetEnvironmentVariable("CRASHDUMP_SPAWN_CHILD_THAT_CRASHES") == "1") + { + Process self = Process.GetCurrentProcess(); + string path = self.MainModule!.FileName!; + string argPrefix = path.EndsWith("dotnet") || path.EndsWith("dotnet.exe") + ? $"exec \"{Assembly.GetEntryAssembly()!.Location}\" " + : string.Empty; + + Process child = Process.Start(new ProcessStartInfo(path, $"{argPrefix}--child-crash") + { + UseShellExecute = false, + })!; + + // Wait for the child to fully exit so its crash dump is written before we crash too. + child.WaitForExit(); + } + Environment.FailFast("CrashDump"); context.Complete(); return Task.CompletedTask; From ca61424e410131fe94090e6ba4cf7c50f36e3a18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Sat, 16 May 2026 10:48:26 +0200 Subject: [PATCH 3/4] Use regex matching for dump file name pattern (#4186) Addresses PR review feedback: - Switch from glob pattern (Directory.EnumerateFiles searchPattern) to regex matching so that literal glob metacharacters ('*' or '?') in user-supplied dump file names (legal on Linux/macOS) are matched literally and do not cause unrelated .dmp files to be picked up. - Treat an empty dumpDirectory (returned by Path.GetDirectoryName for a bare filename on .NET Core/5+) as '.' (current working directory) so enumeration is not silently skipped when the configured pattern has no directory prefix. - Replace ReplaceCrashDumpPlaceholdersWithWildcard with BuildDumpFileNameRegex / BuildDumpFileNameRegexPattern, add GetDumpDirectory helper, and update unit tests to cover literal '*'/'?' in the file name and the no-directory-component case. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CrashDumpProcessLifetimeHandler.cs | 55 +++++++++++++------ .../CrashDumpTests.cs | 55 ++++++++++++++++--- 2 files changed, 86 insertions(+), 24 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs index c0ee97c904..e6afc439c0 100644 --- a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs +++ b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs @@ -69,19 +69,27 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH // %h (hostname), %t (timestamp), etc. that are expanded by the .NET runtime when it writes the dump. // See "Dump name formatting" in: // https://github.com/dotnet/runtime/blob/main/docs/design/coreclr/botr/xplat-minidump-generation.md - // We replace every placeholder with a wildcard so we can collect not just the testhost dump but also - // dumps produced by any of its child processes that may have crashed alongside it. + // We convert the file name part of the pattern into a regular expression (escaping literal characters + // and turning %X placeholders into '.*') so we can collect not just the testhost dump but also dumps + // produced by any of its child processes that may have crashed alongside it. Using a regex (instead + // of passing the pattern as a glob to Directory.EnumerateFiles) ensures that any literal glob + // metacharacter (e.g. '*' or '?') in the configured file name is matched literally and not as a + // wildcard, which would otherwise cause unrelated files to be picked up on file systems that allow + // these characters in file names (e.g. Linux/macOS). string dumpFileNamePattern = _netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern; - string? dumpDirectory = Path.GetDirectoryName(dumpFileNamePattern); - string searchPattern = ReplaceCrashDumpPlaceholdersWithWildcard(Path.GetFileName(dumpFileNamePattern)); + string dumpDirectory = GetDumpDirectory(dumpFileNamePattern); + Regex dumpFileNameRegex = BuildDumpFileNameRegex(Path.GetFileName(dumpFileNamePattern)); bool publishedAny = false; - if (dumpDirectory is not null && Directory.Exists(dumpDirectory)) + if (Directory.Exists(dumpDirectory)) { - foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory, searchPattern)) + foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory)) { - await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); - publishedAny = true; + if (dumpFileNameRegex.IsMatch(Path.GetFileName(dumpFile))) + { + await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + publishedAny = true; + } } } @@ -89,7 +97,7 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH { string expectedDumpFile = dumpFileNamePattern.Replace("%p", testHostProcessInformation.PID.ToString(CultureInfo.InvariantCulture)); await _outputDisplay.DisplayAsync(this, new ErrorMessageOutputDeviceData(string.Format(CultureInfo.InvariantCulture, CrashDumpResources.CannotFindExpectedCrashDumpFile, expectedDumpFile)), cancellationToken).ConfigureAwait(false); - if (dumpDirectory is not null && Directory.Exists(dumpDirectory)) + if (Directory.Exists(dumpDirectory)) { foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory, "*.dmp")) { @@ -99,28 +107,43 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH } } - internal static string ReplaceCrashDumpPlaceholdersWithWildcard(string fileName) + internal static string GetDumpDirectory(string dumpFileNamePattern) + { + // Path.GetDirectoryName returns "" (not null) for a bare filename on .NET Core/5+; treat that as + // the current working directory so the dump enumeration is not silently skipped. + string? rawDirectory = Path.GetDirectoryName(dumpFileNamePattern); + return rawDirectory is null or "" ? "." : rawDirectory; + } + + internal static Regex BuildDumpFileNameRegex(string fileName) + => new(BuildDumpFileNameRegexPattern(fileName), RegexOptions.CultureInvariant); + + internal static string BuildDumpFileNameRegexPattern(string fileName) { - var sb = new StringBuilder(fileName.Length); + var sb = new StringBuilder("^"); + bool lastWasWildcard = false; for (int i = 0; i < fileName.Length; i++) { if (fileName[i] == '%' && i + 1 < fileName.Length) { - // Replace any %X placeholder with '*'. Collapse consecutive wildcards to keep the search - // pattern minimal and avoid confusing search engines with redundant '**' sequences. - if (sb.Length == 0 || sb[sb.Length - 1] != '*') + // Replace any %X placeholder with ".*". Collapse consecutive wildcards to keep the regex + // simple and to avoid backtracking on patterns like "%p%t". + if (!lastWasWildcard) { - sb.Append('*'); + sb.Append(".*"); + lastWasWildcard = true; } i++; } else { - sb.Append(fileName[i]); + sb.Append(Regex.Escape(fileName[i].ToString())); + lastWasWildcard = false; } } + sb.Append('$'); return sb.ToString(); } } diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs index 52af939c2f..eecbfca3bc 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs @@ -48,15 +48,54 @@ public async Task CrashDump_CommandLineOptions_Are_AlwaysValid() } [TestMethod] - [DataRow("MyApp_%p_crash.dmp", "MyApp_*_crash.dmp")] - [DataRow("%e_%p_crash.dmp", "*_*_crash.dmp")] - [DataRow("%p%t_crash.dmp", "*_crash.dmp")] - [DataRow("customdumpname.dmp", "customdumpname.dmp")] - [DataRow("dump_%p_%t_%h.dmp", "dump_*_*_*.dmp")] - [DataRow("trailing%", "trailing%")] - public void ReplaceCrashDumpPlaceholdersWithWildcard_ConvertsPlaceholdersToWildcards(string fileName, string expected) + [DataRow("MyApp_%p_crash.dmp", @"^MyApp_.*_crash\.dmp$")] + [DataRow("%e_%p_crash.dmp", @"^.*_.*_crash\.dmp$")] + [DataRow("%p%t_crash.dmp", @"^.*_crash\.dmp$")] + [DataRow("customdumpname.dmp", @"^customdumpname\.dmp$")] + [DataRow("dump_%p_%t_%h.dmp", @"^dump_.*_.*_.*\.dmp$")] + [DataRow("trailing%", "^trailing%$")] + // Glob metacharacters that may appear literally in a user-supplied filename must be escaped so they are + // matched literally, not treated as wildcards. This guards against picking up unrelated dump files on + // file systems that allow these characters in file names (e.g. Linux/macOS). + [DataRow("my*dump_%p.dmp", @"^my\*dump_.*\.dmp$")] + [DataRow("dump?_%p.dmp", @"^dump\?_.*\.dmp$")] + public void BuildDumpFileNameRegexPattern_ConvertsPlaceholdersToRegex(string fileName, string expected) { - string actual = CrashDumpProcessLifetimeHandler.ReplaceCrashDumpPlaceholdersWithWildcard(fileName); + string actual = CrashDumpProcessLifetimeHandler.BuildDumpFileNameRegexPattern(fileName); Assert.AreEqual(expected, actual); } + + [TestMethod] + public void BuildDumpFileNameRegex_LiteralGlobMetacharactersInName_DoesNotOverMatch() + { + Regex regex = CrashDumpProcessLifetimeHandler.BuildDumpFileNameRegex("my*dump_%p.dmp"); + Assert.IsTrue(regex.IsMatch("my*dump_123.dmp"), "Literal '*' must be matched literally."); + Assert.IsFalse(regex.IsMatch("myXYZdump_123.dmp"), "Literal '*' must not act as a wildcard."); + Assert.IsFalse(regex.IsMatch("mydump_123.dmp"), "Literal '*' must require at least the '*' character to be present."); + } + + [TestMethod] + [DataRow("dump_%p.dmp")] + [DataRow("")] + [DataRow("customdumpname.dmp")] + public void GetDumpDirectory_WhenPatternHasNoDirectoryComponent_ReturnsCurrentDirectory(string pattern) + { + // The CrashDump runtime writes dumps to the current working directory when the configured pattern + // contains no directory prefix. Previously the extension's enumeration was silently skipped in that + // case because Path.GetDirectoryName returns "" (not null), and Directory.Exists("") is false. + string actual = CrashDumpProcessLifetimeHandler.GetDumpDirectory(pattern); + + Assert.AreEqual(".", actual); + } + + [TestMethod] + public void GetDumpDirectory_WhenPatternHasDirectoryComponent_ReturnsDirectory() + { + string directory = Path.Combine(Path.GetTempPath(), "dumps"); + string pattern = Path.Combine(directory, "dump_%p.dmp"); + + string actual = CrashDumpProcessLifetimeHandler.GetDumpDirectory(pattern); + + Assert.AreEqual(directory, actual); + } } From 8444dedcab184ee2fcd14b385bf23fc5d511e64e Mon Sep 17 00:00:00 2001 From: Copilot <223556219+Copilot@users.noreply.github.com> Date: Sat, 16 May 2026 15:04:48 +0200 Subject: [PATCH 4/4] Address review feedback and fix .NET Framework unit test - Snapshot pre-existing files in the dump directory before the testhost starts and exclude them when publishing crash dumps, so stale dumps from previous runs that match the same pattern are no longer surfaced as artifacts. - Use Path.GetFileName + ordinal-ignore-case comparison instead of EndsWith for the dotnet executable detection in the CrashDump test asset. - Add a 60-second bounded WaitForExit for the spawned child process and kill it on timeout to keep acceptance tests reliable. - Handle empty pattern in GetDumpDirectory so Path.GetDirectoryName('') no longer throws ArgumentException on .NET Framework. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- .../CrashDumpProcessLifetimeHandler.cs | 35 ++++++++++++++++--- .../CrashDumpTests.cs | 22 ++++++++++-- 2 files changed, 49 insertions(+), 8 deletions(-) diff --git a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs index e6afc439c0..e0a0480404 100644 --- a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs +++ b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs @@ -18,6 +18,7 @@ internal sealed class CrashDumpProcessLifetimeHandler : ITestHostProcessLifetime private readonly IMessageBus _messageBus; private readonly IOutputDevice _outputDisplay; private readonly CrashDumpConfiguration _netCoreCrashDumpGeneratorConfiguration; + private HashSet _preExistingDumpFiles = new(StringComparer.OrdinalIgnoreCase); public CrashDumpProcessLifetimeHandler( ICommandLineOptions commandLineOptions, @@ -51,7 +52,21 @@ public Task IsEnabledAsync() public Task BeforeTestHostProcessStartAsync(CancellationToken _) => Task.CompletedTask; - public Task OnTestHostProcessStartedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken) => Task.CompletedTask; + public Task OnTestHostProcessStartedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken) + { + // Snapshot any pre-existing files in the dump directory so we can later restrict dump publication + // to files that appeared during this run. Without this, when the results/dump directory is reused + // across runs, stale dumps from a previous crash whose names also match the configured pattern + // would be surfaced as artifacts of the current failure. + ApplicationStateGuard.Ensure(_netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern is not null); + string dumpDirectory = GetDumpDirectory(_netCoreCrashDumpGeneratorConfiguration.DumpFileNamePattern); + if (Directory.Exists(dumpDirectory)) + { + _preExistingDumpFiles = new HashSet(Directory.EnumerateFiles(dumpDirectory), StringComparer.OrdinalIgnoreCase); + } + + return Task.CompletedTask; + } public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testHostProcessInformation, CancellationToken cancellationToken) { @@ -85,7 +100,8 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH { foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory)) { - if (dumpFileNameRegex.IsMatch(Path.GetFileName(dumpFile))) + if (dumpFileNameRegex.IsMatch(Path.GetFileName(dumpFile)) + && !_preExistingDumpFiles.Contains(dumpFile)) { await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); publishedAny = true; @@ -101,7 +117,10 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH { foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory, "*.dmp")) { - await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + if (!_preExistingDumpFiles.Contains(dumpFile)) + { + await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + } } } } @@ -109,8 +128,14 @@ public async Task OnTestHostProcessExitedAsync(ITestHostProcessInformation testH internal static string GetDumpDirectory(string dumpFileNamePattern) { - // Path.GetDirectoryName returns "" (not null) for a bare filename on .NET Core/5+; treat that as - // the current working directory so the dump enumeration is not silently skipped. + // Path.GetDirectoryName returns "" (not null) for a bare filename on .NET Core/5+ but throws + // ArgumentException for an empty string on .NET Framework; treat both as the current working + // directory so the dump enumeration is not silently skipped. + if (dumpFileNamePattern is null or "") + { + return "."; + } + string? rawDirectory = Path.GetDirectoryName(dumpFileNamePattern); return rawDirectory is null or "" ? "." : rawDirectory; } diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs index 48ee07e510..e6f6c6afe0 100644 --- a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs +++ b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs @@ -111,6 +111,7 @@ public override (string ID, string Name, string Code) GetAssetsToGenerate() => ( #file Program.cs using System; using System.Diagnostics; +using System.IO; using System.Reflection; using System.Threading; using System.Threading.Tasks; @@ -170,7 +171,9 @@ public Task ExecuteRequestAsync(ExecuteRequestContext context) { Process self = Process.GetCurrentProcess(); string path = self.MainModule!.FileName!; - string argPrefix = path.EndsWith("dotnet") || path.EndsWith("dotnet.exe") + string fileName = Path.GetFileName(path); + string argPrefix = string.Equals(fileName, "dotnet", StringComparison.OrdinalIgnoreCase) + || string.Equals(fileName, "dotnet.exe", StringComparison.OrdinalIgnoreCase) ? $"exec \"{Assembly.GetEntryAssembly()!.Location}\" " : string.Empty; @@ -179,8 +182,21 @@ public Task ExecuteRequestAsync(ExecuteRequestContext context) UseShellExecute = false, })!; - // Wait for the child to fully exit so its crash dump is written before we crash too. - child.WaitForExit(); + // Wait for the child to fully exit (with a bounded timeout to avoid hanging the test run) + // so its crash dump is written before we crash too. + if (!child.WaitForExit(60_000)) + { + try + { + child.Kill(); + } + catch + { + // Best effort: process may have just exited. + } + + throw new InvalidOperationException("Child process did not exit within the expected timeout (60s)."); + } } Environment.FailFast("CrashDump");