Skip to content

[POC – do not merge] Drive Microsoft.Testing.Platform apps over the dotnettestcli pipe protocol - #16287

Closed
Jakub Jareš (nohwnd) wants to merge 3 commits into
microsoft:mainfrom
nohwnd:nohwnd-integrate-ytest-mtp-pipeprotocol
Closed

[POC – do not merge] Drive Microsoft.Testing.Platform apps over the dotnettestcli pipe protocol#16287
Jakub Jareš (nohwnd) wants to merge 3 commits into
microsoft:mainfrom
nohwnd:nohwnd-integrate-ytest-mtp-pipeprotocol

Conversation

@nohwnd

@nohwnd Jakub Jareš (nohwnd) commented Jul 15, 2026

Copy link
Copy Markdown
Member

⚠️ Proof of concept — do not merge as-is. This is a spike to show the approach and get the full acceptance suite green end-to-end, not something to ship. The pipe-protocol core is vendored (copied) from an external repo and still needs a proper home: it should move into testfx and be consumed from there, plus a licensing and API review. Opening it as a draft for discussion.


vstest drives Microsoft.Testing.Platform test apps over a hand-written TCP JSON-RPC "server mode" client. This swaps that for the named-pipe dotnettestcli protocol — the same one dotnet test speaks — and drives it directly from the two MTP proxy managers.

The transport and binary protocol core is vendored from Youssef1313/MTPSharding (YTest.MTP.PipeProtocol, MIT) under Client/MTP/PipeProtocol, with the author's permission. It's internal, folder-scoped as generated code, with a couple of compiler polyfills so it builds on net462/netstandard2.0. As noted above, the intent is to move it into testfx and consume it from there — vendoring here just keeps the spike self-contained and avoids a new shipping DLL and the binding-redirect churn that comes with it.

What changed:

  • TestApplication gains the three seams the drivers need: env-var injection, a file-artifact callback, and cancellation that kills the child process.
  • MtpMessageConverter maps the wire records onto TestCase/TestResult; MtpProxyDiscoveryManager and MtpProxyExecutionManager are rewritten on top of it.
  • The old JSON-RPC layer is deleted (MtpServerConnection, MtpJson, MtpClientHelpers, MtpTestNodeConverter).

The public seam (MtpProxyManagerFactory) and the datacollector forwarder are unchanged, so the detection/routing story is the same.

The full MtpUnderVstestTests suite is green — run, mixed run, TRX, Blame, runsettings env vars, per-test stdout/stderr, and the generic collector — on both the .NET and .NET Framework consoles.

