Add regression test for MTP controller-only handshake exit path

[Copilot]

Bug Fix

dotnet test (MTP path) could surface a misleading KeyNotFoundException when only the test host controller handshook and the process exited before a test host handshake. The exception obscured the real process failure and lacked explicit regression coverage after the original fix.

• What was the bug?

  The exit path could reach assembly-completion logic for an execution id that never had AssemblyRunStarted, triggering dictionary lookup failure (KeyNotFoundException) instead of reporting handshake/process failure semantics.

• How did you fix it?

  Added focused regression coverage in TestApplicationHandlerTests for the controller-handshake-only scenario:

  • Simulate a handshake with HostType = "TestHostController" (no TestHost handshake).
  • Invoke OnTestProcessExited(...).
  • Assert no exception is thrown and handshake failure is reported.

• Testing

  Added a new unit/regression test case in test/dotnet.Tests/CommandTests/Test/TestApplicationHandlerTests.cs to lock this behavior.

handler.OnHandshakeReceived(
    new HandshakeMessage(new Dictionary<byte, string>
    {
        [HandshakeMessagePropertyNames.ExecutionId] = "execution-id",
        [HandshakeMessagePropertyNames.HostType] = "TestHostController",
        [HandshakeMessagePropertyNames.Framework] = ".NET 9.0.0",
        [HandshakeMessagePropertyNames.Architecture] = "x64",
    }),
    gotSupportedVersion: true);

Action act = () => handler.OnTestProcessExited(exitCode: 1, outputData: "out", errorData: "err");

act.Should().NotThrow();
output.HasHandshakeFailure.Should().BeTrue();

[Copilot]

Pull request overview

Adds regression coverage for the MTP dotnet test controller-only handshake exit path so process exit handling reports handshake failure instead of surfacing an unrelated exception.

Changes:

• Adds a focused TestApplicationHandlerTests test for TestHostController handshake without TestHost.
• Includes an incidental SARIF file encoding/newline change that should be removed.

Show a summary per file

File	Description
test/dotnet.Tests/CommandTests/Test/TestApplicationHandlerTests.cs	Adds the regression test for controller-only handshake exit handling.
src/Microsoft.CodeAnalysis.NetAnalyzers/src/Microsoft.CodeAnalysis.NetAnalyzers.sarif	Contains an unrelated generated asset encoding/newline change.

Copilot's findings

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

[Evangelink]

Code review using two complementary lenses: (1) general dotnet/sdk conventions, and (2) the testfx expert-reviewer 21-dimension methodology.

Verification

• Built dotnet.Tests.csproj clean (0 warn / 0 err) on this branch.
• Ran the new test in isolation: Total: 1, Failed: 0.
• Traced production code in TestApplicationHandler.OnTestProcessExited (lines 270–284) and TerminalTestReporter.AssemblyRunCompleted (line 734, _assemblies[executionId]) — confirmed the test exactly exercises the HandshakeFailure branch that the original fix introduced, and would fail if that guard were removed.

Strengths

• Single concern, good scope discipline (the unrelated NetAnalyzers.sarif drift was explicitly reverted in 16b97f0).
• File location, file-scoped namespace, FluentAssertions + Moq usage all match TestProgressStateTests.cs and the rest of the folder.
• Internals reachable via InternalsVisibleTo("dotnet.Tests", …) in src/Cli/dotnet/Properties/AssemblyInfo.cs. Global usings (Xunit, FluentAssertions) are leveraged correctly.

Expert-reviewer dimension summary (test-only PR — most dimensions are N/A)

#	Dimension	Verdict
13	Test Completeness & Coverage	🟡 1 MODERATE
17	Documentation Accuracy	🟢 1 NIT

✅ 19/21 dimensions clean (10 Test Isolation, 11 Assertion Quality, 12 Flakiness, 15 Code Structure, 16 Naming, 21 Scope all clean; the rest are N/A for a test-only change).

• Dim 13 — Test Completeness: only the negative branch of OnTestProcessExited is covered. Add a companion [Fact] for HostType = "TestHost" → AssemblyRunCompleted (no throw, HasHandshakeFailure == false) to lock in the positive branch.
• Dim 17 — Documentation: add a one-line comment back-referencing the originating issue / fix PR (the test name carries intent, but an explicit // Regression for #… makes triage cheaper).

Pre-existing follow-up (out of scope here)
The comment block at src/Cli/dotnet/Commands/Test/MTP/TestApplicationHandler.cs:273-276 is inside the AssemblyRunCompleted branch but reads as if it belonged to the HandshakeFailure branch — its phrasing inverts the actual flow. Worth a cleanup commit later.

Verdict: No BLOCKING findings → COMMENT (not REQUEST_CHANGES). Safe to merge as-is; both findings above are quality nits.

[Evangelink]

[NIT] Documentation Accuracy (dim 17)

The test name carries the intent, but adding a one-line back-reference to the originating issue or fix PR makes future triage cheaper. Suggested:

// Regression for <issue/PR>: previously `OnTestProcessExited` could throw `KeyNotFoundException`
// when only a `TestHostController` handshake was received before the process exited.
[Fact]
public void OnTestProcessExited_WithOnlyControllerHandshake_ReportsHandshakeFailure()

[Evangelink]

[MODERATE] Test Completeness & Coverage (dim 13)

The new test covers only the else branch of OnTestProcessExited (controller-only → HandshakeFailure). Consider adding a companion [Fact] for the positive branch so a future change that breaks the normal AssemblyRunStarted/AssemblyRunCompleted pairing is also caught here:

[Fact]
public void OnTestProcessExited_WithTestHostHandshake_ReportsAssemblyRunCompleted()
{
    // ... same setup ...
    handler.OnHandshakeReceived(
        new HandshakeMessage(new Dictionary<byte, string>
        {
            [HandshakeMessagePropertyNames.ExecutionId] = "execution-id",
            [HandshakeMessagePropertyNames.Architecture] = "x64",
            [HandshakeMessagePropertyNames.Framework] = ".NET 9.0.0",
            [HandshakeMessagePropertyNames.HostType] = "TestHost",
            [HandshakeMessagePropertyNames.InstanceId] = "instance-id",
        }),
        gotSupportedVersion: true);

    Action act = () => handler.OnTestProcessExited(exitCode: 0, outputData: "", errorData: "");

    act.Should().NotThrow();
    output.HasHandshakeFailure.Should().BeFalse();
}

This is a quality nit, not a blocker for this PR — feel free to follow up separately.