-
Notifications
You must be signed in to change notification settings - Fork 288
Collect all crash dumps in directory regardless of main process exit status #6817
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 7 commits
8df1ee6
d53479c
b4a22dd
c114745
aa1052f
f36cbf7
5aae7c4
40aac17
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,6 +72,24 @@ public async Task CrashDump_InvalidFormat_ShouldFail() | |
| testHostResult.AssertOutputContains("Option '--crashdump-type' has invalid arguments: 'invalid' is not a valid dump type. Valid options are 'Mini', 'Heap', 'Triage' and 'Full'"); | ||
| } | ||
|
|
||
| [TestMethod] | ||
| public async Task CrashDump_WithChildProcess_CollectsMultipleDumps() | ||
| { | ||
| if (RuntimeInformation.IsOSPlatform(OSPlatform.OSX)) | ||
| { | ||
| // TODO: Investigate failures on macos | ||
| return; | ||
| } | ||
|
|
||
| string resultDirectory = Path.Combine(AssetFixture.TargetAssetPath, Guid.NewGuid().ToString("N")); | ||
| var testHost = TestInfrastructure.TestHost.LocateFrom(AssetFixture.TargetAssetPath, "CrashDumpWithChild", TargetFrameworks.NetCurrent); | ||
| TestHostResult testHostResult = await testHost.ExecuteAsync($"--crashdump --results-directory {resultDirectory}", cancellationToken: TestContext.CancellationToken); | ||
| testHostResult.AssertExitCodeIs(ExitCodes.TestHostProcessExitedNonGracefully); | ||
|
|
||
| string[] dumpFiles = Directory.GetFiles(resultDirectory, "*.dmp", SearchOption.AllDirectories); | ||
| Assert.IsGreaterThanOrEqualTo(2, dumpFiles.Length, $"Expected at least 2 dump files (parent and child), but found {dumpFiles.Length}. Dumps: {string.Join(", ", dumpFiles.Select(Path.GetFileName))}\n{testHostResult}"); | ||
| } | ||
|
|
||
| public sealed class TestAssetFixture() : TestAssetFixtureBase(AcceptanceFixture.NuGetGlobalPackagesFolder) | ||
| { | ||
| private const string AssetName = "CrashDumpFixture"; | ||
|
|
@@ -82,6 +100,11 @@ public sealed class TestAssetFixture() : TestAssetFixtureBase(AcceptanceFixture. | |
| Sources | ||
| .PatchTargetFrameworks(TargetFrameworks.All) | ||
| .PatchCodeWithReplace("$MicrosoftTestingPlatformVersion$", MicrosoftTestingPlatformVersion)); | ||
|
|
||
| yield return ("CrashDumpWithChildFixture", "CrashDumpWithChildFixture", | ||
| SourcesWithChild | ||
| .PatchTargetFrameworks(TargetFrameworks.All) | ||
| .PatchCodeWithReplace("$MicrosoftTestingPlatformVersion$", MicrosoftTestingPlatformVersion)); | ||
| } | ||
|
|
||
| public string TargetAssetPath => GetAssetPath(AssetName); | ||
|
|
@@ -152,6 +175,113 @@ public Task ExecuteRequestAsync(ExecuteRequestContext context) | |
| return Task.CompletedTask; | ||
| } | ||
| } | ||
| """; | ||
|
|
||
| private const string SourcesWithChild = """ | ||
| #file CrashDumpWithChild.csproj | ||
| <Project Sdk="Microsoft.NET.Sdk"> | ||
| <PropertyGroup> | ||
| <TargetFrameworks>$TargetFrameworks$</TargetFrameworks> | ||
| <OutputType>Exe</OutputType> | ||
| <UseAppHost>true</UseAppHost> | ||
| <Nullable>enable</Nullable> | ||
| <LangVersion>preview</LangVersion> | ||
| </PropertyGroup> | ||
| <ItemGroup> | ||
| <PackageReference Include="Microsoft.Testing.Extensions.CrashDump" Version="$MicrosoftTestingPlatformVersion$" /> | ||
| </ItemGroup> | ||
| </Project> | ||
|
|
||
| #file Program.cs | ||
| using System; | ||
| using System.Diagnostics; | ||
| using System.IO; | ||
| using System.Linq; | ||
| using System.Threading; | ||
| using System.Threading.Tasks; | ||
| using System.Globalization; | ||
| using Microsoft.Testing.Platform; | ||
| using Microsoft.Testing.Platform.Extensions.TestFramework; | ||
| using Microsoft.Testing.Platform.Builder; | ||
| using Microsoft.Testing.Platform.Capabilities.TestFramework; | ||
| using Microsoft.Testing.Extensions; | ||
| using Microsoft.Testing.Platform.Messages; | ||
| using Microsoft.Testing.Platform.Requests; | ||
| using Microsoft.Testing.Platform.Services; | ||
|
|
||
| public class Startup | ||
| { | ||
| public static async Task<int> Main(string[] args) | ||
| { | ||
| Process self = Process.GetCurrentProcess(); | ||
| string path = self.MainModule!.FileName!; | ||
|
|
||
| // Handle child process execution | ||
| if (args.Length > 0 && args[0] == "--child") | ||
| { | ||
| // Child process crashes immediately | ||
| Environment.FailFast("Child process crash"); | ||
| return 1; | ||
| } | ||
|
|
||
| // Start a child process that will also crash (only when running as testhost controller) | ||
|
||
| if (args.Any(a => a == "--internal-testhostcontroller-pid")) | ||
| { | ||
| try | ||
| { | ||
| var childProcess = Process.Start(new ProcessStartInfo(path, "--child") | ||
| { | ||
| UseShellExecute = false, | ||
| RedirectStandardOutput = true, | ||
| RedirectStandardError = true, | ||
| }); | ||
|
|
||
| if (childProcess != null) | ||
| { | ||
| // Give child process time to start and crash | ||
| Thread.Sleep(500); | ||
| } | ||
| } | ||
| catch | ||
| { | ||
| // Ignore any errors starting child process | ||
| } | ||
| } | ||
|
|
||
| ITestApplicationBuilder builder = await TestApplication.CreateBuilderAsync(args); | ||
| builder.RegisterTestFramework(_ => new TestFrameworkCapabilities(), (_,__) => new DummyTestFramework()); | ||
| builder.AddCrashDumpProvider(); | ||
| using ITestApplication app = await builder.BuildAsync(); | ||
| return await app.RunAsync(); | ||
| } | ||
| } | ||
|
|
||
| public class DummyTestFramework : ITestFramework | ||
| { | ||
| public string Uid => nameof(DummyTestFramework); | ||
|
|
||
| public string Version => "2.0.0"; | ||
|
|
||
| public string DisplayName => nameof(DummyTestFramework); | ||
|
|
||
| public string Description => nameof(DummyTestFramework); | ||
|
|
||
| public Task<bool> IsEnabledAsync() => Task.FromResult(true); | ||
|
|
||
| public Task<CreateTestSessionResult> CreateTestSessionAsync(CreateTestSessionContext context) | ||
| => Task.FromResult(new CreateTestSessionResult() { IsSuccess = true }); | ||
|
|
||
| public Task<CloseTestSessionResult> CloseTestSessionAsync(CloseTestSessionContext context) | ||
| => Task.FromResult(new CloseTestSessionResult() { IsSuccess = true }); | ||
|
|
||
| public Task ExecuteRequestAsync(ExecuteRequestContext context) | ||
| { | ||
| // Parent process crashes | ||
| Environment.FailFast("Parent process crash"); | ||
| context.Complete(); | ||
| return Task.CompletedTask; | ||
| } | ||
| } | ||
| """; | ||
| } | ||
|
|
||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This still looks wrong, what if the main process does not crash?
The reason why vstest crash dumper checks for crash (it does that in stupid way of comparing test counts, but whatever), is that we always collect dump on process exit when procdump crash dumper is used. This allows us to capture out of memory and stack overflow exceptions in .net framework processes. In .net core process this is handled in a better way, the process will be able to capture dump on OOM or on stack overflow, and you don't have to collect dump always.
So all that is needed here is making unique folder per run, and reporting all dumps from there.
If this later expands to use procdump, it would need to detect premature exit of the testhost process (at least), no matter what the exit code is, to be able to distunguish a crash of the process or just successful exit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot please update handling based on this comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the
HasExitedGracefullycheck and simplified the logic to always collect all dumps from the directory. The implementation now:.dmpfiles in the directory, even if main process exits gracefullyCommit: 40aac17