[quality-improver] CrashDumpProcessLifetimeHandler: enablement gating, `--crash-report-if-supported` no-op path, and graceful/HangDump exit branche [Content truncated due to length]

[github-actions]

🎯 Repository Quality Improvement Report — CrashDump lifetime-handler branch coverage gap

Analysis Date: 2026-09-04
Focus Area: crashdump-lifetime-handler-branch-coverage-gap
Strategy Type: Custom

Executive Summary

CrashDumpProcessLifetimeHandler (src/Platform/Microsoft.Testing.Extensions.CrashDump/CrashDumpProcessLifetimeHandler.cs) is the extension entry point that decides whether crash-dump/crash-report collection is active and short-circuits publishing when the host exited cleanly or was already handled by HangDump. test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs only exercises the "unhealthy exit → publish a dump" happy path (OnTestHostProcessExitedAsync_*) plus the static regex/directory helpers. It never calls IsEnabledAsync, BeforeTestHostProcessStartAsync, or OnTestHostProcessExitedAsync with a graceful exit or a HangDump-killed process, so several independent boolean/branch decisions in the handler are entirely unverified by unit tests.

Specifically untested:

• IsEnabledAsync() — the three-way OR across --crash-dump / --crash-report / --crash-report-if-supported command-line options combined with CrashDumpConfiguration.Enable. No test asserts any of the 2^4 relevant combinations return false/true correctly.
• BeforeTestHostProcessStartAsync — the best-effort --crash-report-if-supported informational-message path, including the Interlocked.Exchange guard that must emit the message exactly once, and the platform-conditional (NETCOREAPP/Windows vs. .NET Framework) message selection.
• OnTestHostProcessExitedAsync early-return branches: IsCrashHandlingEffective() returning false (handling disabled) and the graceful-exit / AppDomain.CurrentDomain.GetData("ProcessKilledByHangDump") == "true" branch that deletes the sequence file and skips publishing instead of calling _artifactPublisher.PublishAsync.

This matters because these are the gating decisions controlling whether crash artifacts are collected at all — a silent regression here (e.g., IsEnabledAsync always returning true/false, or the graceful-exit short-circuit accidentally firing for crashed hosts) would not be caught by any existing unit test, and would likely only surface as flaky/missing dumps in the field.

Full Analysis Report

Focus Area: CrashDump lifetime-handler branch coverage gap

Current State Assessment

Metrics Collected:

Metric	Value	Status
CrashDumpProcessLifetimeHandler public/internal methods	6 (IsEnabledAsync, BeforeTestHostProcessStartAsync, OnTestHostProcessStartedAsync, OnTestHostProcessExitedAsync, static helpers)	⚠️
Methods with direct unit test coverage	OnTestHostProcessStartedAsync/OnTestHostProcessExitedAsync (unhealthy-exit path only), static regex/directory helpers	⚠️
IsEnabledAsync unit tests	0	❌
BeforeTestHostProcessStartAsync unit tests	0	❌
Graceful-exit / HangDump-killed branch unit tests	0	❌

Findings

Strengths

• The unhealthy-exit dump-publishing path (multiple placeholder patterns, PID matching, false-positive-warning suppression) is well covered by 5 existing tests in CrashDumpTests.cs.
• Regex/filename-pattern helpers (BuildDumpFileNameRegex, GetDumpDirectory, etc.) have solid coverage.

Areas for Improvement

• ❌ High severity: IsEnabledAsync() combinational logic is completely untested — a change to the option-OR logic or the CrashDumpConfiguration.Enable gate would not be caught.
• ❌ Medium severity: BeforeTestHostProcessStartAsync's "emit informational message only once" (Interlocked.Exchange) and platform-specific message branch are untested — a regression could spam duplicate messages or pick the wrong platform string.
• ❌ Medium severity: OnTestHostProcessExitedAsync's early-return for IsCrashHandlingEffective() == false and for graceful/HangDump-killed exits are untested — a bug here could suppress dumps that should be collected, or (worse) trigger unnecessary publishing/deletion.

---

🤖 Suggested Improvement Tasks

The following actionable tasks address the findings above. All can be added to test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs, reusing the existing fake ICommandLineOptions/IMessageBus/IOutputDevice/CrashDumpConfiguration test doubles already present in that file.

Task 1: Add IsEnabledAsync coverage for each option/config combination

Priority: High
Estimated Effort: Small

Add tests asserting IsEnabledAsync() returns true when any of --crash-dump, --crash-report, or --crash-report-if-supported is set and CrashDumpConfiguration.Enable is true, and false when none are set, and false when CrashDumpConfiguration.Enable is false even if an option is set.

---

Task 2: Test BeforeTestHostProcessStartAsync's once-only informational message

Priority: Medium
Estimated Effort: Small

Add a test that sets only --crash-report-if-supported, calls BeforeTestHostProcessStartAsync twice, and asserts the informational message is emitted exactly once (verifying the Interlocked.Exchange guard on _ifSupportedIgnoredMessageEmitted). Also add a test asserting no message is emitted when --crash-report is also set (per the early-return guard).

---

Task 3: Test OnTestHostProcessExitedAsync when crash handling is not effective

Priority: Medium
Estimated Effort: Small

Add a test where no crash-dump/report option is set (so IsCrashHandlingEffective() is false) and assert OnTestHostProcessExitedAsync returns without publishing any artifact and without deleting the sequence file.

---

Task 4: Test the graceful-exit and HangDump-killed short-circuit

Priority: High
Estimated Effort: Small

Add a test with hasExitedGracefully: true and a separate test with AppDomain.CurrentDomain.SetData("ProcessKilledByHangDump", "true") set before calling OnTestHostProcessExitedAsync with hasExitedGracefully: false. Assert in both cases that _sequenceFileHandler.TryDelete()-equivalent behavior occurs (no crash dump is published) — e.g., assert the message bus/output device did not receive a FileArtifact publish call. Remember to reset the AppDomain data after the test to avoid cross-test leakage.

---

Task 5: Add a cancellation test for OnTestHostProcessExitedAsync

Priority: Low
Estimated Effort: Small

Add a test asserting OnTestHostProcessExitedAsync throws OperationCanceledException when invoked with an already-cancelled CancellationToken, verifying the cancellationToken.ThrowIfCancellationRequested() guard at the top of the method.

---

📊 Historical Context

This is the 51st run of the repository quality improvement analysis, following a recent streak of unit-test-coverage-gap findings across MTP extensions (Retry, HangDump, TrxReport, GitHubActionsReport). This continues that pattern for the CrashDump extension, targeting a previously unexamined component.

---

🎯 Recommendations

Immediate Actions (This Week)

1. Add IsEnabledAsync and graceful/HangDump-exit branch tests (Tasks 1 & 4) — Priority: High

Short-term Actions (This Month)

1. Add the remaining BeforeTestHostProcessStartAsync and cancellation tests (Tasks 2, 3, 5) — Priority: Medium

Next analysis: 2026-09-05 — Focus area selected based on diversity algorithm

🤖 Automated content by GitHub Copilot. Generated by the Repository Quality Improver workflow. · auto · 53.9 AIC · ⌖ 10.4 AIC · ⊞ 16.8K · [◷]( · ◷)

_{Add this agentic workflow to your repo}

To install this agentic workflow, run

gh aw add githubnext/agentics/workflows/repository-quality-improver.md@main

• expires on Sep 6, 2026, 10:32 PM UTC