Stop forcing verbose createdump diagnostics in CrashDump extension

[Amaury Levé (Evangelink)]

Fixes#7045

Problem

When the Microsoft.Testing.Extensions.CrashDump extension is enabled in MTP, the runtime's createdump emits megabytes of [createdump] diagnostics (thread RIPs, AUXV, module mappings, ...) to stdout on Linux/macOS whenever a testhost crashes. In the reporter's case (#7045) this produced a 160 MB log for a single crashed test, large enough to make the GitHub Actions UI unresponsive.

The cause is that CrashDumpEnvironmentVariableProvider unconditionally set DOTNET_CreateDumpDiagnostics and DOTNET_CreateDumpVerboseDiagnostics (and the COMPlus_ twins) to ""1"", which turns on the runtime's verbose createdump logging. VSTest does not enable these. HangDump already does the right thing by calling WriteDump(..., logDumpGeneration: false). So this extension was the odd one out.

Fix

• Stop setting CreateDumpDiagnostics / CreateDumpVerboseDiagnostics (both DOTNET_ and COMPlus_) in CrashDumpEnvironmentVariableProvider. Drop the corresponding ValidateBothPrefixes calls and the now-unused private const fields.
• Add the missing COMPlus_ twins of the createdump / crash-report environment variables to the test infrastructure WellKnownEnvironmentVariables.ToSkipEnvironmentVariables list (the DOTNET_ ones were already there - flagged by Jakub Jareš (@nohwnd) in the issue).

Opt back in

Anyone who actually wanted the verbose createdump output (e.g. when filing a runtime issue) can opt in by setting DOTNET_CreateDumpDiagnostics=1 (and/or DOTNET_CreateDumpVerboseDiagnostics=1) in the parent process environment - the platform no longer overrides or locks those variables.

Out of scope (tracked separately / referred to other repos)

• Lowering the default --crashdump-type from Full to Mini to match VSTest (would change dump file size; deserves its own RFC).
• Spurious ""A test session start event was received without a corresponding test session end"" - fixed by [dotnet test][MTP] Avoid surfacing MissingTestSessionEnd when the exit code is already non-success dotnet/sdk#51857.
• macOS dump hang when UseAppHost is the default - UseAppHost shouldn't be required for running xunit.v3 projects xunit/xunit#3432 (entitlements workaround documented in MTP docs).
• Dump tooling not surfacing the recursive frame in VS / dotnet dump analyze - runtime/diagnostics issue.

[Copilot]

Pull request overview

Stops the CrashDump extension from forcing verbose createdump diagnostics that produced massive (160 MB) GitHub Actions logs on testhost crashes, aligning behavior with VSTest and HangDump.

Changes:

• Remove unconditional setting of CreateDumpDiagnostics / CreateDumpVerboseDiagnostics (both DOTNET_ and COMPlus_ prefixes) in CrashDumpEnvironmentVariableProvider, including the related validation calls and unused constants.
• Add the COMPlus_ twins of the createdump/crash-report environment variables to the test infrastructure skip list.

Show a summary per file

File	Description
src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpEnvironmentVariableProvider.cs	Drops setting + validation of verbose createdump diagnostics env vars and the now-unused constants.
test/Utilities/Microsoft.Testing.TestInfrastructure/WellKnownEnvironmentVariables.cs	Adds missing COMPlus_ createdump/crash-report variables to the skip list.

Copilot's findings

• Files reviewed: 2/2 changed files
• Comments generated: 0

[Amaury Levé (Evangelink)]

Code Review — PR #8350: Stop forcing verbose createdump diagnostics in CrashDump extension

Reviewed 2 files across all 21 dimensions. The change is well-scoped and clearly justified by the 160 MB log catastrophe described in #7045.

#	Dimension	Result	Severity
1	Algorithmic Correctness	✅ LGTM	MAJOR
2	Threading & Concurrency	N/A	BLOCKING
3	Security & IPC Contract Safety	✅ LGTM	BLOCKING
4	Public API & Binary Compatibility	N/A — all internal sealed	BLOCKING
5	Performance & Allocations	✅ Positive — eliminates megabyte log floods	MAJOR
6	Cross-TFM Compatibility	✅ LGTM — existing #if !NETCOREAPP guard unaffected	MAJOR
7	Resource & IDisposable Management	N/A	MAJOR
8	Defensive Coding at Boundaries	N/A	MAJOR
9	Localization & Resources	N/A — no user-facing strings changed	MAJOR
10	Test Isolation	✅ Positive — WellKnownEnvironmentVariables gap filled for COMPlus_ prefixes	MAJOR
11	Assertion Quality	N/A	MAJOR
12	Flakiness Patterns	N/A	BLOCKING
13	Test Completeness & Coverage	✅ LGTM — removal of unconditional noise; existing tests remain valid	MAJOR
14	Data-Driven Test Coverage	N/A	MODERATE
15	Code Structure & Simplification	✅ LGTM — clean removal, no dead code left behind	MODERATE
16	Naming & Conventions	✅ LGTM	NIT
17	Documentation Accuracy	✅ LGTM — PR body is thorough	NIT
18	Analyzer & Code Fix Quality	N/A	MAJOR
19	IPC Wire Compatibility	N/A	MAJOR
20	Build Infrastructure & Dependencies	N/A	MODERATE
21	Scope & PR Discipline	✅ LGTM — minimal, focused diff	MODERATE

Notes

• Consistency between UpdateAsync and ValidateTestHostEnvironmentVariablesAsync: The two ValidateBothPrefixes calls are correctly removed to match the now-absent SetVariable calls. ✅
• WellKnownEnvironmentVariables: The DOTNET_ twins (CreateDumpDiagnostics, CreateDumpVerboseDiagnostics, CreateDumpLogToFile, EnableCrashReport, EnableCrashReportOnly) were already present; the PR symmetrically adds the missing COMPlus_ counterparts. ✅
• Opt-in path preserved: Users can still set DOTNET_CreateDumpDiagnostics=1 / DOTNET_CreateDumpVerboseDiagnostics=1 in the parent process — the extension no longer overrides them. ✅
• Pre-existing minor gap (out of scope): DOTNET_DbgMiniDumpType and DOTNET_DbgEnableElfDumpOnMacOS are still absent from the ToSkipEnvironmentVariables list while their COMPlus_ twins are present. Not introduced by this PR; worth a follow-up.

No blocking or major issues found. This is a clean, well-reasoned fix.

Generated by Expert Code Review (on open) for issue #8350 · ● 6.5M