Add crash report support to the CrashDump extension#8191
Conversation
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Extends the CrashDump MTP extension with new --crashreport and --crashreport-only options that wire the test host runtime variables DOTNET_EnableCrashReport / DOTNET_EnableCrashReportOnly and publish the resulting *.crashreport.json as artifacts.
Changes:
- New CLI options with mutual-exclusion validation (
--crashreportrequires--crashdump;--crashreport-onlyexcludes the others). - Environment variable provider sets the new runtime variables (and keeps
DbgEnableMiniDumpfor--crashreport-only); lifetime handler emits new "dump only / report only / dump + report" messages and publishes crash report artifacts. - New resx/xlf entries, PACKAGE.md doc update, unit/acceptance/help-info test coverage for the new options.
Show a summary per file
| File | Description |
|---|---|
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpCommandLineOptions.cs | Adds option name constants for the two new flags. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpCommandLineProvider.cs | Registers new options and adds combined-options validation via chained ternary. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpEnvironmentVariableProvider.cs | Sets/validates DOTNET_EnableCrashReport(Only) and conditionalizes DbgEnableMiniDump. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs | Picks crash message variant; publishes .crashreport.json artifacts in addition to dumps. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/CrashDumpResources.resx | New resource strings for descriptions, errors, and crash messages. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/*.xlf | Auto-added state="new" entries mirroring the resx additions across all locales. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/PACKAGE.md | Documents new crash report capabilities. |
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs | Adds unit tests for valid/invalid combinations of the new flags. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/CrashDumpTests.cs | Acceptance tests for dump+report, report-only, and --crashreport without --crashdump. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs | Updates expected help/info output to include the new options. |
Copilot's findings
- Files reviewed: 22/22 changed files
- Comments generated: 3
| => commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName) && | ||
| !commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) | ||
| ? ValidationResult.InvalidTask(CrashDumpResources.CrashReportRequiresCrashDumpErrorMessage) | ||
| : commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName) && | ||
| (commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) || | ||
| commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName)) | ||
| ? ValidationResult.InvalidTask(CrashDumpResources.CrashReportOnlyCannotBeCombinedErrorMessage) | ||
| : ValidationResult.ValidTask; |
There was a problem hiding this comment.
Done in e8b4818. Refactored into explicit if-statements (one per invalid combination) so future mutually-exclusive crash options are easy to add.
| if (crashReportEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableCrashReportVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
| } | ||
|
|
||
| if (crashReportOnlyEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableCrashReportOnlyVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
| } |
There was a problem hiding this comment.
Done in e8b4818. Renamed EnableMiniDumpValue to the neutral EnabledValue since it's now also used for DOTNET_EnableCrashReport and DOTNET_EnableCrashReportOnly.
| TestHostResult testHostResult = await testHost.ExecuteAsync($"--crashreport-only --crashdump-filename customdumpname.dmp --results-directory {resultDirectory}", cancellationToken: TestContext.CancellationToken); | ||
| testHostResult.AssertExitCodeIs(ExitCode.TestHostProcessExitedNonGracefully); | ||
|
|
||
| Assert.IsEmpty(Directory.GetFiles(resultDirectory, "customdumpname.dmp", SearchOption.AllDirectories), $"Unexpected dump file found\n{testHostResult}"); |
There was a problem hiding this comment.
Done in e8b4818. Switched to Directory.EnumerateFiles(*).Where(f => string.Equals(Path.GetFileName(f), "customdumpname.dmp", StringComparison.Ordinal)) so the assertion isn't tricked by Windows pattern-matching matching customdumpname.dmp.crashreport.json.
…shDump tests/code - Reorder --crashreport / --crashreport-only after --crashdump-type in the Help and Info expectations to match the platform's alphabetical ordering (CommandLineHandler.PrintOptionsAsync OrderBy(option.Name)). - Refactor CrashDump option validation into explicit if-statements for clarity. - Rename EnableMiniDumpValue -> EnabledValue (now reused for crash report vars). - Make CrashReportOnly_CustomDumpName_CreateOnlyCrashReport robust on Windows by doing an exact filename comparison instead of relying on Directory.GetFiles pattern matching, which can match 'customdumpname.dmp.crashreport.json'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Revert the validation refactor, EnabledValue rename, and the Windows-safe filename check in CrashReportOnly_CustomDumpName_CreateOnlyCrashReport. The HelpInfoAllExtensionsTests ordering fix is preserved. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs:1
- Mirror of the existing dump-fallback path: if
Path.GetDirectoryName(expectedCrashReportFile)returns an empty string (when the dump file pattern is just a filename with no directory component),Directory.GetFiles("", ...)will throwArgumentException. The pre-existing dump branch has the same issue, but consider falling back to the current directory or skipping the scan when the directory is empty so the new code path doesn't introduce another instance of the same brittleness.
// Copyright (c) Microsoft Corporation. All rights reserved.
- Files reviewed: 22/22 changed files
- Comments generated: 8
| => commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName) && | ||
| !commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) | ||
| ? ValidationResult.InvalidTask(CrashDumpResources.CrashReportRequiresCrashDumpErrorMessage) | ||
| : commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOnlyOptionName) && | ||
| (commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashDumpOptionName) || | ||
| commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName)) | ||
| ? ValidationResult.InvalidTask(CrashDumpResources.CrashReportOnlyCannotBeCombinedErrorMessage) | ||
| : ValidationResult.ValidTask; |
| <value>[net6.0+ only] Generate a crash report file if the test process crashes. Requires '--crashdump'.</value> | ||
| </data> | ||
| <data name="CrashReportRequiresCrashDumpErrorMessage" xml:space="preserve"> | ||
| <value>You specified '--crashreport' but did not enable crash dumps, add --crashdump to the command line</value> |
| if (crashDumpEnabled || crashReportOnlyEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableMiniDumpVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
| } |
| { | ||
| 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")) | ||
| string expectedCrashReportFile = $"{expectedDumpFile}.crashreport.json"; |
| } | ||
|
|
||
| [TestMethod] | ||
| public async Task CrashReportOnly_CustomDumpName_CreateOnlyCrashReport() |
| if (_commandLineOptions.IsOptionSet(CrashDumpCommandLineOptions.CrashReportOptionName)) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| if (!environmentVariables.TryGetVariable($"{prefix}{EnableCrashReportVariable}", out OwnedEnvironmentVariable? enableCrashReport) | ||
| || enableCrashReport.Value != EnableMiniDumpValue) | ||
| { | ||
| AddError(errors, $"{prefix}{EnableCrashReportVariable}", EnableMiniDumpValue, enableCrashReport?.Value); |
| [TestMethod] | ||
| public async Task CrashReportOnly_Cannot_Be_Combined_With_Other_Crash_Options() | ||
| { | ||
| var provider = new CrashDumpCommandLineProvider(); | ||
| var options = new Dictionary<string, string[]> | ||
| { | ||
| { CrashDumpCommandLineOptions.CrashDumpOptionName, [] }, |
| if (crashReportEnabled) | ||
| { | ||
| foreach (string prefix in Prefixes) | ||
| { | ||
| environmentVariables.SetVariable(new($"{prefix}{EnableCrashReportVariable}", EnableMiniDumpValue, false, true)); | ||
| } | ||
| } |
- CrashDumpCommandLineProvider: replace chained ternary with explicit if-statements for clarity and easier extension. - CrashDumpEnvironmentVariableProvider: rename EnableMiniDumpValue to EnabledValue since it's now reused for the crash report environment variables (DOTNET_EnableCrashReport / DOTNET_EnableCrashReportOnly). - CrashReportOnly_CustomDumpName_CreateOnlyCrashReport: do an explicit filename comparison instead of relying on Directory.GetFiles' pattern matching, which on Windows can also match 'customdumpname.dmp.crashreport.json' for the literal pattern 'customdumpname.dmp'. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
@copilot address review comments |
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
New Feature
What does this feature do?
The CrashDump extension can now ask the .NET runtime to generate JSON crash reports in addition to, or instead of, a dump. This makes crash triage lighter-weight in CI, especially for environments where full dump collection is expensive or impractical.
Why is this feature needed?
DOTNET_EnableCrashReportandDOTNET_EnableCrashReportOnlyare already available in the runtime, but the MTP CrashDump extension only exposed dump generation. Surfacing crash reports gives a cheaper diagnostic path and improves crash investigation on machines where developers cannot inspect native dumps directly.Implementation details
CLI surface
--crashreportto generate a crash report alongside--crashdump--crashreport-onlyto request only the JSON crash report--crashreportrequires--crashdump--crashreport-onlycannot be combined with other crash optionsRuntime configuration
DOTNET_EnableCrashReportDOTNET_EnableCrashReportOnlyDbgEnableMiniDumpenabled for--crashreport-only, since the runtime still requires createdump activation to emit the reportArtifacts and user-visible behavior
*.crashreport.jsonCoverage
--crashreportusageExample