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
- 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>
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>
- 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>
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 decide -
ExitCode— document "must not be read beforeHasExitedis true" - Example 5 — define or reference
Quote, note it is a placeholder
- 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>
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