[test-improver] Add unit tests for DotnetTestConnection.StartServerControlChannelAsync guard conditions

Goal and rationale

DotnetTestConnection.StartServerControlChannelAsync gates the server-initiated-cancel control channel behind a multi-condition early-return guard (_transportKind != NamedPipe, browser/WASI, !IsServerControlChannelSupported, empty _serverControlPipeName, or an already-installed _serverControlPipeClient). Only the Http-transport short-circuit branch had a direct test (IsCompatibleProtocolAsync_HttpDoesNotEnableControlPipeAndInvalidFlagsRemainFalse); the three named-pipe-path guard terms were untested, so a regression in any one of them (e.g. connecting with an empty pipe name, replacing an already-running listener, or starting the channel without a negotiated capability) could slip in silently.

Note: this repeats the goal of a run logged in repo memory on 2026-10-16, whose reported PR never actually landed on main (a recurring "orphaned PR" discrepancy this agent has hit before) — this PR supersedes that attempt with freshly authored and verified tests.

Approach

Added 3 [TestMethod]s to DotnetTestConnectionTests.cs:

StartServerControlChannelAsync_EmptyPipeNameDespiteNegotiatedSupportDoesNotConnect — capability negotiated but pipe name left empty.
StartServerControlChannelAsync_AlreadyStartedDoesNotReplaceTheExistingClient — a control-pipe client is already installed; the call must be a no-op and must not replace it.
StartServerControlChannelAsync_CapabilityNotNegotiatedDoesNotConnectEvenWithAPipeName — a stray non-empty pipe name alone (without IsServerControlChannelSupported) must not start the channel.

All three assert the callback is never invoked and that _serverControlPipeClient/_serverControlListenerTask stay null (or, for the "already started" case, stay exactly the pre-existing instance).

Gotcha carried over from prior runs: NamedPipeClient is [Embedded] and compiled as separately linked source into the production and test assemblies, so it is not type-identical across them even with InternalsVisibleTo — constructing one directly in the test (new NamedPipeClient(...)) throws at the field-assignment boundary. The "already started" test instead builds its sentinel via Activator.CreateInstance against typeof(DotnetTestConnection).Assembly.GetType("...NamedPipeClient"), mirroring the existing ConnectionClient reflection pattern already used in this file for the embedded transport-client contract.

Coverage impact

Suite	Before	After
`Microsoft.Testing.Platform.UnitTests` (net8.0)	3270	3273
`Microsoft.Testing.Platform.UnitTests` (net9.0)	3271	3274

Trade-offs

Small, additive test file change only — no production code touched. Reflection-based field/property access adds minor maintenance coupling to DotnetTestConnection's private member names, consistent with the existing style of this test file.

Reproducibility

./build.sh -restore
export PATH="$PWD/.dotnet:$PATH"
dotnet build test/UnitTests/Microsoft.Testing.Platform.UnitTests/Microsoft.Testing.Platform.UnitTests.csproj -c Debug -f net8.0
./artifacts/bin/Microsoft.Testing.Platform.UnitTests/Debug/net8.0/Microsoft.Testing.Platform.UnitTests --report-trx --results-directory /tmp/trx
dotnet format whitespace TestFx.slnx --verify-no-changes --include test/UnitTests/Microsoft.Testing.Platform.UnitTests/ServerMode/DotnetTestConnectionTests.cs

Test Status