vstest drove Microsoft.Testing.Platform apps over a hand-written TCP JSON-RPC
server-mode client. Swap it for the named-pipe dotnettestcli protocol, vendored
from Youssef1313/MTPSharding (MIT, with the author's permission) under
Client/MTP/PipeProtocol, and drive it directly from the two proxy managers.

- Vendor the transport + binary protocol core (internal, folder-scoped
  generated-code editorconfig, net462/netstandard2.0 compiler polyfills).
- TestApplication gains three seams: env-var injection, a file-artifact
  callback, and cancellation that kills the child process.
- MtpMessageConverter maps the wire records onto TestCase/TestResult;
  MtpProxyDiscoveryManager and MtpProxyExecutionManager are rewritten on top.
- Delete the old JSON-RPC layer (MtpServerConnection, MtpJson,
  MtpClientHelpers, MtpTestNodeConverter).

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI lite review requested due to automatic review settings July 15, 2026 12:31

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR migrates vstest’s Microsoft.Testing.Platform (MTP) integration from the legacy TCP JSON-RPC “server mode” client to the named-pipe dotnettestcli protocol (the same protocol used by dotnet test). It does this by vendoring the protocol implementation into CrossPlatEngine and rewriting the MTP proxy managers to drive MTP apps via that pipe protocol while continuing to translate MTP messages into vstest TestCase/TestResult updates for loggers and console output.

Changes:

  • Vendored the dotnettestcli pipe transport + binary protocol serializers under Client/MTP/PipeProtocol (with polyfills for net462/netstandard2.0).
  • Rewrote MtpProxyDiscoveryManager and MtpProxyExecutionManager to use the new pipe protocol and new MtpMessageConverter.
  • Removed the old JSON-RPC client implementation and updated acceptance-test documentation + third-party notices.

Reviewed changes

Copilot reviewed 45 out of 45 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
THIRD-PARTY-NOTICES.txt Adds MIT license notice for the vendored MTPSharding protocol code.
test/Microsoft.TestPlatform.Acceptance.IntegrationTests/MtpUnderVstestTests.cs Updates test comments to reflect the new named-pipe protocol path.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpConstants.cs Re-scopes MTP constants to shared integration identifiers (drops JSON-RPC constants).
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpLaunch.cs New launcher resolution helper for MTP .exe/.dll sources.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpMessageConverter.cs New mapping from pipe-protocol records to vstest TestCase/TestResult.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyDiscoveryManager.cs Switches discovery to pipe protocol (--list-tests) via TestApplication.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpProxyExecutionManager.cs Switches execution to pipe protocol; forwards results/output and collects file artifacts.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpClientHelpers.cs Deletes legacy JSON-RPC helpers (no longer needed).
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpJson.cs Deletes JSON-RPC Jsonite accessors (no longer needed).
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpServerConnection.cs Deletes the legacy TCP JSON-RPC client implementation.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/MtpTestNodeConverter.cs Deletes the legacy JSON test-node → ObjectModel converter.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/.editorconfig Marks vendored code as generated and disables analyzers for the folder.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/CompilerPolyfills.cs Adds polyfills for records/init/required metadata on net462/netstandard2.0.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Extensions/ProcessExtensions.cs Adds a .NET Framework WaitForExitAsync polyfill for process handling.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/HandshakeMessagePropertyNames.cs Defines handshake property IDs used by the pipe protocol.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/IRequest.cs Introduces request marker interface for protocol messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/IResponse.cs Introduces response marker interface for protocol messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/CommandLineOptionMessages.cs Adds record types for command line option message payloads.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/DiscoveredTestMessage.cs Adds record types for discovery payloads (tests/traits/location).
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/FileArtifactMessages.cs Adds record types for file-artifact payloads.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/HandshakeMessage.cs Adds record type for handshake messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/ModuleMessage.cs Adds record type for module metadata messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/TestResultMessages.cs Adds record types for successful/failed test result payloads.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Messages/UnknownMessage.cs Adds record type for unknown/unhandled message IDs.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/NamedPipeBase.cs Adds base serializer registration/lookup infrastructure.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/NamedPipeServer.cs Adds named-pipe server framing loop + request/response dispatch.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/ObjectFieldIds.cs Defines numeric field IDs and serializer IDs for the wire format.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/ProtocolConstants.cs Defines protocol version constant used in handshake.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/RegisterSerializers.cs Registers all message serializers for the pipe transport.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/BaseSerializer.cs Adds binary read/write helpers and field encoding primitives.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/CommandLineOptionMessagesSerializer.cs Adds binary serializer for command-line options messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/DiscoveredTestMessagesSerializer.cs Adds binary serializer for discovered tests messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/FileArtifactMessagesSerializer.cs Adds binary serializer for file artifact messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/HandshakeMessageSerializer.cs Adds binary serializer for handshake messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/INamedPipeSerializer.cs Adds serializer interface used by the transport.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/ModuleMessageSerializer.cs Adds binary serializer for module messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/TestResultMessagesSerializer.cs Adds binary serializer for test result messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/TestSessionEventSerializer.cs Adds binary serializer for session event messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/UnknownMessageSerializer.cs Adds serializer for unknown message IDs.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/Serializers/VoidResponseSerializer.cs Adds void response serializer for messages that don’t require a payload.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/TestApplication.cs Adds process launcher + pipe accept loop + message dispatch hooks for proxies.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/TestProcessExitInformation.cs Adds process exit info container returned from TestApplication.RunAsync.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/TestSessionEvent.cs Adds record type for session event messages.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/TestStates.cs Adds numeric constants mapping to MTP test state values.
src/Microsoft.TestPlatform.CrossPlatEngine/Client/MTP/PipeProtocol/VoidResponse.cs Adds singleton response type for “no payload” responses.

@nohwnd Jakub Jareš (nohwnd) changed the title Drive Microsoft.Testing.Platform apps over the dotnettestcli pipe protocol [POC – do not merge] Drive Microsoft.Testing.Platform apps over the dotnettestcli pipe protocol Jul 15, 2026
@nohwnd
Jakub Jareš (nohwnd) marked this pull request as draft July 15, 2026 13:18
On Unix, NamedPipeServerStream.WaitForConnectionAsync surfaces an OperationCanceledException whose CancellationToken is not reference-equal to the token we passed. The 'when (ex.CancellationToken == token)' guard therefore missed on shutdown and the accept loop fell through to Environment.FailFast, killing vstest.console after the test results were written but before the run summary. Guard on token.IsCancellationRequested instead, which is stable across platforms.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings July 15, 2026 14:16

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 45 out of 45 changed files in this pull request and generated 6 comments.

Keep the ServerMode.Client.Sources implementation that superseded the vendored pipe client in microsoft#16300.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>

🤖
Copilot AI review requested due to automatic review settings August 14, 2026 22:27

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@nohwnd

Copy link
Copy Markdown
Member Author

The merge in b6c109e resolved every conflict in favour of main and reverted the whole spike with it. The tree at b6c109e is identical to the merge base 9cbb02d, git diff 9cbb02d b6c109e prints nothing, Client/MTP/PipeProtocol is gone, the proxy manager rewrite is gone, and MtpTestNodeConverter.cs is back even though the description above says it was deleted. That is also why every check reports "skipping", there is nothing in the diff to build.

So this PR currently contains no changes at all. Nothing is lost, the spike is still intact at 588c968 which is the second parent of that merge, but the merge has to be redone before the PR shows anything again.

The 15 review comments all point at files under Client/MTP/PipeProtocol that are not on the branch anymore, so I am answering and resolving them instead of editing code that is not there. I checked each one against 588c968 and they are all correct, so I am collecting them here so they are not lost when the threads are resolved.

Framing and partial reads

  • BaseSerializer, the non NETCOREAPP path ignores the return value of Stream.Read in ReadString, ReadStringValue, ReadInt, ReadLong, ReadUShort and ReadBool, while the NETCOREAPP path uses ReadExactly. Partial reads corrupt deserialization on net462 and netstandard2.0.
  • BaseSerializer.ReadByte casts Stream.ReadByte() to byte, so at EOF -1 becomes 255 instead of throwing. This one affects every TFM.
  • NamedPipeServer reads the 4 byte message size with BitConverter.ToInt32(_readBuffer, 0) straight after a single ReadAsync, so a short first read desynchronizes the stream.
  • NamedPipeServer resets _messageBuffer.Position and _serializationBuffer.Position but not their length, so a shorter message following a longer one can still see stale bytes.

FailFast

  • WaitConnectionAsync and OnRequest both end in Environment.FailFast(ex.ToString()), which takes down the whole vstest process on any unexpected exception and loses the run summary. 588c968 already fixed the concrete case where a cancelled accept loop on Unix hit this, but the catch all is still there and should surface the exception instead.

Handshake and async

  • _handshakes is a plain Dictionary written from multiple pipe connections, so parallel connects can race, and a duplicate key throws.
  • SpinWait.SpinUntil(() => _afterProcessStartTask.IsCompleted) sits inside an already async OnRequest, it should just await the task.
  • RunAsync has no connect or handshake timeout, so an app that starts but never connects and never exits hangs the run until the user cancels.

API surface

  • TestProcessExitInformation is public while the rest of the vendored layer is internal, and it is not in PublicAPI.Unshipped.txt. It should be internal. The doc comment also says "after it exists", should be "exits".
  • VoidResponseSerializer.Deserialize news up a VoidResponse per message although VoidResponse.CachedInstance already exists.

One thing worth deciding before any of this gets fixed: this is vendored code, and the plan in the description is to move it into testfx and consume it from there. Fixing it here means the copy drifts from https://github.com/Youssef1313/MTPSharding, so most of these belong upstream instead.

🤖

@nohwnd

Copy link
Copy Markdown
Member Author

The vs client is now shipping as sources in MTP, will go with that first as it is providing a more readable and debuggable api surface (messages are not binary), and is already used in VS.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants