Add unit test branch coverage for CrashDumpProcessLifetimeHandler

[Copilot]

CrashDumpProcessLifetimeHandler lacked unit tests for its enablement gating, --crash-report-if-supported informational message deduplication, and early-exit branches for graceful or HangDump-killed test host processes.

Changes

• Enablement gating (IsEnabledAsync): Added tests verifying the three-way CLI option check (--crash-dump, --crash-report, --crash-report-if-supported) combined with CrashDumpConfiguration.Enable.
• Informational message gating (BeforeTestHostProcessStartAsync):
  • Verified once-only emission under the Interlocked.Exchange guard on Windows/.NET Framework when --crash-report-if-supported is passed.
  • Verified no-op on non-Windows platforms and when --crash-report is also present.
• Early-exit paths (OnTestHostProcessExitedAsync):
  • Verified short-circuit when crash handling is not effective (no sequence file deletion, no artifact publishing).
  • Verified sequence file cleanup and artifact publication bypass when hasExitedGracefully: true or when AppDomain.CurrentDomain.GetData("ProcessKilledByHangDump") == "true".
  • Added cancellation token verification.
• Extension metadata: Added tests asserting Uid, DisplayName, Description, and Version properties.
• Test infrastructure: Updated CapturingOutputDevice to capture TextOutputDeviceData in addition to error messages.

[TestMethod]
public async Task OnTestHostProcessExitedAsync_WhenKilledByHangDump_DeletesSequenceFileAndDoesNotPublish()
{
    // Setup fake environment and sequence file
    AppDomain.CurrentDomain.SetData("ProcessKilledByHangDump", "true");
    try
    {
        await handler.OnTestHostProcessExitedAsync(new TestHostProcessInformation(..., hasExitedGracefully: false), CancellationToken.None);

        Assert.IsFalse(File.Exists(sequenceFilePath));
        Assert.IsEmpty(messageBus.Events);
    }
    finally
    {
        AppDomain.CurrentDomain.SetData("ProcessKilledByHangDump", null);
    }
}

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

[Copilot]

Copilot wasn't able to review any files in this pull request.

[Copilot]

🟡 Changes recommended

The diff does not implement any of the CrashDump tests or behavior described by the PR metadata.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

• Files reviewed: 26/26 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The HangDump test races through process-global state, and unrelated localization changes should be removed or separated.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (1)

src/Analyzers/MSTest.Analyzers/xlf/Resources.zh-Hant.xlf:766

• These analyzer localization entries (and the matching changes in the other 25 analyzer/code-fix XLF files) are unrelated to the stated CrashDump test-coverage change, and neither source .resx file is changed by this PR. Please remove the unrelated generated XLF changes or move the localization synchronization to a dedicated PR so this change remains reviewable and scoped to #11047.

      <trans-unit id="OSPlatformAttributesShouldBeConsistentDescription">
        <source>'SupportedOSPlatformAttribute' and 'UnsupportedOSPlatformAttribute' inform platform compatibility analysis but do not control MSTest execution. Test methods and test classes that use these attributes should also declare an equivalent '[OSCondition]' so unsupported tests are skipped at run time.</source>
        <target state="new">'SupportedOSPlatformAttribute' and 'UnsupportedOSPlatformAttribute' inform platform compatibility analysis but do not control MSTest execution. Test methods and test classes that use these attributes should also declare an equivalent '[OSCondition]' so unsupported tests are skipped at run time.</target>
        <note>{Locked="'SupportedOSPlatformAttribute'"}{Locked="'UnsupportedOSPlatformAttribute'"}{Locked="MSTest"}{Locked="'[OSCondition]'"}</note>

• Files reviewed: 27/27 changed files
• Comments generated: 1
• Review effort level: Balanced

[Copilot]

🟡 Changes recommended

The HangDump test introduces a parallel shared-state race, and most added scenarios duplicate existing coverage.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Review details

Suppressed comments (5)

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs:1057

