Uh oh!
There was an error while loading. Please reload this page.
Add RFC 017: Custom test host launcher - #9349
Conversation
Proposes an experimental ITestHostProcessLauncher MTP extension point that lets an extension control how the out-of-process test host is launched (replacing Process.Start), enabling packaged WinUI/MSIX (AUMID activation, #2784), debugger, elevated, container, and remote launch scenarios. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Adds a new design document (RFC 017) proposing an experimental Microsoft.Testing.Platform extension point (ITestHostProcessLauncher) to let extensions customize how the out-of-process test host is launched, while keeping platform-owned responsibilities (args/env prep, IPC, lifetime handling, exit reconciliation) unchanged.
Changes:
- Introduces RFC 017 documentation describing the proposed
ITestHostProcessLauncherAPI surface and registration shape. - Documents intended platform integration points (controller host selection, launch-site delegation, singleton enforcement).
- Provides worked examples (WinUI/MSIX activation, debugger attach, elevation, container, remote) plus alternatives and open questions.
Show a summary per file
| File | Description |
|---|---|
| docs/RFCs/017-TestHost-Process-Launcher.md | New RFC describing an experimental extension point for customizing out-of-process test host process launching. |
Copilot's findings
- Files reviewed: 1/1 changed files
- Comments generated: 4
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Fix TestHostOchestratorHost -> TestHostOrchestratorHost type name (the file name has the typo, the type does not) - SSH example: EnvironmentVariables values are nullable; skip unset (null) vars instead of Quote(null) - WinUI example: drop unnecessary async (no await) to avoid CS1998; return Task.FromResult - Clarify #2784 relationship: it is titled for unpackaged WinUI, but this launcher serves both packaged (AUMID) and unpackaged WinUI scenarios Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The 'Alternatives' link pointed to #alternatives but the heading is 'Alternatives considered' (#alternatives-considered). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Make the proposal agnostic of the launch mechanism ("we don't know if it's a
process"):
- Rename ITestHostProcessLauncher -> ITestHostLauncher and ITestHostProcessHandle
-> ITestHostHandle; demote ProcessId to an optional int? used only for
diagnostics; rename Kill() -> Terminate(). Rename the file accordingly
(017-TestHost-Process-Launcher.md -> 017-TestHost-Launcher.md).
- Frame the motivation around packaged Windows apps (UWP and packaged WinUI share
the same MSIX deploy + AUMID-activate mechanism) while keeping debugger, elevation,
container, and remote as worked examples.
- Document the platform integration detail that makes PID-less launchers work
(premature-exit gated on HasExited only) and reference the implementation
(Microsoft.Testing.Extensions.PackagedApp) and PR.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>- Contract: phrase env-var delivery as "the host must end up with the values in
context.EnvironmentVariables" (mechanism left to the launcher) instead of
"must receive", since AUMID activation cannot inject per-launch env vars.
- Packaged WinUI/MSIX example: stop claiming MSTestRunnerWinUI demonstrates the
controller pipe handshake (it self-hosts in-process); escape/quote the activation
args instead of string.Join(' ', ...) so quoting is preserved; spell out that the
launcher must bridge the pipe name / correlation id another way.
- Fix MD051: link fragment #alternatives -> #alternatives-considered.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Drop the redundant Exited event from ITestHostHandle (consumers use WaitForExitAsync); the platform synthesizes the internal informational event from the exit task. - Replace the int? ProcessId with an optional free-form string Identifier (diagnostics only: PID, container id, remote host:pid, ...), since the handle is mechanism-agnostic. - Fix remaining review nits: use Terminate() consistently (drop stray "Kill" reference), filter null-valued env vars in the debugger/container examples (consistent with the SSH example), and spell "queryable". Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Amaury Levé (Evangelink)
left a comment
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Posted via a maintainer's GitHub token, so it appears under their account — the account owner did not write or approve this content personally. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Documentation-only PR — only docs/RFCs/017-TestHost-Launcher.md changed (new file, 406 lines). Most of the 22 review dimensions (Threading, Performance, Security, Cross-TFM, IPC wire, Test Isolation, Analyzer Quality, etc.) are N/A. Dimensions evaluated: 17 — Documentation Accuracy and 21 — Scope & PR Discipline.
Scope & PR Discipline — ✅
- RFC references the blocking issue (#2784) and the implementation PR (#9454).
- Goals / Non-goals / Alternatives / Open Questions sections are all present and well-structured.
- The design rationale (why
ITestHostProcessLauncher→ITestHostLauncher, why notITestHostExecutionOrchestrator, why not exposeIProcessHandler) is thorough.
Documentation Accuracy — 4 findings
| # | Severity | Finding |
|---|---|---|
| 1 | 🟡 MODERATE | WaitForExitAsync() has no CancellationToken parameter — platform cannot propagate overall-run cancellation cooperatively (inline comment on line 160) |
| 2 | 🟡 MODERATE | ITestHostHandle does not extend IDisposable — OS resources held by adapters (process handles, sockets) have no deterministic cleanup path via the platform (inline comment on line 163) |
| 3 | 🟡 MODERATE | ExitCode behavior is undefined when read before HasExited — implementers need a contract to code against (inline comment on line 158) |
| 4 | ⚪ NIT | Quote / PasteArguments helpers in Examples 1 and 5 are called but never defined or referenced, leaving a potential argument-injection pitfall for readers (inline comment on line 340) |
✅ 20/22 dimensions clean (2 N/A, 2 have findings above).
WaitForExitAsync— addCancellationTokenoverload or document that callers must drive cancellation throughTerminate()ITestHostHandle— addIDisposable(orIAsyncDisposable) or add this to the Open Questions for #9454 to decideExitCode— document "must not be read beforeHasExitedis true"- Example 5 — define or reference
Quote, note it is a placeholder
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- ITestHostHandle now extends IDisposable and WaitForExitAsync takes a CancellationToken (both supported by the runtime and the netstandard2.0 polyfill). The platform disposes the handle after exit and passes its cancellation token while waiting, reconciling the real exit code afterwards. - Document ExitCode-before-HasExited as undefined; note Quote/PasteArguments are placeholders for proper argument quoting; fix the container example so Terminate() tears down the container (docker stop) rather than only killing the local client. - Record the design evolution in Alternatives. Implemented in PR #9454. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Uh oh!
There was an error while loading. Please reload this page.
017 is already taken on main by 017-TestHost-Launcher.md (#9349), so rename docs/RFCs/017-Artifact-Post-Processing.md -> 018-Artifact-Post-Processing.md. Rework the design into a layered, decisive recommendation: - Phase 1 (ship first): typed, invocation-agnostic IArtifactPostProcessor engine contract (experimental) + a user-facing ITool per kind (merge-trx) sharing one engine. Delivers user value with no SDK/protocol change (one platform-API change: promote ITool to public). - Phase 2: in-memory election (from data the SDK already has via handshake + live FileArtifactMessages), a platform-owned internal dispatcher ITool, and merged artifacts returned over the existing dotnet-test pipe as FileArtifactMessage. - Election is minimal set-cover (fewest relaunches), arch-constrained for binary coverage; live-channel alternative reframed to lead with reuse of the existing pipe; experimental gating; cross-arch notes. Also fixes markdownlint issues and corrects codebase-accuracy points surfaced in review (ITool is internal today; TrxReportEngine has no file-merge path yet; no --list-tools switch; public records must avoid synthesized init accessors). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Summary
Adds RFC 017 — Custom test host launcher (
docs/RFCs/017-TestHost-Launcher.md).The RFC proposes a public, experimental MTP extension point —
ITestHostLauncher— that lets an extension control how the out-of-process test host is launched, instead of the platform always callingProcess.Start. The platform keeps owning everything around the launch (argument/environment preparation, the controller↔host IPC pipe, PID tracking,ITestHostProcessLifetimeHandlercallbacks, and exit-code reconciliation) and delegates only the single "create and start the test host" step.The hook is deliberately agnostic of the launch mechanism ("we don't know if it's a process"): the launcher does not have to start a local OS process. It returns an
ITestHostHandle(anIDisposable) that exposes only lifecycle (WaitForExitAsync(CancellationToken),ExitCode,HasExited,Terminate) plus an optional free-formstring? Identifierused purely for diagnostics. This covers packaged-app activation, containers, and remote launches where no local PID exists.The motivating scenario is testing packaged Windows apps — both UWP and packaged WinUI (which ship as MSIX and share the same deploy + AUMID-activate mechanism, the reason VSTest has a single
UwpTestHostRuntimeProvider). This is the blocker behind #2784. The RFC also includes worked examples for debugger attach, elevation, container, and remote.What changed since the first draft
This reworks the original process-centric draft to be generic:
ITestHostProcessLauncher→ITestHostLauncher;ITestHostProcessHandle→ITestHostHandle.int ProcessIdis replaced by an optional free-formstring? Identifier(diagnostics only); the redundantExitedevent is dropped in favor ofWaitForExitAsync;Kill()→Terminate().ITestHostHandleextendsIDisposable, andWaitForExitAsynctakes aCancellationToken(both supported by the runtime and the repo's netstandard2.0 polyfill).017-TestHost-Process-Launcher.md→017-TestHost-Launcher.md.HasExitedonly).Implementation
The hook is implemented, with a reference consumer (
Microsoft.Testing.Extensions.PackagedApp) and unit + acceptance tests, in #9454.Related: #2784, #9454