diff --git a/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs b/src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs index dce2b1b5da..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) { @@ -65,21 +80,95 @@ 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 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 = GetDumpDirectory(dumpFileNamePattern); + Regex dumpFileNameRegex = BuildDumpFileNameRegex(Path.GetFileName(dumpFileNamePattern)); + + bool publishedAny = false; + if (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)) + { + 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; + } + } } - 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 (Directory.Exists(dumpDirectory)) + { + foreach (string dumpFile in Directory.EnumerateFiles(dumpDirectory, "*.dmp")) + { + if (!_preExistingDumpFiles.Contains(dumpFile)) + { + await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpArtifactDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + } + } + } + } + } + + internal static string GetDumpDirectory(string dumpFileNamePattern) + { + // 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; + } + + internal static Regex BuildDumpFileNameRegex(string fileName) + => new(BuildDumpFileNameRegexPattern(fileName), RegexOptions.CultureInvariant); + + internal static string BuildDumpFileNameRegexPattern(string fileName) + { + var sb = new StringBuilder("^"); + bool lastWasWildcard = false; + for (int i = 0; i < fileName.Length; i++) + { + if (fileName[i] == '%' && i + 1 < fileName.Length) { - await _messageBus.PublishAsync(this, new FileArtifact(new FileInfo(dumpFile), CrashDumpResources.CrashDumpDisplayName, CrashDumpResources.CrashDumpArtifactDescription)).ConfigureAwait(false); + // 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(".*"); + lastWasWildcard = true; + } + + i++; + } + else + { + sb.Append(Regex.Escape(fileName[i].ToString())); + lastWasWildcard = false; } } + + sb.Append('$'); + return sb.ToString(); } } diff --git a/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs b/test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs index cf7e844be3..849a4edbc2 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,9 @@ 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; using System.Globalization; @@ -97,6 +129,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 +165,40 @@ 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") + { + using Process self = Process.GetCurrentProcess(); + string path = self.MainModule!.FileName!; + 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; + + using Process child = Process.Start(new ProcessStartInfo(path, $"{argPrefix}--child-crash") + { + UseShellExecute = false, + })!; + + // 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"); context.Complete(); return Task.CompletedTask; diff --git a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs index 4f2b1640f2..eecbfca3bc 100644 --- a/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs +++ b/test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs @@ -46,4 +46,56 @@ 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%$")] + // 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.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); + } }