• The existing BeforeTestHostProcessStartAsync_StrictCrashReportIsSet_DoesNotDisplayUnsupportedMessage test at lines 426–443 already verifies this exact option combination and outcome. Remove this duplicate test rather than maintaining the same scenario twice.

    [TestMethod]
    public async Task BeforeTestHostProcessStartAsync_WhenBothCrashReportAndIfSupportedSet_EmitsNoMessage()
    {

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs:1077

• This platform-split test duplicates BeforeTestHostProcessStartAsync_CrashReportIfSupported_DisplaysUnsupportedMessageOnce at lines 396–424, which already calls twice and verifies both the Windows/.NET Framework message and the non-Windows no-op. Remove the redundant Windows and non-Windows variants.

    [TestMethod]
    [OSCondition(ConditionMode.Include, OperatingSystems.Windows, IgnoreMessage = "Informational message is only emitted on Windows where crash reports are unsupported.")]
    public async Task BeforeTestHostProcessStartAsync_WhenIfSupportedSetOnWindows_EmitsMessageExactlyOnce()

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs:1123

• The disabled, graceful-exit, and HangDump-exit scenarios added here duplicate the existing tests at lines 445–531. Remove these duplicate methods so each branch has one authoritative test and fixture setup.

    [TestMethod]
    public async Task OnTestHostProcessExitedAsync_WhenCrashHandlingIsNotEffective_ReturnsWithoutPublishingOrDeletingSequenceFile()
    {

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs:1202

• This test mutates process-wide AppDomain state while the assembly enables method-level parallelization (Program.cs:11). Unlike the existing equivalent test at lines 500–531, it is not marked [DoNotParallelize] and resets the key to null rather than restoring its prior value, so concurrent exit tests can be forced down the HangDump branch. Remove the duplicate, or add isolation and preserve/restore the original value.

    public async Task OnTestHostProcessExitedAsync_WhenProcessKilledByHangDump_DeletesSequenceFileAndDoesNotPublish()
    {
        string tempDir = Directory.CreateDirectory(Path.Combine(Path.GetTempPath(), "crashdump-tests-" + Guid.NewGuid().ToString("N"))).FullName;
        try
        {
            AppDomain.CurrentDomain.SetData("ProcessKilledByHangDump", "true");

test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs:1241

• OnTestHostProcessExitedAsync_PreCancelledToken_ThrowsBeforeHandlingExit at lines 533–562 already verifies this exception and additionally proves cancellation happens before sequence-file deletion or publishing. Remove this weaker duplicate test.

    [TestMethod]
    public async Task OnTestHostProcessExitedAsync_WhenCancellationRequested_ThrowsOperationCanceledException()
    {

• Files reviewed: 1/1 changed files
• Comments generated: 1
• Review effort level: Balanced

[github-actions]

🧪 Expert test review — PR #11087

Grade	Test	Mutation	Notes	How to improve
C (70–79)	new CrashDumpTests. OnTestHostProcessExitedAsync_ WhenProcessKilledByHangDump_ DeletesSequenceFileAndDoesNotPublish	2/3 killed	Mutates process-wide AppDomain data without [DoNotParallelize], risking cross-test flakiness.	Add [DoNotParallelize] like the sibling test at line 501 that sets the same flag.
B (80–89)	new CrashDumpTests. OnTestHostProcessExitedAsync_ WhenCrashHandlingIsNotEffective_ ReturnsWithoutPublishingOrDeletingSequenceFile	2/2 killed	Assertions correctly verify the early-return short-circuit; cleanup uses a swallow-all catch.	Narrow the finally catch to IOException/UnauthorizedAccessException.
B (80–89)	new CrashDumpTests. OnTestHostProcessExitedAsync_ WhenHasExitedGracefully_ DeletesSequenceFileAndDoesNotPublish	2/2 killed	Correctly verifies the graceful-exit deletion path; cleanup uses a swallow-all catch.	Narrow the finally catch to IOException/UnauthorizedAccessException.
A (90–100)	new CrashDumpTests. IsEnabledAsync_ WhenOptionIsSetAndConfigurationEnabled_ ReturnsTrue	2/2 killed	Data-driven across all three option names; kills both the option-check and the && logic.	—
A (90–100)	new CrashDumpTests. IsEnabledAsync_ WhenOptionIsSetAndConfigurationDisabled_ ReturnsFalse	2/2 killed	Mirrors the enabled case to cover the Enable=false branch.	—
A (90–100)	new CrashDumpTests. IsEnabledAsync_ WhenNoOptionIsSet_ ReturnsFalse	2/2 killed	The configEnable=true row kills an &&↔|| flip on the option check.	—
A (90–100)	new CrashDumpTests. BeforeTestHostProcessStartAsync_ WhenIfSupportedNotSet_ EmitsNoMessage	1/1 killed	Verifies the guard clause short-circuits before any output is displayed.	—
A (90–100)	new CrashDumpTests. BeforeTestHostProcessStartAsync_ WhenBothCrashReportAndIfSupportedSet_ EmitsNoMessage	1/1 killed	Covers the combined-flag branch of the guard clause distinct from the single-flag case.	—
A (90–100)	new CrashDumpTests. BeforeTestHostProcessStartAsync_ WhenIfSupportedSetOnWindows_ EmitsMessageExactlyOnce	2/2 killed	Calling the method twice and asserting a single message kills the Interlocked.Exchange idempotency guard.	—
A (90–100)	new CrashDumpTests. BeforeTestHostProcessStartAsync_ WhenIfSupportedSetOnNonWindows_ EmitsNoMessage	1/1 killed	Complements the Windows case to cover the OS-conditional branch.	—
A (90–100)	new CrashDumpTests. OnTestHostProcessExitedAsync_ WhenCancellationRequested_ ThrowsOperationCanceledException	1/1 killed	Uses ThrowsExactlyAsync against a pre-canceled token, precisely matching the production ThrowIfCancellationRequested call.	—
A (90–100)	new CrashDumpTests. CrashDumpProcessLifetimeHandler_ MetadataProperties_ ReturnExpectedValues	0/0 (no meaningful points)	Asserts every metadata property against its expected resource/constant value.	—

Note: three additional high-confidence findings on these same tests (a missing [DoNotParallelize] on the HangDump test, and generic catch { } cleanup blocks in two other tests) already have live inline review comments from other bots on this PR, so no duplicate inline suggestions were posted for them here — see the table's "How to improve" column for a summary of each.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 91.4 AIC · ⌖ 7.23 AIC · ⊞ 17K · [◷]( · ◷)

[Copilot]

🟢 Approval recommended

The added tests correctly cover distinct behavior without introducing production changes.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

🧪 Expert test review — PR #11087

Grade	Test	Mutation	Notes	How to improve
B (80–89)	new CrashDumpTests. BeforeTestHostProcessStartAsync_ WhenIfSupportedNotSet_ EmitsNoMessage	2/2 killed	Correctly verifies the early-return branch when no crash-report option is set; missing .ConfigureAwait(false) is the only nit.	Add .ConfigureAwait(false) to the awaited call, matching every other async test in this file.
A (90–100)	new CrashDumpTests. CrashDumpProcessLifetimeHandler_ MetadataProperties_ ReturnExpectedValues	6/6 killed	Exercises every metadata property (Uid, Version, DisplayName, Description, DataTypesProduced) against real production constants.	—

Both new tests target CrashDumpProcessLifetimeHandler. The BeforeTestHostProcessStartAsync_WhenIfSupportedNotSet_EmitsNoMessage test is a valid, non-duplicate addition covering the early-return path (--crash-report-if-supported not set), distinct from the two adjacent existing tests that cover the "if-supported set" and "strict crash-report set" branches. The _MetadataProperties_ test adds solid coverage of the simple property getters and DataTypesProduced, none of which were previously asserted directly. No correctness, reliability, or duplication issues were found beyond the noted ConfigureAwait nit.

_{This advisory comment was generated automatically. Grades are heuristic
and informational — they do not block merging. Suggestions on the Files
changed tab can be applied with one click. Re-run with
/review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 96.5 AIC · ⌖ 2.61 AIC · ⊞ 17K · [◷]( · ◷)

[github-actions]

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 96.5 AIC · ⌖ 2.61 AIC · ⊞ 17K · ◷

[github-actions]

🧵 Parallel-safety audit — PR #11087

Nothing audited here touches process-global state, shared filesystem paths, or [ResourceLock] / [DoNotParallelize] declarations. The two new tests each construct their own local CrashDumpProcessLifetimeHandler, RecordingMessageBus, and output-device instances (no statics, env vars, CWD, or shared paths involved). Nothing to flag for parallel-safety.

Audited Microsoft.Testing.Extensions.UnitTests at scope MethodLevel, workers CPU count (via [assembly: Parallelize(Scope = ExecutionScope.MethodLevel, Workers = 0)] in Program.cs).

_{Re-run with /parallel-audit.}

🤖 Automated content by GitHub Copilot. Generated by the Parallel-safety audit on PR (on open / sync) workflow. · auto · 37.4 AIC · ⌖ 3.43 AIC · ⊞ 24.9K · [◷]( · ◷)

[Copilot]

🟢 Approval recommended

The remaining tests cover distinct behavior and introduce no unresolved correctness issues.

Review details

• Files reviewed: 1/1 changed files
• Comments generated: 0 new
• Review effort level: Balanced

[github-actions]

🧪 Expert test review — PR #11087

Reviewed the two test methods newly added in the diff (lines 977-1009 of test/UnitTests/Microsoft.Testing.Extensions.UnitTests/CrashDumpTests.cs). Both are focused, single-behavior tests with no anti-patterns, reliability, or convention issues. No inline suggestions were warranted.

Grade	Test	Mutation	Notes	How to improve
A (90–100)	new CrashDumpTests. BeforeTestHostProcessStartAsync_ WhenIfSupportedNotSet_ EmitsNoMessage	1/1 killed	Verifies the early-return guard when `--crash-report-if-supported` is not set; removing the guard would fail the assertion.	—
A (90–100)	new CrashDumpTests. CrashDumpProcessLifetimeHandler_ MetadataProperties_ ReturnExpectedValues	0/0 (no meaningful points)	Asserts all five metadata pass-through properties against their expected resource/version values.	—

_{This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Suggestions on the Files changed tab can be applied with one click. Re-run with /review-tests.}

🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 141.3 AIC · ⌖ 2.49 AIC · ⊞ 17K · [◷]( · ◷)

[github-actions]

Final resolution for Azure Pipelines build [1586701]((dev.azure.com/redacted) (microsoft.testfx, completed).

No test-product failures, retries, or flaky results were found. All 91,060 normalized test records (CTRF/TRX/JUnit) across the matrix report passed (90,650) or skipped (410) — failureOrRetryCount: 0, timelineSignalCount: 0, diagnosticCount: 0.

The build's overall failed status stems from the Linux Release job, which was cancelled due to a worker timeout (Job cancelled due to worker timeout, and the Test task step reporting Bash exited with code '7' as a consequence of that cancellation) — this is an infrastructure/agent-level failure, not a test defect, and falls outside this agent's scope (test failures, flakiness, crash/hang, duration regressions). No crash/hang diagnostics or dumps were collected (diagnosticCount: 0, diagnostics.json empty).

No durable issue threshold is met: this is a single-run infrastructure timeout isolated to this PR's build, with no evidence of a recurring or code-level defect.

Next step: if the Linux Release leg times out again on a rebuild, it may indicate a worker/agent capacity or hang issue worth escalating to pipeline infrastructure owners; re-run the PR build to confirm.

🤖 Automated content by GitHub Copilot. Generated by the Pipeline Test Triage workflow. · auto · 34.6 AIC · ⌖ 1.75 AIC · ⊞ 9.3K · [◷]( · ◷)