Retarget the MTP client onto Microsoft.Testing.Platform.ServerMode.Client.Sources

Merged
NO
2mo ago
+1662-1066

What this does

vstest hand-maintained an MTP (Microsoft.Testing.Platform) server-mode JSON-RPC client under
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/. testfx now ships that client as a source-only
package, Microsoft.Testing.Platform.ServerMode.Client.Sources, built from the MTP server's own protocol
and serialization source, so the wire format cannot drift from the server.

This deletes vstest's transport core and retargets the glue onto the package's IMtpServerClient:

  • Deleted: MtpServerConnection, MtpJson, MtpConstants, MtpClientHelpers.
  • Retargeted: MtpProxyDiscoveryManager, MtpProxyExecutionManager, MtpTestNodeConverter — launch via
    MtpServerClient.Launch, drive InitializeAsync/DiscoverTestsAsync/RunTestsAsync/ExitAsync, read
    node updates from the TestNodesUpdated event with typed MtpTestNodeUpdate accessors, and bridge
    EqtTrace through DelegateMtpClientLogger.
  • Added: MtpClientOptionsFactory — builds MtpServerClientOptions and maps server log levels.
  • Added: MtpServerClientFactory — a testability seam so the proxies can be unit-tested against a fake
    client.
  • Unchanged: MtpProxyManagerFactory, MtpDataCollectionForwarder (transport-agnostic).

The package is a compile-time source dependency (PrivateAssets="all"): it compiles into
Microsoft.TestPlatform.CrossPlatEngine.dll with no shipped DLL, no runtime dependency, and no new
public API. On net462/netstandard2.0 the package compiles its own nullable-annotation polyfills; vstest
already imports the identical set from CoreUtilities, so MTP_CLIENT_EXCLUDE_NULLABLE_ATTRIBUTES is
defined on CrossPlatEngine to defer to those and avoid a CS0436 clash (no-op on net8.0).

Package dependency

The package ships from microsoft/testfx#10085 and is restored from the public dnceng dotnet-tools feed
(already in NuGet.config). Pinned to 2.4.0-preview.26410.1 via
MicrosoftTestingPlatformServerModeClientSourcesVersion in eng/Versions.props. No local feed and no
NuGet.config change are needed — the earlier interim eng/local-mtp-feed is removed.

Local validation

Build: CrossPlatEngine compiles clean on all three TFMs (net462, netstandard2.0, net8.0) under
TreatWarningsAsErrors and the public-API analyzers, and adds no public API.

CrossPlatEngine MTP unit tests: 71/71 on both target frameworks (net481 and net11.0) — the converter,
options factory, both proxies, and the client factory seam, exercised against a fake client.

MtpUnderVstestTests acceptance (9 methods on each of the net481 and net11.0 console runners, 18 total):

  • .NET runner (CrossPlatEngine net8, System.Text.Json): 9/9. Discovery, execution, per-test standard
    output, run-settings environment-variable injection, an out-of-proc data collector, blame, a mixed
    classic+MTP run, a single-TRX run, and a non-ASCII test-name round-trip. This is the end-to-end proof
    that the retarget drives the real System.Text.Json client correctly.
  • .NET Framework runner (CrossPlatEngine net462, Jsonite): 6/9. Same scenarios pass except 3
    /logger:trx tests, which fail to load the TRX logger in my locally-built net462 runner at logger
    argument-processing, before the MTP client runs. This is a pre-existing local-deployment issue, not the
    retarget: a classic (non-MTP) /logger:trx test fails identically on this runner, the same test methods
    pass on the .NET runner, and the source package is compile-only so it adds no runtime assembly. It does
    not reproduce on a clean checkout.

This branch reconciles parallel hardening from @azat-msft and @nohwnd on the same feature (the client
factory seam, the six MTP unit-test files, and the non-ASCII framing guard) and merges the latest main,
including the change that disables the MTP testhost by default. The non-ASCII acceptance test is aligned
with the other MTP tests to opt back into the MTP testhost.

Resolved questions (confirmed by the package owner against server + client source)

  • G1 — the normalized Node shape is a documented contract. Both formatter paths (Jsonite on
    net462/netstandard2.0, System.Text.Json on .NET) materialize the graph the same way: every JSON object
    becomes IDictionary<string, object?>, every array ICollection<object>, strings/bools/null as-is.
    Numbers are boxed per formatter, so numerics must be coerced, never hard-cast. vstest reads
    standardOutput, standardError, location.file, location.line-start, traits, and the vstest.*
    bridge properties from the raw Node dictionary by string key; MtpTestNodeConverter coerces the
    numeric ones (TryGetRawInt handles int/long/double/float/decimal).
  • G2 — ordering guarantee. Once DiscoverTestsAsync/RunTestsAsync completes, every
    TestNodesUpdated handler has already run, which lets vstest drop its old completion-sentinel drain.
  • G3 — uid-only run filter. The server projects node.Uid alone when building the run filter and
    never reads DisplayName, so sending the uid as the identity is correct.
  • G4 — artifacts come from the run response. MtpRunResult.Artifacts is the complete set (absolute
    file path, producer uid, type, display name, description), mapped to AttachmentSet. The
    AttachmentsReceived event is not additive on this request/response path, so there is no double
    counting. Validated by the passing data-collector and blame tests on the net462 axis.
  • G5 — MtpServerClientOptions.EnvironmentVariables is keyed ordinal. This PR keys both of vstest's
    own environment-variable dictionaries the same way vstest's classic path does (case-insensitive on
    Windows) so case-variant keys collapse before the values reach the ordinal package dictionary.

🤖