Build succeeded (0 warnings) on net8.0 and net9.0. Full Microsoft.Testing.Platform.UnitTests suite: 3273 total net8.0 (was 3270), 3274 total net9.0 (was 3271), 0 failed, 22 skipped (pre-existing), no regressions. dotnet format whitespace --verify-no-changes clean (only the expected harmless F#-project warning).

Effectiveness evidence

Behavior and inputs	Independent expectation and source	Wrong implementation and distinguishing assertion	Real path and boundary/locale coverage	Evidence
Empty pipe name despite `IsServerControlChannelSupported=true`	Per the method's own doc comment and guard clause, an empty/null pipe name must short-circuit regardless of the negotiated capability flag	Dropping the `RoslynString.IsNullOrEmpty(_serverControlPipeName)` guard term: test asserts callback never invoked and `_serverControlPipeClient`/`_serverControlListenerTask` stay null	Exercises the real `StartServerControlChannelAsync` entry point directly (not mocked)	Executed red/green (see below)
Already-installed `_serverControlPipeClient`	Idempotency contract documented in the method's XML remarks ("invoked once per connection... the single-threaded lifecycle makes the plain fields safe") — a second call must not replace the existing client	Dropping the `_serverControlPipeClient is not null` guard term: test asserts `AreSame(existingClient, ...)` after the call, which a replaced-instance implementation would fail	Real entry point; sentinel built via reflection against the production assembly's own `[Embedded]` `NamedPipeClient` type (not a test-only stand-in with different semantics)	Executed red/green (see below)
Pipe name present but `IsServerControlChannelSupported=false`	A stray/leftover pipe-name field alone must not be sufficient to start the channel — the capability flag is the actual gate per the handshake code that sets both together	Dropping the `!IsServerControlChannelSupported` guard term: test asserts callback never invoked and no client installed	Real entry point	Executed red/green (see below)

Executed verification: Simultaneously removed all three guard terms from StartServerControlChannelAsync (collapsing the condition to just transport-kind/OS checks). Rebuilt (0 errors, 1 expected nullability warning from the now-unguarded NamedPipeClient construction) and ran the full suite: exactly 3/3273 failed — all three new tests, at the intended assertions (ArgumentException-adjacent null-pipe-name failure surfaced as a downstream AreSame/IsNull mismatch, the "already started" AreSame reference-inequality failure, and the "not negotiated" IsNull failure). No other test was affected. Reverted the fault (git diff on the production file confirmed empty afterward) and reran: 0/3273 failed on net8.0, 0/3274 on net9.0, 22 pre-existing skips on each, no regressions.

Note

This was originally intended as a pull request, but the git push operation failed.

Original error: The process '/usr/bin/git' failed with exit code 128

Workflow Run: View run details and download bundle artifact

The bundle file is available in the agent artifact in the workflow run linked above.

Create the pull request manually

# Download the artifact from the workflow run
gh run download '39076994810' -n agent -D '/tmp/agent-39076994810'

# Resolve the bundle source ref, fetch it into a temporary ref, then create the local branch
bundle_path='/tmp/agent-39076994810/aw-test-assist-dotnet-test-connection-control-channel-guards-2.bundle'
temp_ref='refs/bundles/create-pr-test-assist-dotnet-test-connection-control-channel-guards-2-27ea89f4be146987-412f0a6a'
target_ref='refs/heads/test-assist/dotnet-test-connection-control-channel-guards-2-27ea89f4be146987'
bundle_source_ref=$(git bundle list-heads "$bundle_path" | awk '$2 ~ /^refs\/heads\// { print $2 }')
if [ -z "$bundle_source_ref" ]; then
  bundle_source_ref=$(git bundle list-heads "$bundle_path" | awk '$2 == "HEAD" { print $2 }')
fi
if [ "$(printf '%s\n' "$bundle_source_ref" | sed '/^$/d' | wc -l | tr -d ' ')" != "1" ]; then
  echo "Expected exactly one bundle source ref, found: $bundle_source_ref" >&2
  exit 1
fi
git fetch "$bundle_path" "${bundle_source_ref}:${temp_ref}"
git update-ref "$target_ref" "$temp_ref"
git checkout 'test-assist/dotnet-test-connection-control-channel-guards-2-27ea89f4be146987'
# Ensure the working tree matches the updated branch
git reset --hard
# Remove the temporary bundle ref
git update-ref -d "$temp_ref"

# Push the branch to the target remote
git push 'origin' 'test-assist/dotnet-test-connection-control-channel-guards-2-27ea89f4be146987'

# Create the pull request
gh pr create --title '[test-improver] Add unit tests for DotnetTestConnection.StartServerControlChannelAsync guard conditions' --base 'main' --head 'test-assist/dotnet-test-connection-control-channel-guards-2-27ea89f4be146987' --repo 'microsoft/testfx'

Warning

Firewall blocked 92 domains

The following domains were blocked by the firewall during workflow execution:

0t3vsblobprodcus362.vsblob.vsassets.io
11vvsblobprodcus336.vsblob.vsassets.io
1javsblobprodcus364.vsblob.vsassets.io
1k9vsblobprodcus379.vsblob.vsassets.io
1oavsblobprodcus350.vsblob.vsassets.io
1p7vsblobprodcus324.vsblob.vsassets.io
1s1vsblobprodcus386.vsblob.vsassets.io
2kmvsblobprodcus39.vsblob.vsassets.io
2zrvsblobprodcus388.vsblob.vsassets.io
37bvsblobprodcus311.vsblob.vsassets.io
37cvsblobprodcus359.vsblob.vsassets.io
4m6vsblobprodcus384.vsblob.vsassets.io
4myvsblobprodcus32.vsblob.vsassets.io
4vyvsblobprodcus361.vsblob.vsassets.io
4zjvsblobprodcus390.vsblob.vsassets.io
51yvsblobprodcus36.vsblob.vsassets.io
5dkvsblobprodcus355.vsblob.vsassets.io
5rqvsblobprodcus385.vsblob.vsassets.io
6s7vsblobprodcus313.vsblob.vsassets.io
7k6vsblobprodcus337.vsblob.vsassets.io
7tjvsblobprodcus341.vsblob.vsassets.io
80zvsblobprodcus35.vsblob.vsassets.io
8xbvsblobprodcus382.vsblob.vsassets.io
96bvsblobprodcus338.vsblob.vsassets.io
ajhvsblobprodcus363.vsblob.vsassets.io
bcnvsblobprodcus378.vsblob.vsassets.io
builds.dotnet.microsoft.com
c50vsblobprodcus330.vsblob.vsassets.io
c78vsblobprodcus322.vsblob.vsassets.io
cflvsblobprodcus383.vsblob.vsassets.io
ci.dot.net
ckzvsblobprodcus347.vsblob.vsassets.io
d0svsblobprodcus381.vsblob.vsassets.io
data.nuget.org
dlbvsblobprodcus316.vsblob.vsassets.io
e7bvsblobprodcus348.vsblob.vsassets.io
fdpvsblobprodcus345.vsblob.vsassets.io
frdvsblobprodcus327.vsblob.vsassets.io
gbsvsblobprodcus365.vsblob.vsassets.io
gervsblobprodcus329.vsblob.vsassets.io
h6tvsblobprodcus346.vsblob.vsassets.io
i1qvsblobprodcus353.vsblob.vsassets.io
ibzvsblobprodcus369.vsblob.vsassets.io
imzvsblobprodcus368.vsblob.vsassets.io
jd4vsblobprodcus366.vsblob.vsassets.io
josvsblobprodcus372.vsblob.vsassets.io
jrqvsblobprodcus343.vsblob.vsassets.io
k0ivsblobprodcus356.vsblob.vsassets.io
k4kvsblobprodcus344.vsblob.vsassets.io
kgfvsblobprodcus314.vsblob.vsassets.io
kh4vsblobprodcus325.vsblob.vsassets.io
kijvsblobprodcus387.vsblob.vsassets.io
kmuvsblobprodcus389.vsblob.vsassets.io
kxqvsblobprodcus376.vsblob.vsassets.io
l49vsblobprodcus358.vsblob.vsassets.io
l7avsblobprodcus319.vsblob.vsassets.io
ljcvsblobprodcus317.vsblob.vsassets.io
lylvsblobprodcus31.vsblob.vsassets.io
m16vsblobprodcus374.vsblob.vsassets.io
m6xvsblobprodcus342.vsblob.vsassets.io
m8dvsblobprodcus37.vsblob.vsassets.io
mfjvsblobprodcus373.vsblob.vsassets.io
n3kvsblobprodcus335.vsblob.vsassets.io
nn8vsblobprodcus340.vsblob.vsassets.io
o3svsblobprodcus318.vsblob.vsassets.io
ofvvsblobprodcus315.vsblob.vsassets.io
p2ovsblobprodcus312.vsblob.vsassets.io
pc2vsblobprodcus360.vsblob.vsassets.io
pdfvsblobprodcus380.vsblob.vsassets.io
pe3vsblobprodcus354.vsblob.vsassets.io
pe4vsblobprodcus351.vsblob.vsassets.io
pkgs.dev.azure.com
pkvvsblobprodcus321.vsblob.vsassets.io
rcxvsblobprodcus328.vsblob.vsassets.io
s4uvsblobprodcus326.vsblob.vsassets.io
s8mvsblobprodcus38.vsblob.vsassets.io
se1vsblobprodcus349.vsblob.vsassets.io
sqdvsblobprodcus333.vsblob.vsassets.io
st8vsblobprodcus339.vsblob.vsassets.io
tphvsblobprodcus375.vsblob.vsassets.io
u3hvsblobprodcus371.vsblob.vsassets.io
u6ovsblobprodcus377.vsblob.vsassets.io
ukkvsblobprodcus352.vsblob.vsassets.io
uy6vsblobprodcus34.vsblob.vsassets.io
v53vsblobprodcus320.vsblob.vsassets.io
vb4vsblobprodcus33.vsblob.vsassets.io
vwvvsblobprodcus334.vsblob.vsassets.io
x3yvsblobprodcus370.vsblob.vsassets.io
xupvsblobprodcus332.vsblob.vsassets.io
yluvsblobprodcus367.vsblob.vsassets.io
yttvsblobprodcus357.vsblob.vsassets.io
ytvvsblobprodcus310.vsblob.vsassets.io

To allow these domains, add them to the network.allowed list in your workflow frontmatter:

network:
  allowed:
    - defaults
    - "0t3vsblobprodcus362.vsblob.vsassets.io"
    - "11vvsblobprodcus336.vsblob.vsassets.io"
    - "1javsblobprodcus364.vsblob.vsassets.io"
    - "1k9vsblobprodcus379.vsblob.vsassets.io"
    - "1oavsblobprodcus350.vsblob.vsassets.io"
    - "1p7vsblobprodcus324.vsblob.vsassets.io"
    - "1s1vsblobprodcus386.vsblob.vsassets.io"
    - "2kmvsblobprodcus39.vsblob.vsassets.io"
    - "2zrvsblobprodcus388.vsblob.vsassets.io"
    - "37bvsblobprodcus311.vsblob.vsassets.io"
    - "37cvsblobprodcus359.vsblob.vsassets.io"
    - "4m6vsblobprodcus384.vsblob.vsassets.io"
    - "4myvsblobprodcus32.vsblob.vsassets.io"
    - "4vyvsblobprodcus361.vsblob.vsassets.io"
    - "4zjvsblobprodcus390.vsblob.vsassets.io"
    - "51yvsblobprodcus36.vsblob.vsassets.io"
    - "5dkvsblobprodcus355.vsblob.vsassets.io"
    - "5rqvsblobprodcus385.vsblob.vsassets.io"
    - "6s7vsblobprodcus313.vsblob.vsassets.io"
    - "7k6vsblobprodcus337.vsblob.vsassets.io"
    - "7tjvsblobprodcus341.vsblob.vsassets.io"
    - "80zvsblobprodcus35.vsblob.vsassets.io"
    - "8xbvsblobprodcus382.vsblob.vsassets.io"
    - "96bvsblobprodcus338.vsblob.vsassets.io"
    - "ajhvsblobprodcus363.vsblob.vsassets.io"
    - "bcnvsblobprodcus378.vsblob.vsassets.io"
    - "builds.dotnet.microsoft.com"
    - "c50vsblobprodcus330.vsblob.vsassets.io"
    - "c78vsblobprodcus322.vsblob.vsassets.io"
    - "cflvsblobprodcus383.vsblob.vsassets.io"
    - "ci.dot.net"
    - "ckzvsblobprodcus347.vsblob.vsassets.io"
    - "d0svsblobprodcus381.vsblob.vsassets.io"
    - "data.nuget.org"
    - "dlbvsblobprodcus316.vsblob.vsassets.io"
    - "e7bvsblobprodcus348.vsblob.vsassets.io"
    - "fdpvsblobprodcus345.vsblob.vsassets.io"
    - "frdvsblobprodcus327.vsblob.vsassets.io"
    - "gbsvsblobprodcus365.vsblob.vsassets.io"
    - "gervsblobprodcus329.vsblob.vsassets.io"
    - "h6tvsblobprodcus346.vsblob.vsassets.io"
    - "i1qvsblobprodcus353.vsblob.vsassets.io"
    - "ibzvsblobprodcus369.vsblob.vsassets.io"
    - "imzvsblobprodcus368.vsblob.vsassets.io"
    - "jd4vsblobprodcus366.vsblob.vsassets.io"
    - "josvsblobprodcus372.vsblob.vsassets.io"
    - "jrqvsblobprodcus343.vsblob.vsassets.io"
    - "k0ivsblobprodcus356.vsblob.vsassets.io"
    - "k4kvsblobprodcus344.vsblob.vsassets.io"
    - "kgfvsblobprodcus314.vsblob.vsassets.io"
    - "kh4vsblobprodcus325.vsblob.vsassets.io"
    - "kijvsblobprodcus387.vsblob.vsassets.io"
    - "kmuvsblobprodcus389.vsblob.vsassets.io"
    - "kxqvsblobprodcus376.vsblob.vsassets.io"
    - "l49vsblobprodcus358.vsblob.vsassets.io"
    - "l7avsblobprodcus319.vsblob.vsassets.io"
    - "ljcvsblobprodcus317.vsblob.vsassets.io"
    - "lylvsblobprodcus31.vsblob.vsassets.io"
    - "m16vsblobprodcus374.vsblob.vsassets.io"
    - "m6xvsblobprodcus342.vsblob.vsassets.io"
    - "m8dvsblobprodcus37.vsblob.vsassets.io"
    - "mfjvsblobprodcus373.vsblob.vsassets.io"
    - "n3kvsblobprodcus335.vsblob.vsassets.io"
    - "nn8vsblobprodcus340.vsblob.vsassets.io"
    - "o3svsblobprodcus318.vsblob.vsassets.io"
    - "ofvvsblobprodcus315.vsblob.vsassets.io"
    - "p2ovsblobprodcus312.vsblob.vsassets.io"
    - "pc2vsblobprodcus360.vsblob.vsassets.io"
    - "pdfvsblobprodcus380.vsblob.vsassets.io"
    - "pe3vsblobprodcus354.vsblob.vsassets.io"
    - "pe4vsblobprodcus351.vsblob.vsassets.io"
    - "pkgs.dev.azure.com"
    - "pkvvsblobprodcus321.vsblob.vsassets.io"
    - "rcxvsblobprodcus328.vsblob.vsassets.io"
    - "s4uvsblobprodcus326.vsblob.vsassets.io"
    - "s8mvsblobprodcus38.vsblob.vsassets.io"
    - "se1vsblobprodcus349.vsblob.vsassets.io"
    - "sqdvsblobprodcus333.vsblob.vsassets.io"
    - "st8vsblobprodcus339.vsblob.vsassets.io"
    - "tphvsblobprodcus375.vsblob.vsassets.io"
    - "u3hvsblobprodcus371.vsblob.vsassets.io"
    - "u6ovsblobprodcus377.vsblob.vsassets.io"
    - "ukkvsblobprodcus352.vsblob.vsassets.io"
    - "uy6vsblobprodcus34.vsblob.vsassets.io"
    - "v53vsblobprodcus320.vsblob.vsassets.io"
    - "vb4vsblobprodcus33.vsblob.vsassets.io"
    - "vwvvsblobprodcus334.vsblob.vsassets.io"
    - "x3yvsblobprodcus370.vsblob.vsassets.io"
    - "xupvsblobprodcus332.vsblob.vsassets.io"
    - "yluvsblobprodcus367.vsblob.vsassets.io"
    - "yttvsblobprodcus357.vsblob.vsassets.io"
    - "ytvvsblobprodcus310.vsblob.vsassets.io"

See Network Configuration for more information.

🤖 Automated content by GitHub Copilot. Generated by the Test Improver workflow. · copilot · auto · 410.2 AIC · ⌖ 16.9 AIC · ⊞ 22.2K · [◷]( · ◷)
_{Comment /test-assist to run again}

_{Add this agentic workflow to your repo}

To install this agentic workflow, run

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

Navigation Menu

Repository navigation

[test-improver] Add unit tests for DotnetTestConnection.StartServerControlChannelAsync guard conditions #11998

Goal and rationale

Approach

Coverage impact

Trade-offs

Reproducibility

Test Status

Effectiveness evidence

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

[test-improver] Add unit tests for DotnetTestConnection.StartServerControlChannelAsync guard conditions #11998

Description

Goal and rationale

Approach

Coverage impact

Trade-offs

Reproducibility

Test Status

Effectiveness evidence

Activity

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions