Uh oh!
There was an error while loading. Please reload this page.
Add --crash-report-if-supported and --hangdump-type-if-supported options - #8666
Merged
Amaury Levé (Evangelink) merged 5 commits intoMay 31, 2026
Merged
Conversation
Companion options that silently no-op when the underlying mechanism is unsupported on the current OS/TFM, so a single CLI line works on every build leg (issue #7126). - --crash-report-if-supported (arity 0): mirrors --crash-report but is ignored on Windows (DOTNET_EnableCrashReportOnly is not honored there) and on .NET Framework (no createdump runtime). - --hangdump-type-if-supported <Mini|Heap|Full|Triage|None> (arity 1): mirrors --hangdump-type but maps requested types unsupported on the current TFM (today: Triage on .NET Framework) to the closest equivalent (Mini). Each variant emits a single informational line when it no-ops so users can see the substitution happened. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR adds “best-effort” companion CLI options in Microsoft.Testing.Platform diagnostics extensions to reduce CI matrix friction by silently no-op’ing (with a single console message) when the underlying crash-report or dump-type mechanism isn’t supported on the current runtime/OS.
Changes:
- Add
--crash-report-if-supported(CrashDump) and--hangdump-type-if-supported(HangDump) options with mutual-exclusion validation against their strict counterparts. - Implement runtime/OS gating and fallback behavior (CrashReport ignored on Windows/.NET Framework; HangDump type mapping when requested type isn’t supported).
- Add/extend unit tests, update help/info acceptance expectations, and update localized resources.
Show a summary per file
| File | Description |
|---|---|
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/HangDumpTests.cs | Adds unit coverage for --hangdump-type-if-supported validation, mutual exclusion, and mapping helpers. |
| test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs | Adds unit coverage for --crash-report-if-supported, mutual exclusion, arity, and “effective” gating helper. |
| test/IntegrationTests/Microsoft.Testing.Platform.Acceptance.IntegrationTests/HelpInfoAllExtensionsTests.cs | Updates --help / --info expectations to include the new options and their descriptions. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpCommandLineProvider.cs | Registers --hangdump-type-if-supported, validates values across TFMs, enforces mutual exclusion, and adds mapping helpers. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/HangDumpProcessLifetimeHandler.cs | Applies best-effort dump-type mapping and emits a single message when a fallback occurs. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/ExtensionResources.resx | Adds new HangDump option description + mutual-exclusion/fallback messages. |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.cs.xlf | Localization update for new HangDump strings (Czech). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.de.xlf | Localization update for new HangDump strings (German). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.es.xlf | Localization update for new HangDump strings (Spanish). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.fr.xlf | Localization update for new HangDump strings (French). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.it.xlf | Localization update for new HangDump strings (Italian). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ja.xlf | Localization update for new HangDump strings (Japanese). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ko.xlf | Localization update for new HangDump strings (Korean). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pl.xlf | Localization update for new HangDump strings (Polish). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.pt-BR.xlf | Localization update for new HangDump strings (Portuguese - Brazil). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.ru.xlf | Localization update for new HangDump strings (Russian). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.tr.xlf | Localization update for new HangDump strings (Turkish). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hans.xlf | Localization update for new HangDump strings (Chinese Simplified). |
| src/Platform/Microsoft.Testing.Extensions.HangDump/Resources/xlf/ExtensionResources.zh-Hant.xlf | Localization update for new HangDump strings (Chinese Traditional). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpCommandLineOptions.cs | Defines the new crash-report-if-supported option name constant. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpCommandLineProvider.cs | Registers --crash-report-if-supported, enforces mutual exclusion, and treats it as a main option. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpEnvironmentVariableProvider.cs | Gates env-var application via IsCrashReportEffective so Windows/.NET Framework no-op cases don’t error. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs | Enables handler for --crash-report-if-supported to emit the informational line and avoids artifact scanning when ineffective. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/CrashDumpResources.resx | Adds CrashDump option description + mutual-exclusion and “ignored” info messages; updates Windows unsupported message. |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.cs.xlf | Localization update for new CrashDump strings (Czech). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.de.xlf | Localization update for new CrashDump strings (German). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.es.xlf | Localization update for new CrashDump strings (Spanish). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.fr.xlf | Localization update for new CrashDump strings (French). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.it.xlf | Localization update for new CrashDump strings (Italian). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.ja.xlf | Localization update for new CrashDump strings (Japanese). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.ko.xlf | Localization update for new CrashDump strings (Korean). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.pl.xlf | Localization update for new CrashDump strings (Polish). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.pt-BR.xlf | Localization update for new CrashDump strings (Portuguese - Brazil). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.ru.xlf | Localization update for new CrashDump strings (Russian). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.tr.xlf | Localization update for new CrashDump strings (Turkish). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.zh-Hans.xlf | Localization update for new CrashDump strings (Chinese Simplified). |
| src/Platform/Microsoft.Testing.Extensions.CrashDump/Resources/xlf/CrashDumpResources.zh-Hant.xlf | Localization update for new CrashDump strings (Chinese Traditional). |
Copilot's findings
- Files reviewed: 37/37 changed files
- Comments generated: 3
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Aligns three locations that still described --hangdump-type-if-supported as falling back to the default 'Full' (the original design) instead of the actual closest-supported-type mapping (Triage -> Mini on netfx): - HangDumpCommandLineProvider.cs: AllHangDumpTypeOptions comment. - HelpInfoAllExtensionsTests.cs: --help and --info expectations. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Amaury Levé (Evangelink)
left a comment
MemberAuthor
There was a problem hiding this comment.
Review of PR #8666 — --crash-report-if-supported / --hangdump-type-if-supported
Summary
The overall design is sound and well-structured: the new -if-supported companions are wired in at every layer (CLI validation, env-var provider, lifecycle callbacks), the mutual-exclusion checks are correct, the IsCrashReportEffective predicate cleanly avoids double-activation, and the MapToSupportedDumpType / IsHangDumpTypeSupportedOnCurrentRuntime pair is a solid runtime-dispatch pattern.
21-dimension verdict
| # | Dimension | Verdict | Notes |
|---|---|---|---|
| 1 | Algorithmic Correctness | ✅ | IsCrashReportEffective, MapToSupportedDumpType, and the IsCrashHandlingEffective guard all trace correctly for every branch (netfx/net·win/net·unix). ApplicationStateGuard.Ensure guards are preserved. |
| 2 | Threading & Concurrency | _ifSupportedIgnoredMessageEmitted is a non-volatilebool read/written across potential thread switches; see inline comment. | |
| 3 | Security | ✅ | No new file operations or untrusted input. |
| 4 | Public API / Binary Compat | ✅ | All new constants and helpers are internal. No PublicAPI.Unshipped.txt changes needed. |
| 5 | Performance | ✅ | Cold path only; no hot-path impact. |
| 6 | Cross-TFM Compatibility | ✅ | #if !NETCOREAPP / #if NET guards are correct and consistent. |
| 7 | Resource / IDisposable | ✅ | No new disposables. |
| 8 | Defensive Coding | ✅ | Existing ApplicationStateGuard.Ensure guards preserved; new guards added only for the effective paths. |
| 9 | Localization | ✅ | All strings in .resx. XLF files carry target state="new" markers (build-generated, not hand-edited). |
| 10 | Test Isolation | ✅ | No shared static mutable state added. |
| 11 | Assertion Quality | ✅ | Unit tests use MSTest assertions as required for MTP test projects. |
| 12 | Flakiness | ✅ | No time-dependent assertions. |
| 13 | CLI / Option Consistency | --hangdump-type-if-supported is classified as a "sub-option" (requires --hangdump), consistent with --hangdump-type — but the option name implies it could stand alone. The PR description says this is intentional; worth a note in the --help description or error message so users aren't confused. | |
| 14 | Output / UX | WarningMessageOutputDeviceData (yellow) used for graceful no-op paths. When the user chose -if-supportedbecause they expect the platform not to support it, a yellow warning is noise. See inline comments on CrashDumpProcessLifetimeHandler.cs:102 and HangDumpProcessLifetimeHandler.cs:121. | |
| 15 | Test Coverage | ✅ | Unit tests cover IsCrashReportEffective, MapToSupportedDumpType, mutual exclusion, and argument validation. |
| 16 | Naming & Conventions | ✅ | Naming is clear and consistent with the existing --crashdump / --hangdump family. |
| 17 | Comment Quality | ✅ | Inline comments are detailed and reference the upstream runtime issue (dotnet/runtime#80191). |
| 18 | Error Messages | ✅ | Mutual-exclusion messages guide the user toward the correct option. |
| 19 | Scope Discipline | ✅ | PR is tightly focused on the two new companion options. |
| 20 | Help/Info Test Updates | ✅ | HelpInfoAllExtensionsTests expectations updated. |
| 21 | XLF / Localization Pipeline | ✅ | XLF files correctly updated with target state="new" by the build tool. |
Actionable items
_ifSupportedIgnoredMessageEmitted— addvolatile(CrashDumpProcessLifetimeHandler.cs:44): the "emit once" guard can be bypassed under concurrent test-host restarts without a memory barrier.WarningMessageOutputDeviceData→ informational format (CrashDumpProcessLifetimeHandler.cs:102/110,HangDumpProcessLifetimeHandler.cs:121): the-if-supportedvariants are explicitly opt-in best-effort; a yellow warning contradicts the intent and adds noise to CI logs.
Generated by Expert Code Review (on open) for issue #8666 · sonnet46 3.6M
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Use FormattedTextOutputDeviceData instead of WarningMessageOutputDeviceData for the '-if-supported' no-op / fallback messages. These are expected, graceful paths; rendering them as yellow warnings would mislead CI users. (CrashDumpProcessLifetimeHandler.cs x2, HangDumpProcessLifetimeHandler.cs x1) - Replace the plain bool one-shot guard in CrashDumpProcessLifetimeHandler with Interlocked.Exchange on an int field, so that concurrent invocations (e.g. test-host controller retries) cannot race past the guard. Also restructure to early-return when the option will not emit anything on the current runtime/OS, so the guard is only claimed when we actually emit. - Collapse the nested 'if' in HangDumpCommandLineProvider.ValidateOptionArgumentsAsync for --hangdump-type-if-supported into a single conditional return (also satisfies IDE0046). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Copilot's findings
Comments suppressed due to low confidence (1)
test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs:359
- The placeholder-to-regex conversion test no longer covers several common createdump placeholders/pattern shapes (e.g. %e, %h, %t, literal-only patterns). Those DataRow cases previously validated that placeholders are expanded to wildcards across multiple tokens and adjacent placeholders; dropping them reduces coverage for BuildDumpFileNameRegexPattern and makes regressions easier to miss.
- Files reviewed: 37/37 changed files
- Comments generated: 1
Uh oh!
There was an error while loading. Please reload this page.
…ents The earlier comment in CrashDumpEnvironmentVariableProvider above the 'crashReportEnabled' assignment said 'IsEnabledAsync gates this method, so at least one of --crashdump / --crash-report / --crash-report-if-supported is set here.' That wording suggested '--crash-report-if-supported' alone is sufficient to reach UpdateAsync / ValidateTestHostEnvironmentVariablesAsync even on Windows / .NET Framework, where the option is intentionally a no-op (IsCrashReportEffective returns false and IsEnabledAsync is false unless '--crashdump' is also set). Reword both occurrences to refer to an *effective* crash-report request so future readers do not misinterpret the precondition. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink)
deleted the
dev/amauryleve/crash-report-if-supported
branch
May 31, 2026 06:41
This was referenced May 31, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes#7126 (companion options for
--crash-reportand--hangdump-type).Problem
@bart-vmware pointed out that
--crash-reporterrors out on Windows because the .NET runtime ignoresDOTNET_EnableCrashReportOnlythere. The same kind of friction exists for--hangdump-type Triageon .NET Framework (Triage is a netcoreapp-only dump type). The current behaviour forces consumers to maintain different CLI commands per OS / TFM in their CI scripts.Solution
Introduce two new
-if-supportedcompanion options that behave identically to the strict variants when the underlying mechanism is supported, and silently no-op (with a single info line on the console) when it is not.--crash-report(errors on Windows)--crash-report-if-supported(no-op on Windows / .NET Framework)--hangdump-type <Mini|Heap|Full|Triage|None>(Triage rejected on netfx)--hangdump-type-if-supported <…>(Triage on netfx maps to Mini)This lets users keep a single CI invocation across all build legs.
Naming rationale
We considered short forms (
--crash-report?,--crash-report-best-effort, etc.) and decided on the explicit long-if-supportedsuffix:-if-supportedvariants can follow the same pattern.Behaviour matrix
--crash-report-if-supported--crash-reportMutually exclusive with
--crash-report.--hangdump-type-if-supported <type>Mini,Heap,Full,Triage,NoneMini/Heap/Full/NoneTriageMini(info message), asMiniis the closest equivalentMutually exclusive with
--hangdump-type.Implementation notes
The lifetime handler's
IsEnabledAsyncreturnstruefor the no-op case (so it can emit the info message), but the env-var provider'sIsEnabledAsyncand the lifecycle methods are gated onIsCrashReportEffective/IsHangDumpTypeSupportedOnCurrentRuntimeto avoid:ValidateTestHostEnvironmentVariablesAsync.DbgEnableMiniDump=1on Windows when the mechanism is known to be ignored.ApplicationStateGuard.Ensurechecks on a dump file name pattern that was never set up.Tests
IsCrashReportEffectiveandMapToSupportedDumpType(added toCrashDumpTests/HangDumpTests).--crashdump, never rejected on any platform, satisfies the-main-option-missingrule, both variants registered as arity-0.HelpInfoAllExtensionsTestsexpectations for both human-readable and structured--infooutput.Local validation: 85/89 tests pass on net8.0 (4 Windows-skipped pre-existing CrashReport tests), 84/88 on net472 (same 4 skipped). Production projects (
Microsoft.Testing.Extensions.CrashDump,Microsoft.Testing.Extensions.HangDump) and the unit-test project all build clean (0 warnings, 0 errors).Acceptance tests for end-to-end behaviour aren't included here yet; happy to follow up if reviewers want them.
Why not an environment variable?
@bart-vmware also suggested keeping the hard error but letting users opt into a "downgrade to info" via an environment variable. We discarded that route in favour of an explicit CLI option for the following reasons:
--help/--infoand is grep-able in CI scripts. An environment variable only surfaces when the user already hit the error and read the message, which is exactly the friction we are trying to remove.--crash-report-if-supportedclearly conveys the user's intent ("I want a crash report when I can get one"). A script that setsMTP_ALLOW_UNSUPPORTED_CRASH_REPORT=1(or similar) and then calls--crash-reporthides that intent in the environment.--crash-report,--hangdump-type Triage, plus every future option in the same situation), inflating the env-var surface. The-if-supportedsuffix is a uniform naming convention we can reuse going forward.--crash-report/--hangdump-type— the two variants coexist, are mutually exclusive at validation time, and a single CI matrix can mix the two if it really wants to.The strict
--crash-report/--hangdump-typeare unchanged, so callers that prefer the fail-fast contract keep their current behaviour.