From 45dc9002677bac5c0ca8e09d15c32b0b0ea630e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Mon, 22 Jun 2026 19:30:25 +0200 Subject: [PATCH 1/7] Add RFC 017: Custom test host process launcher 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> --- docs/RFCs/017-TestHost-Process-Launcher.md | 300 +++++++++++++++++++++ 1 file changed, 300 insertions(+) create mode 100644 docs/RFCs/017-TestHost-Process-Launcher.md diff --git a/docs/RFCs/017-TestHost-Process-Launcher.md b/docs/RFCs/017-TestHost-Process-Launcher.md new file mode 100644 index 0000000000..08399dfc71 --- /dev/null +++ b/docs/RFCs/017-TestHost-Process-Launcher.md @@ -0,0 +1,300 @@ +# RFC 017 - Custom test host process launcher + +- [ ] Approved in principle +- [x] Under discussion +- [ ] Implementation +- [ ] Shipped + +## Summary + +Introduce **`ITestHostProcessLauncher`**: a public, experimental Microsoft.Testing.Platform (MTP) extension point that lets an extension control **how** the out-of-process test host is launched, instead of the platform always doing `Process.Start`. The platform still owns everything around the launch — argument/environment preparation, the controller↔host IPC pipe, PID tracking, `ITestHostProcessLifetimeHandler` callbacks, and exit-code reconciliation — and simply delegates the single "create the process" step to the registered launcher. + +The motivating scenario is testing **packaged WinUI/MSIX applications**, which cannot be started with `Process.Start` and must be activated by AUMID (see [#2784](https://github.com/microsoft/testfx/issues/2784)). But the abstraction is deliberately generic: the same hook enables launching the test host under a debugger, elevated, inside a container, or on a remote machine. + +## Motivation + +MTP runs the test host out-of-process whenever a "test host controller" extension is active (hang dump, crash dump, or any `ITestHostProcessLifetimeHandler` / `ITestHostEnvironmentVariableProvider`). That work happens in `TestHostControllersTestHost`, which prepares a `ProcessStartInfo` (arguments, environment variables including the `MONITORTOHOST` pipe name) and then launches the host with a single call: + +```csharp +using IProcess testHostProcess = process.Start(processStartInfo); +``` + +Everything downstream only needs five things from the returned handle — `Id`, the `Exited` event, `WaitForExitAsync()`, `ExitCode`, and `Kill()` — plus the child connecting back on the named pipe whose name was injected via an environment variable. **`Process.Start` is the only assumption that does not hold universally.** Several real scenarios need a different launch mechanism: + +- **Packaged WinUI/MSIX**: a packaged app must be activated by Application User Model ID (AUMID) via `IApplicationActivationManager`, not started from an executable path. This is the blocker behind [#2784](https://github.com/microsoft/testfx/issues/2784) and the reason VSTest's `UwpTestHostRuntimeProvider` exists. +- **Debugger attach/launch**: start the host suspended (or under a debugger launcher such as `vsdbg` / `WinDbg` / `dlv`) and only then resume. +- **Elevation**: run the test host as administrator (UAC) or as another user. +- **Container / remote**: launch the host inside a container (`docker run`) or on a remote device over SSH/WinRM, then bridge the pipe. + +Today none of these is possible without forking the platform. The existing experimental `ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives)). This RFC adds the *minimal* hook at exactly the launch site. + +## Goals + +- Let an extension substitute the test host process creation step while the platform keeps owning argument/env preparation, IPC, lifetime-handler dispatch, and exit-code handling. +- Keep hang dump, crash dump, and all `ITestHostProcessLifetimeHandler` / `ITestHostEnvironmentVariableProvider` extensions working unchanged when a custom launcher is present. +- Be generic enough to cover WinUI activation, debugger, elevation, container, and remote launch with one shape. +- Follow MTP's experimental-API conventions so the surface can evolve before stabilizing. + +## Non-goals + +- Replacing the *entire* run loop (that is `ITestHostExecutionOrchestrator`'s job). +- Remote **device deployment/bootstrapping** of the Windows App SDK framework + agent (VSTest's `Microsoft.UniversalApps.Deployment` has no public redistributable; out of scope — local launch only). +- Shipping the WinUI deployment extension itself. This RFC only adds the platform hook; the WinUI extension is a separate deliverable that consumes it. +- Changing the in-process (single-process, `ConsoleTestHost`) execution path. + +## Detailed design + +### Where it plugs in + +The hook lives in the **test host controllers** layer, next to the existing lifetime-handler and environment-variable-provider extension points, and is registered through `ITestHostControllersManager`. + +```csharp +namespace Microsoft.Testing.Platform.Extensions.TestHostControllers; + +/// +/// Allows an extension to control how the out-of-process test host is launched, +/// replacing the platform's default Process.Start behavior. +/// +[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] +public interface ITestHostProcessLauncher : ITestHostControllersExtension // : IExtension +{ + /// + /// Creates and starts the test host process. The platform has already prepared the + /// file name, arguments, and environment variables (including the controller IPC pipe + /// name). The implementation must return a handle the platform can monitor. + /// + Task LaunchTestHostProcessAsync( + TestHostProcessLaunchContext context, + CancellationToken cancellationToken); +} +``` + +The platform passes the fully-prepared launch information: + +```csharp +[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] +public sealed class TestHostProcessLaunchContext +{ + public TestHostProcessLaunchContext( + string fileName, + IReadOnlyList arguments, + IReadOnlyDictionary environmentVariables, + string workingDirectory); + + /// The default test host executable path the platform would have started. + public string FileName { get; } + + /// Arguments, already including the test host controller PID option. + public IReadOnlyList Arguments { get; } + + /// + /// The final environment for the test host, after all + /// ran. Includes the + /// controller↔host IPC pipe name the host must connect back on. + /// + public IReadOnlyDictionary EnvironmentVariables { get; } + + public string WorkingDirectory { get; } +} +``` + +And the launcher returns a public-safe handle (a subset of the internal `IProcess`, which the platform adapts to its monitoring): + +```csharp +[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] +public interface ITestHostProcessHandle +{ + int ProcessId { get; } + int ExitCode { get; } + bool HasExited { get; } + event EventHandler Exited; + Task WaitForExitAsync(); + void Kill(); +} +``` + +Registration mirrors the existing methods on `ITestHostControllersManager`: + +```csharp +public interface ITestHostControllersManager +{ + // existing: AddEnvironmentVariableProvider(...), AddProcessLifetimeHandler(...) + + [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] + void AddTestHostProcessLauncher( + Func factory); + + [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] + void AddTestHostProcessLauncher(CompositeExtensionFactory factory) + where T : class, ITestHostProcessLauncher; +} +``` + +### Platform integration (what changes inside MTP) + +1. **Swap the launch call.** In `TestHostControllersTestHost.InternalRunAsync`, at the current `process.Start(processStartInfo)` site (after `BeforeTestHostProcessStartAsync` and after all env-var providers ran), if a launcher is registered, build a `TestHostProcessLaunchContext` from the `ProcessStartInfo` and `await launcher.LaunchTestHostProcessAsync(...)`. Otherwise keep the default `process.Start`. The returned `ITestHostProcessHandle` is adapted to the internal `IProcess` monitoring contract — which already only uses `Id` / `Exited` / `WaitForExitAsync` / `ExitCode` / `Kill`. +2. **Force the controller host.** Add the launcher to `TestHostControllerConfiguration` and make `RequireProcessRestart` `true` when a launcher is registered. Host selection is gated on `RequireProcessRestart` (`TestHostControllersManager.BuildAsync` → checked in `TestHostBuilder.Modes.cs`); without this, a run with *only* a launcher (no dump/lifetime extension) would stay in-process and there would be nothing to launch. +3. **Singleton.** Validate that at most one launcher is registered, matching the orchestrator's "Multiple … not supported" behavior. A duplicate registration fails fast at build time. +4. **Preserve ordering and services.** Because the call stays at the same point, `ITestHostEnvironmentVariableProvider`, the `MONITORTOHOST` IPC pipe, the PID handshake, and `ITestHostProcessLifetimeHandler` (and therefore hang dump and crash dump) all keep working with no changes. + +### Contract requirements on the launcher + +- The launched process **must** be a real OS process with a queryable PID, it **must** inherit/receive `context.EnvironmentVariables` (so it connects back on the controller pipe), and it **must** be passed `context.Arguments`. +- The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `Exited`) and support `Kill()` (hang dump terminates the host through it). +- If the launcher cannot start the process it should throw; the platform surfaces it as a platform-setup failure. + +### Minimal-surface variant + +If we prefer not to expose a handle type initially, the launcher can instead return `Task` (the PID) and the platform wraps it internally via the existing `IProcessHandler.GetProcessById(pid)`, which already provides `Id` / `Exited` / `WaitForExit` / `ExitCode` / `Kill`. That reduces the public surface to the interface + context only, at the cost of requiring the launched thing to be a local, query-able OS process (true for AUMID-activated packaged apps; not true for a process living inside a container or on a remote host). Because the goal is also to support container/remote, the RFC proposes the **handle-returning** shape as primary. + +## Examples + +All examples assume the extension is registered on the builder, e.g. from a `…Extensions` helper: + +```csharp +builder.TestHostControllers.AddTestHostProcessLauncher(sp => new MyLauncher(sp)); +``` + +### 1. Packaged WinUI / MSIX (the motivating case) + +Activate the packaged app by AUMID instead of starting an exe. The activated app self-hosts MTP (as the `MSTestRunnerWinUI` sample already does) and connects back on the env-provided pipe. + +```csharp +internal sealed class WinUiAppxLauncher(IServiceProvider sp) : ITestHostProcessLauncher +{ + public string Uid => nameof(WinUiAppxLauncher); + public string Version => "1.0.0"; + public string DisplayName => "WinUI MSIX launcher"; + public string Description => "Deploys and AUMID-activates a packaged WinUI test app."; + public Task IsEnabledAsync() => Task.FromResult(true); + + public async Task LaunchTestHostProcessAsync( + TestHostProcessLaunchContext context, CancellationToken cancellationToken) + { + // 1. Parse the .appxrecipe / AppxManifest.xml next to context.FileName to get the AUMID + // and (in Developer Mode) register the loose layout: + // await new PackageManager().RegisterPackageByUriAsync(manifestUri, options); + string aumid = AppxManifest.ResolveAumid(context.FileName); + + // 2. Activate, passing the SAME args the platform prepared. + var aam = (IApplicationActivationManager)new ApplicationActivationManager(); + aam.ActivateApplication(aumid, string.Join(' ', context.Arguments), + ACTIVATEOPTIONS.AO_NONE, out uint pid); + + // 3. Wrap the returned PID. The app inherits context.EnvironmentVariables + // (incl. the MONITORTOHOST pipe name) via its activation/launch profile. + return new ProcessIdHandle((int)pid); + } +} +``` + +> Note: enabling the controller→host pipe across the AppContainer sandbox requires a loopback/pipe-ACL step (e.g. `CheckNetIsolation LoopbackExempt` or granting the package SID on the pipe). That belongs to the WinUI extension, not the platform. + +### 2. Launch under a debugger + +Start the host suspended and attach a debugger before it runs (useful for `--debug`-style flows or CI repro). + +```csharp +public async Task LaunchTestHostProcessAsync( + TestHostProcessLaunchContext context, CancellationToken cancellationToken) +{ + var psi = new ProcessStartInfo(context.FileName) { UseShellExecute = false }; + foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); + foreach (var kvp in context.EnvironmentVariables) psi.Environment[kvp.Key] = kvp.Value; + psi.Environment["DOTNET_DefaultDiagnosticPortSuspend"] = "1"; // start suspended + + Process p = Process.Start(psi)!; + await DebuggerLauncher.AttachAsync(p.Id, cancellationToken); // e.g. vsdbg / WinDbg / dlv + await DebuggerLauncher.ResumeAsync(p.Id, cancellationToken); + return new ProcessHandleAdapter(p); +} +``` + +### 3. Elevated (run as administrator) + +```csharp +public Task LaunchTestHostProcessAsync( + TestHostProcessLaunchContext context, CancellationToken cancellationToken) +{ + var psi = new ProcessStartInfo(context.FileName) + { + UseShellExecute = true, // required for the UAC "runas" verb + Verb = "runas", + }; + foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); + // NOTE: UseShellExecute = true cannot pass per-process env vars; an elevated launcher + // must forward context.EnvironmentVariables another way (e.g. a temp response file the + // host reads, or a broker that sets them) so the host still finds the controller pipe. + Process p = Process.Start(psi)!; + return Task.FromResult(new ProcessHandleAdapter(p)); +} +``` + +This example deliberately shows a sharp edge: elevation via the shell loses per-process environment variables, so the launcher is responsible for re-delivering them. The platform contract only requires that the host ends up with `context.EnvironmentVariables`. + +### 4. Container + +Run the test host inside a container and bridge the pipe. The returned handle tracks the `docker run` client process; killing it tears down the container. + +```csharp +public Task LaunchTestHostProcessAsync( + TestHostProcessLaunchContext context, CancellationToken cancellationToken) +{ + var args = new List { "run", "--rm", "--init" }; + foreach (var kvp in context.EnvironmentVariables) { args.Add("-e"); args.Add($"{kvp.Key}={kvp.Value}"); } + // Map the controller pipe into the container (Windows named pipe / Unix domain socket mount). + args.Add("test-image:latest"); + args.Add(context.FileName); + args.AddRange(context.Arguments); + + var psi = new ProcessStartInfo("docker") { UseShellExecute = false }; + foreach (string a in args) psi.ArgumentList.Add(a); + Process p = Process.Start(psi)!; + return Task.FromResult(new ProcessHandleAdapter(p)); +} +``` + +### 5. Remote (SSH) + +```csharp +public Task LaunchTestHostProcessAsync( + TestHostProcessLaunchContext context, CancellationToken cancellationToken) +{ + string env = string.Join(' ', context.EnvironmentVariables.Select(kv => $"{kv.Key}={Quote(kv.Value)}")); + string remoteCmd = $"{env} {Quote(context.FileName)} {string.Join(' ', context.Arguments.Select(Quote))}"; + + var psi = new ProcessStartInfo("ssh") { UseShellExecute = false }; + psi.ArgumentList.Add("user@remote-host"); + psi.ArgumentList.Add(remoteCmd); + Process ssh = Process.Start(psi)!; // tunnel the controller pipe over the SSH connection + return Task.FromResult(new ProcessHandleAdapter(ssh)); +} +``` + +## Alternatives considered + +### Reuse `ITestHostExecutionOrchestrator` + +MTP already ships an experimental `ITestHostExecutionOrchestrator` (`ITestHostOrchestratorManager.AddTestHostOrchestrator`). It was rejected as the vehicle because it sits **above** the controller: `OrchestrateTestHostExecutionAsync` runs in `TestHostOchestratorHost` and replaces the *entire* execution, returning only an exit code. An implementation would have to re-create everything `TestHostControllersTestHost` provides — environment-variable providers, the `MONITORTOHOST` IPC/PID handshake, and the `ITestHostProcessLifetimeHandler` fan-out that **hang dump and crash dump depend on**. That is the wrong granularity for "launch the process differently." The orchestrator remains the right tool for whole-run concerns (e.g. retry/repeat that re-runs the host). + +### Make the internal `IProcessHandler` replaceable via DI + +`IProcessHandler` / `IProcess` are `internal` and surface `Process`-specific members (e.g. `MainModule`). Exposing them publicly would leak implementation detail and over-commit the surface. A purpose-built, minimal `ITestHostProcessHandle` is cleaner and evolvable. + +### Do nothing (keep `Process.Start`) + +Leaves [#2784](https://github.com/microsoft/testfx/issues/2784) unsolvable on MTP for packaged apps and blocks the debugger/elevation/container/remote scenarios, all of which today require forking the platform. + +## Compatibility and conventions + +- **Experimental.** All new types and methods are gated behind `[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")]`, consistent with the other test-host-controller-era experimental APIs. +- **Public API tracking.** New members are added to `PublicAPI.Unshipped.txt` with the `[TPEXP]` prefix. +- **No `init` accessors** on any new public API, per repo policy. +- **No behavior change when unused.** If no launcher is registered, the platform behaves exactly as today (`Process.Start`), and the controller host is selected only when it already would be. + +## Open questions + +- **Handle vs PID surface.** Ship the `ITestHostProcessHandle` shape (supports container/remote) or start with the PID-only minimal variant and grow later? +- **CLI/debug integration.** Should the platform expose a built-in `--launcher`-style selector, or is builder/MSBuild registration sufficient for v1? +- **Cancellation semantics.** Define precisely what `Kill()` must guarantee for remote/container launchers (best-effort teardown vs. synchronous termination). +- **Multiple launchers.** Singleton for v1; is there ever a composition story (e.g. debugger + elevation), or do implementers compose manually? From 07af93a13f14f608c4e456f713c6b4e6853402e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Tue, 23 Jun 2026 09:09:08 +0200 Subject: [PATCH 2/7] Address RFC 017 review comments - 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> --- docs/RFCs/017-TestHost-Process-Launcher.md | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/RFCs/017-TestHost-Process-Launcher.md b/docs/RFCs/017-TestHost-Process-Launcher.md index 08399dfc71..cd1693272c 100644 --- a/docs/RFCs/017-TestHost-Process-Launcher.md +++ b/docs/RFCs/017-TestHost-Process-Launcher.md @@ -9,7 +9,7 @@ Introduce **`ITestHostProcessLauncher`**: a public, experimental Microsoft.Testing.Platform (MTP) extension point that lets an extension control **how** the out-of-process test host is launched, instead of the platform always doing `Process.Start`. The platform still owns everything around the launch — argument/environment preparation, the controller↔host IPC pipe, PID tracking, `ITestHostProcessLifetimeHandler` callbacks, and exit-code reconciliation — and simply delegates the single "create the process" step to the registered launcher. -The motivating scenario is testing **packaged WinUI/MSIX applications**, which cannot be started with `Process.Start` and must be activated by AUMID (see [#2784](https://github.com/microsoft/testfx/issues/2784)). But the abstraction is deliberately generic: the same hook enables launching the test host under a debugger, elevated, inside a container, or on a remote machine. +The motivating scenario is testing **WinUI applications** (see [#2784](https://github.com/microsoft/testfx/issues/2784)): **packaged/MSIX** apps in particular cannot be started with `Process.Start` and must be activated by AUMID, while unpackaged WinUI apps similarly benefit from a custom launch step. (#2784's title calls out the *unpackaged* case specifically; this launcher is the generic hook that serves both packaged and unpackaged WinUI scenarios.) But the abstraction is deliberately generic: the same hook enables launching the test host under a debugger, elevated, inside a container, or on a remote machine. ## Motivation @@ -168,12 +168,12 @@ internal sealed class WinUiAppxLauncher(IServiceProvider sp) : ITestHostProcessL public string Description => "Deploys and AUMID-activates a packaged WinUI test app."; public Task IsEnabledAsync() => Task.FromResult(true); - public async Task LaunchTestHostProcessAsync( + public Task LaunchTestHostProcessAsync( TestHostProcessLaunchContext context, CancellationToken cancellationToken) { // 1. Parse the .appxrecipe / AppxManifest.xml next to context.FileName to get the AUMID // and (in Developer Mode) register the loose layout: - // await new PackageManager().RegisterPackageByUriAsync(manifestUri, options); + // new PackageManager().RegisterPackageByUriAsync(manifestUri, options); string aumid = AppxManifest.ResolveAumid(context.FileName); // 2. Activate, passing the SAME args the platform prepared. @@ -183,7 +183,7 @@ internal sealed class WinUiAppxLauncher(IServiceProvider sp) : ITestHostProcessL // 3. Wrap the returned PID. The app inherits context.EnvironmentVariables // (incl. the MONITORTOHOST pipe name) via its activation/launch profile. - return new ProcessIdHandle((int)pid); + return Task.FromResult(new ProcessIdHandle((int)pid)); } } ``` @@ -260,7 +260,9 @@ public Task LaunchTestHostProcessAsync( public Task LaunchTestHostProcessAsync( TestHostProcessLaunchContext context, CancellationToken cancellationToken) { - string env = string.Join(' ', context.EnvironmentVariables.Select(kv => $"{kv.Key}={Quote(kv.Value)}")); + string env = string.Join(' ', context.EnvironmentVariables + .Where(kv => kv.Value is not null) + .Select(kv => $"{kv.Key}={Quote(kv.Value!)}")); // values are nullable; skip unset vars string remoteCmd = $"{env} {Quote(context.FileName)} {string.Join(' ', context.Arguments.Select(Quote))}"; var psi = new ProcessStartInfo("ssh") { UseShellExecute = false }; @@ -275,7 +277,7 @@ public Task LaunchTestHostProcessAsync( ### Reuse `ITestHostExecutionOrchestrator` -MTP already ships an experimental `ITestHostExecutionOrchestrator` (`ITestHostOrchestratorManager.AddTestHostOrchestrator`). It was rejected as the vehicle because it sits **above** the controller: `OrchestrateTestHostExecutionAsync` runs in `TestHostOchestratorHost` and replaces the *entire* execution, returning only an exit code. An implementation would have to re-create everything `TestHostControllersTestHost` provides — environment-variable providers, the `MONITORTOHOST` IPC/PID handshake, and the `ITestHostProcessLifetimeHandler` fan-out that **hang dump and crash dump depend on**. That is the wrong granularity for "launch the process differently." The orchestrator remains the right tool for whole-run concerns (e.g. retry/repeat that re-runs the host). +MTP already ships an experimental `ITestHostExecutionOrchestrator` (`ITestHostOrchestratorManager.AddTestHostOrchestrator`). It was rejected as the vehicle because it sits **above** the controller: `OrchestrateTestHostExecutionAsync` runs in `TestHostOrchestratorHost` and replaces the *entire* execution, returning only an exit code. An implementation would have to re-create everything `TestHostControllersTestHost` provides — environment-variable providers, the `MONITORTOHOST` IPC/PID handshake, and the `ITestHostProcessLifetimeHandler` fan-out that **hang dump and crash dump depend on**. That is the wrong granularity for "launch the process differently." The orchestrator remains the right tool for whole-run concerns (e.g. retry/repeat that re-runs the host). ### Make the internal `IProcessHandler` replaceable via DI From 9c20a11777d9d23173fa6ce8c3e54549f24eb68e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Wed, 24 Jun 2026 10:09:49 +0200 Subject: [PATCH 3/7] Fix MD051 broken link fragment in RFC 017 The 'Alternatives' link pointed to #alternatives but the heading is 'Alternatives considered' (#alternatives-considered). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- docs/RFCs/017-TestHost-Process-Launcher.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/RFCs/017-TestHost-Process-Launcher.md b/docs/RFCs/017-TestHost-Process-Launcher.md index cd1693272c..b6bf76c71d 100644 --- a/docs/RFCs/017-TestHost-Process-Launcher.md +++ b/docs/RFCs/017-TestHost-Process-Launcher.md @@ -26,7 +26,7 @@ Everything downstream only needs five things from the returned handle — `Id`, - **Elevation**: run the test host as administrator (UAC) or as another user. - **Container / remote**: launch the host inside a container (`docker run`) or on a remote device over SSH/WinRM, then bridge the pipe. -Today none of these is possible without forking the platform. The existing experimental `ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives)). This RFC adds the *minimal* hook at exactly the launch site. +Today none of these is possible without forking the platform. The existing experimental `ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives-considered)). This RFC adds the *minimal* hook at exactly the launch site. ## Goals From a059ea284b1e61b9f7d8fad4f8b986e480199a73 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 26 Jun 2026 13:19:12 +0200 Subject: [PATCH 4/7] Rework RFC 017 to a generic test host launcher 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> --- docs/RFCs/017-TestHost-Launcher.md | 390 +++++++++++++++++++++ docs/RFCs/017-TestHost-Process-Launcher.md | 302 ---------------- 2 files changed, 390 insertions(+), 302 deletions(-) create mode 100644 docs/RFCs/017-TestHost-Launcher.md delete mode 100644 docs/RFCs/017-TestHost-Process-Launcher.md diff --git a/docs/RFCs/017-TestHost-Launcher.md b/docs/RFCs/017-TestHost-Launcher.md new file mode 100644 index 0000000000..0281e2f567 --- /dev/null +++ b/docs/RFCs/017-TestHost-Launcher.md @@ -0,0 +1,390 @@ +# RFC 017 - Custom test host launcher + +- [ ] Approved in principle +- [x] Under discussion +- [x] Implementation +- [ ] Shipped + +## Summary + +Introduce **`ITestHostLauncher`**: a public, experimental Microsoft.Testing.Platform (MTP) +extension point that lets an extension control **how** the out-of-process test host is launched, +instead of the platform always doing `Process.Start`. The platform still owns everything around +the launch — argument/environment preparation, the controller↔host IPC pipe, PID tracking, +`ITestHostProcessLifetimeHandler` callbacks, and exit-code reconciliation — and simply delegates +the single "create and start the test host" step to the registered launcher. + +The hook is deliberately **agnostic of the launch mechanism**: the launcher does not have to start a +local OS process. It can deploy and activate a packaged application, launch a container, or start +the host on a remote machine. To make this explicit, the launcher returns an `ITestHostHandle` that +exposes only the lifecycle the platform needs (`WaitForExitAsync`, `ExitCode`, `HasExited`, +`Exited`, `Terminate`); a process id is *optional* and used purely for diagnostics. + +The motivating scenario is **packaging and deployment of MSIX-packaged applications — both UWP and +WinUI** (see [#2784](https://github.com/microsoft/testfx/issues/2784)): packaged/MSIX apps cannot be +started with `Process.Start` and must be deployed and then activated by AUMID. UWP and packaged WinUI +share this exact mechanism, which is why VSTest exposes a single `UwpTestHostRuntimeProvider` for +both; unpackaged apps similarly benefit from a custom deploy + launch step. The same hook also +enables launching the test host under a debugger, elevated, inside a container, or on a remote +machine. + +## Motivation + +MTP runs the test host out-of-process whenever a "test host controller" extension is active (hang +dump, crash dump, or any `ITestHostProcessLifetimeHandler` / `ITestHostEnvironmentVariableProvider`). +That work happens in `TestHostControllersTestHost`, which prepares a `ProcessStartInfo` (arguments, +environment variables including the `MONITORTOHOST` pipe name) and then launches the host with a +single call: + +```csharp +using IProcess testHostProcess = process.Start(processStartInfo); +``` + +Everything downstream only needs a handful of things from the returned handle — a way to observe +exit (`Exited`, `WaitForExitAsync()`, `ExitCode`, `HasExited`), optionally a PID for logging, and a +way to tear it down (`Kill`) — plus the child connecting back on the named pipe whose name was +injected via an environment variable. **`Process.Start` is the only assumption that does not hold +universally.** Several real scenarios need a different launch mechanism: + +- **Packaged WinUI/MSIX**: a packaged app must be deployed (in Developer Mode, register the loose + layout) and then activated by Application User Model ID (AUMID) via `IApplicationActivationManager`, + not started from an executable path. This is the blocker behind + [#2784](https://github.com/microsoft/testfx/issues/2784) and the reason VSTest's + `UwpTestHostRuntimeProvider` exists. +- **Debugger attach/launch**: start the host suspended (or under a debugger launcher such as + `vsdbg` / `WinDbg` / `dlv`) and only then resume. +- **Elevation**: run the test host as administrator (UAC) or as another user. +- **Container / remote**: launch the host inside a container (`docker run`) or on a remote device + over SSH/WinRM, then bridge the pipe — neither of which exposes a local, query-able PID. + +Today none of these is possible without forking the platform. The existing experimental +`ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives)). This +RFC adds the *minimal* hook at exactly the launch site. + +## Goals + +- Let an extension substitute the test host launch step while the platform keeps owning + argument/env preparation, IPC, lifetime-handler dispatch, and exit-code handling. +- Keep hang dump, crash dump, and all `ITestHostProcessLifetimeHandler` / + `ITestHostEnvironmentVariableProvider` extensions working unchanged when a custom launcher is + present. +- Be generic enough to cover WinUI deploy+activate, debugger, elevation, container, and remote + launch with one shape, **without assuming the launched thing is a local OS process**. +- Follow MTP's experimental-API conventions so the surface can evolve before stabilizing. + +## Non-goals + +- Replacing the *entire* run loop (that is `ITestHostExecutionOrchestrator`'s job). +- Remote **device deployment/bootstrapping** of the Windows App SDK framework + agent (VSTest's + `Microsoft.UniversalApps.Deployment` has no public redistributable; out of scope — local launch + only). +- Shipping a *complete* packaged UWP/WinUI (MSIX) deployment story. This RFC adds the platform hook; + a reference consumer (`Microsoft.Testing.Extensions.PackagedApp`) implements the deploy-and-launch path, + while packaged AUMID activation remains a separate follow-up. +- Changing the in-process (single-process, `ConsoleTestHost`) execution path. + +## Detailed design + +### Where it plugs in + +The hook lives in the **test host controllers** layer, next to the existing lifetime-handler and +environment-variable-provider extension points, and is registered through +`ITestHostControllersManager`. + +```csharp +namespace Microsoft.Testing.Platform.Extensions.TestHostControllers; + +/// +/// Allows an extension to control how the out-of-process test host is launched, +/// replacing the platform's default Process.Start behavior. +/// +[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] +public interface ITestHostLauncher : ITestHostControllersExtension // : IExtension +{ + /// + /// Creates and starts the test host. The platform has already prepared the file name, + /// arguments, and environment variables (including the controller IPC pipe name) carried by + /// . The implementation must return a handle the platform can monitor. + /// + Task LaunchTestHostAsync( + TestHostLaunchContext context, + CancellationToken cancellationToken); +} +``` + +The platform passes the fully-prepared launch information: + +```csharp +[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] +public sealed class TestHostLaunchContext +{ + public TestHostLaunchContext( + string fileName, + IReadOnlyList arguments, + IReadOnlyDictionary environmentVariables, + string? workingDirectory); + + /// The default test host executable path the platform would have started. + public string FileName { get; } + + /// Arguments, already including the test host controller PID option. + public IReadOnlyList Arguments { get; } + + /// + /// The final environment for the test host, after all + /// ran. Includes the + /// controller↔host IPC pipe name the host must connect back on. + /// + public IReadOnlyDictionary EnvironmentVariables { get; } + + /// The working directory, or null to inherit the current one. + public string? WorkingDirectory { get; } +} +``` + +And the launcher returns a launch-mechanism-agnostic handle (the platform adapts it to its internal +monitoring contract): + +```csharp +[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] +public interface ITestHostHandle +{ + event EventHandler Exited; + + /// The OS process id, when available. Null for container/remote launches. Logging only. + int? ProcessId { get; } + + int ExitCode { get; } + bool HasExited { get; } + Task WaitForExitAsync(); + + /// Best-effort teardown (e.g. when hang dump aborts the run). + void Terminate(); +} +``` + +Registration mirrors the existing methods on `ITestHostControllersManager`: + +```csharp +public interface ITestHostControllersManager +{ + // existing: AddEnvironmentVariableProvider(...), AddProcessLifetimeHandler(...) + + [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] + void AddTestHostLauncher(Func testHostLauncherFactory); + + [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] + void AddTestHostLauncher(CompositeExtensionFactory compositeServiceFactory) + where T : class, ITestHostLauncher; +} +``` + +### Platform integration (what changes inside MTP) + +1. **Swap the launch call.** In `TestHostControllersTestHost.InternalRunAsync`, at the current + `process.Start(processStartInfo)` site (after `BeforeTestHostProcessStartAsync` and after all + env-var providers ran), if a launcher is registered, build a `TestHostLaunchContext` from the + `ProcessStartInfo` and `await launcher.LaunchTestHostAsync(...)`. Otherwise keep the default + `process.Start`. The returned `ITestHostHandle` is adapted to the internal `IProcess` monitoring + contract — which only uses `Id` / `Exited` / `WaitForExitAsync` / `ExitCode` / `HasExited` / + `Kill`. Because `ProcessId` is optional, the premature-exit check is gated on `HasExited` only + (not on PID availability), so a launcher that returns no PID (container/remote/AUMID) is + monitored purely through the handle lifecycle and the IPC PID handshake. +2. **Force the controller host.** A launcher makes `RequireProcessRestart` `true` when one is + registered (computed in `TestHostControllersManager.BuildAsync`, checked in + `TestHostBuilder.Modes.cs`); without this, a run with *only* a launcher (no dump/lifetime + extension) would stay in-process and there would be nothing to launch. +3. **Singleton.** At most one launcher may be registered; a duplicate fails fast at build time with + a localized "only one test host launcher" error. +4. **Preserve ordering and services.** Because the call stays at the same point, + `ITestHostEnvironmentVariableProvider`, the `MONITORTOHOST` IPC pipe, the PID handshake, and + `ITestHostProcessLifetimeHandler` (and therefore hang dump and crash dump) all keep working with + no changes. + +### Contract requirements on the launcher + +- The launched host **must** receive `context.EnvironmentVariables` (so it connects back on the + controller pipe) and **must** be passed `context.Arguments`. +- The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `HasExited`, + `Exited`) and support `Terminate()` (hang dump terminates the host through it). +- `ProcessId` may be `null` when there is no local, query-able process (container/remote). It is + used only for diagnostics. +- If the launcher cannot start the host it should throw; the platform surfaces it as a + platform-setup failure. + +## Examples + +All examples assume the extension is registered on the builder, e.g. from a `…Extensions` helper: + +```csharp +builder.TestHostControllers.AddTestHostLauncher(sp => new MyLauncher(sp)); +``` + +### 1. Packaged WinUI / MSIX (the motivating case) + +Deploy the loose layout (Developer Mode) and activate the packaged app by AUMID instead of starting +an exe. The activated app self-hosts MTP (as the `MSTestRunnerWinUI` sample already does) and +connects back on the env-provided pipe. + +```csharp +public Task LaunchTestHostAsync( + TestHostLaunchContext context, CancellationToken cancellationToken) +{ + // 1. Parse the .appxrecipe / AppxManifest.xml next to context.FileName to get the AUMID + // and (in Developer Mode) register the loose layout: + // new PackageManager().RegisterPackageByUriAsync(manifestUri, options); + string aumid = AppxManifest.ResolveAumid(context.FileName); + + // 2. Activate, passing the SAME args the platform prepared. + var aam = (IApplicationActivationManager)new ApplicationActivationManager(); + aam.ActivateApplication(aumid, string.Join(' ', context.Arguments), ACTIVATEOPTIONS.AO_NONE, out uint pid); + + // 3. Wrap the returned PID. The app inherits context.EnvironmentVariables + // (incl. the MONITORTOHOST pipe name) via its activation/launch profile. + return Task.FromResult(new ProcessIdHandle((int)pid)); +} +``` + +> Note: enabling the controller→host pipe across the AppContainer sandbox requires a loopback/pipe-ACL +> step (e.g. `CheckNetIsolation LoopbackExempt` or granting the package SID on the pipe). That belongs +> to the package/deploy extension, not the platform. + +### 2. Launch under a debugger + +```csharp +public async Task LaunchTestHostAsync( + TestHostLaunchContext context, CancellationToken cancellationToken) +{ + var psi = new ProcessStartInfo(context.FileName) { UseShellExecute = false }; + foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); + foreach (var kvp in context.EnvironmentVariables) psi.Environment[kvp.Key] = kvp.Value; + psi.Environment["DOTNET_DefaultDiagnosticPortSuspend"] = "1"; // start suspended + + Process p = Process.Start(psi)!; + await DebuggerLauncher.AttachAsync(p.Id, cancellationToken); // e.g. vsdbg / WinDbg / dlv + await DebuggerLauncher.ResumeAsync(p.Id, cancellationToken); + return new ProcessHandleAdapter(p); +} +``` + +### 3. Elevated (run as administrator) + +```csharp +public Task LaunchTestHostAsync( + TestHostLaunchContext context, CancellationToken cancellationToken) +{ + var psi = new ProcessStartInfo(context.FileName) + { + UseShellExecute = true, // required for the UAC "runas" verb + Verb = "runas", + }; + foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); + // NOTE: UseShellExecute = true cannot pass per-process env vars; an elevated launcher + // must forward context.EnvironmentVariables another way (e.g. a temp response file the + // host reads, or a broker that sets them) so the host still finds the controller pipe. + Process p = Process.Start(psi)!; + return Task.FromResult(new ProcessHandleAdapter(p)); +} +``` + +This example deliberately shows a sharp edge: elevation via the shell loses per-process environment +variables, so the launcher is responsible for re-delivering them. The platform contract only +requires that the host ends up with `context.EnvironmentVariables`. + +### 4. Container + +Run the test host inside a container and bridge the pipe. The returned handle tracks the +`docker run` client process; `Terminate()` tears down the container. + +```csharp +public Task LaunchTestHostAsync( + TestHostLaunchContext context, CancellationToken cancellationToken) +{ + var args = new List { "run", "--rm", "--init" }; + foreach (var kvp in context.EnvironmentVariables) { args.Add("-e"); args.Add($"{kvp.Key}={kvp.Value}"); } + // Map the controller pipe into the container (Windows named pipe / Unix domain socket mount). + args.Add("test-image:latest"); + args.Add(context.FileName); + args.AddRange(context.Arguments); + + var psi = new ProcessStartInfo("docker") { UseShellExecute = false }; + foreach (string a in args) psi.ArgumentList.Add(a); + Process p = Process.Start(psi)!; + return Task.FromResult(new ProcessHandleAdapter(p)); +} +``` + +### 5. Remote (SSH) + +```csharp +public Task LaunchTestHostAsync( + TestHostLaunchContext context, CancellationToken cancellationToken) +{ + string env = string.Join(' ', context.EnvironmentVariables + .Where(kv => kv.Value is not null) + .Select(kv => $"{kv.Key}={Quote(kv.Value!)}")); // values are nullable; skip unset vars + string remoteCmd = $"{env} {Quote(context.FileName)} {string.Join(' ', context.Arguments.Select(Quote))}"; + + var psi = new ProcessStartInfo("ssh") { UseShellExecute = false }; + psi.ArgumentList.Add("user@remote-host"); + psi.ArgumentList.Add(remoteCmd); + Process ssh = Process.Start(psi)!; // tunnel the controller pipe over the SSH connection + // The handle tracks the local ssh client; ProcessId here is the ssh client PID (diagnostic only). + return Task.FromResult(new ProcessHandleAdapter(ssh)); +} +``` + +## Alternatives considered + +### Reuse `ITestHostExecutionOrchestrator` + +MTP already ships an experimental `ITestHostExecutionOrchestrator` +(`ITestHostOrchestratorManager.AddTestHostOrchestrator`). It was rejected as the vehicle because it +sits **above** the controller: `OrchestrateTestHostExecutionAsync` runs in +`TestHostOrchestratorHost` and replaces the *entire* execution, returning only an exit code. An +implementation would have to re-create everything `TestHostControllersTestHost` provides — +environment-variable providers, the `MONITORTOHOST` IPC/PID handshake, and the +`ITestHostProcessLifetimeHandler` fan-out that **hang dump and crash dump depend on**. That is the +wrong granularity for "launch the host differently." The orchestrator remains the right tool for +whole-run concerns (e.g. retry/repeat that re-runs the host). + +### Make the internal `IProcessHandler` replaceable via DI + +`IProcessHandler` / `IProcess` are `internal` and surface `Process`-specific members (e.g. +`MainModule`). Exposing them publicly would leak implementation detail, over-commit the surface, and +bake in the "it's always a local process" assumption. A purpose-built, minimal, mechanism-agnostic +`ITestHostHandle` is cleaner and evolvable. + +### A process-centric `ITestHostProcessLauncher` returning a `ProcessId` + +An earlier draft of this RFC named the hook `ITestHostProcessLauncher` and returned an +`ITestHostProcessHandle` whose `ProcessId` was mandatory. That over-commits to "the test host is a +local OS process," which is false for container and remote launches and awkward for AUMID-activated +apps. The current design renames the types to drop "Process", makes `ProcessId` optional, and names +the teardown `Terminate()` instead of `Kill()`. + +### Do nothing (keep `Process.Start`) + +Leaves [#2784](https://github.com/microsoft/testfx/issues/2784) unsolvable on MTP for packaged apps +and blocks the debugger/elevation/container/remote scenarios, all of which today require forking the +platform. + +## Compatibility and conventions + +- **Experimental.** All new types and methods are gated behind + `[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")]`, consistent + with the other test-host-controller-era experimental APIs. +- **Public API tracking.** New members are added to `PublicAPI.Unshipped.txt` with the `[TPEXP]` + prefix. +- **No `init` accessors** on any new public API, per repo policy. +- **No behavior change when unused.** If no launcher is registered, the platform behaves exactly as + today (`Process.Start`), and the controller host is selected only when it already would be. + +## Open questions + +- **CLI/debug integration.** Should the platform expose a built-in `--launcher`-style selector, or + is builder/MSBuild registration sufficient for v1? +- **Cancellation semantics.** Define precisely what `Terminate()` must guarantee for remote/container + launchers (best-effort teardown vs. synchronous termination). +- **Multiple launchers.** Singleton for v1; is there ever a composition story (e.g. debugger + + elevation), or do implementers compose manually? diff --git a/docs/RFCs/017-TestHost-Process-Launcher.md b/docs/RFCs/017-TestHost-Process-Launcher.md deleted file mode 100644 index b6bf76c71d..0000000000 --- a/docs/RFCs/017-TestHost-Process-Launcher.md +++ /dev/null @@ -1,302 +0,0 @@ -# RFC 017 - Custom test host process launcher - -- [ ] Approved in principle -- [x] Under discussion -- [ ] Implementation -- [ ] Shipped - -## Summary - -Introduce **`ITestHostProcessLauncher`**: a public, experimental Microsoft.Testing.Platform (MTP) extension point that lets an extension control **how** the out-of-process test host is launched, instead of the platform always doing `Process.Start`. The platform still owns everything around the launch — argument/environment preparation, the controller↔host IPC pipe, PID tracking, `ITestHostProcessLifetimeHandler` callbacks, and exit-code reconciliation — and simply delegates the single "create the process" step to the registered launcher. - -The motivating scenario is testing **WinUI applications** (see [#2784](https://github.com/microsoft/testfx/issues/2784)): **packaged/MSIX** apps in particular cannot be started with `Process.Start` and must be activated by AUMID, while unpackaged WinUI apps similarly benefit from a custom launch step. (#2784's title calls out the *unpackaged* case specifically; this launcher is the generic hook that serves both packaged and unpackaged WinUI scenarios.) But the abstraction is deliberately generic: the same hook enables launching the test host under a debugger, elevated, inside a container, or on a remote machine. - -## Motivation - -MTP runs the test host out-of-process whenever a "test host controller" extension is active (hang dump, crash dump, or any `ITestHostProcessLifetimeHandler` / `ITestHostEnvironmentVariableProvider`). That work happens in `TestHostControllersTestHost`, which prepares a `ProcessStartInfo` (arguments, environment variables including the `MONITORTOHOST` pipe name) and then launches the host with a single call: - -```csharp -using IProcess testHostProcess = process.Start(processStartInfo); -``` - -Everything downstream only needs five things from the returned handle — `Id`, the `Exited` event, `WaitForExitAsync()`, `ExitCode`, and `Kill()` — plus the child connecting back on the named pipe whose name was injected via an environment variable. **`Process.Start` is the only assumption that does not hold universally.** Several real scenarios need a different launch mechanism: - -- **Packaged WinUI/MSIX**: a packaged app must be activated by Application User Model ID (AUMID) via `IApplicationActivationManager`, not started from an executable path. This is the blocker behind [#2784](https://github.com/microsoft/testfx/issues/2784) and the reason VSTest's `UwpTestHostRuntimeProvider` exists. -- **Debugger attach/launch**: start the host suspended (or under a debugger launcher such as `vsdbg` / `WinDbg` / `dlv`) and only then resume. -- **Elevation**: run the test host as administrator (UAC) or as another user. -- **Container / remote**: launch the host inside a container (`docker run`) or on a remote device over SSH/WinRM, then bridge the pipe. - -Today none of these is possible without forking the platform. The existing experimental `ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives-considered)). This RFC adds the *minimal* hook at exactly the launch site. - -## Goals - -- Let an extension substitute the test host process creation step while the platform keeps owning argument/env preparation, IPC, lifetime-handler dispatch, and exit-code handling. -- Keep hang dump, crash dump, and all `ITestHostProcessLifetimeHandler` / `ITestHostEnvironmentVariableProvider` extensions working unchanged when a custom launcher is present. -- Be generic enough to cover WinUI activation, debugger, elevation, container, and remote launch with one shape. -- Follow MTP's experimental-API conventions so the surface can evolve before stabilizing. - -## Non-goals - -- Replacing the *entire* run loop (that is `ITestHostExecutionOrchestrator`'s job). -- Remote **device deployment/bootstrapping** of the Windows App SDK framework + agent (VSTest's `Microsoft.UniversalApps.Deployment` has no public redistributable; out of scope — local launch only). -- Shipping the WinUI deployment extension itself. This RFC only adds the platform hook; the WinUI extension is a separate deliverable that consumes it. -- Changing the in-process (single-process, `ConsoleTestHost`) execution path. - -## Detailed design - -### Where it plugs in - -The hook lives in the **test host controllers** layer, next to the existing lifetime-handler and environment-variable-provider extension points, and is registered through `ITestHostControllersManager`. - -```csharp -namespace Microsoft.Testing.Platform.Extensions.TestHostControllers; - -/// -/// Allows an extension to control how the out-of-process test host is launched, -/// replacing the platform's default Process.Start behavior. -/// -[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] -public interface ITestHostProcessLauncher : ITestHostControllersExtension // : IExtension -{ - /// - /// Creates and starts the test host process. The platform has already prepared the - /// file name, arguments, and environment variables (including the controller IPC pipe - /// name). The implementation must return a handle the platform can monitor. - /// - Task LaunchTestHostProcessAsync( - TestHostProcessLaunchContext context, - CancellationToken cancellationToken); -} -``` - -The platform passes the fully-prepared launch information: - -```csharp -[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] -public sealed class TestHostProcessLaunchContext -{ - public TestHostProcessLaunchContext( - string fileName, - IReadOnlyList arguments, - IReadOnlyDictionary environmentVariables, - string workingDirectory); - - /// The default test host executable path the platform would have started. - public string FileName { get; } - - /// Arguments, already including the test host controller PID option. - public IReadOnlyList Arguments { get; } - - /// - /// The final environment for the test host, after all - /// ran. Includes the - /// controller↔host IPC pipe name the host must connect back on. - /// - public IReadOnlyDictionary EnvironmentVariables { get; } - - public string WorkingDirectory { get; } -} -``` - -And the launcher returns a public-safe handle (a subset of the internal `IProcess`, which the platform adapts to its monitoring): - -```csharp -[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] -public interface ITestHostProcessHandle -{ - int ProcessId { get; } - int ExitCode { get; } - bool HasExited { get; } - event EventHandler Exited; - Task WaitForExitAsync(); - void Kill(); -} -``` - -Registration mirrors the existing methods on `ITestHostControllersManager`: - -```csharp -public interface ITestHostControllersManager -{ - // existing: AddEnvironmentVariableProvider(...), AddProcessLifetimeHandler(...) - - [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] - void AddTestHostProcessLauncher( - Func factory); - - [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] - void AddTestHostProcessLauncher(CompositeExtensionFactory factory) - where T : class, ITestHostProcessLauncher; -} -``` - -### Platform integration (what changes inside MTP) - -1. **Swap the launch call.** In `TestHostControllersTestHost.InternalRunAsync`, at the current `process.Start(processStartInfo)` site (after `BeforeTestHostProcessStartAsync` and after all env-var providers ran), if a launcher is registered, build a `TestHostProcessLaunchContext` from the `ProcessStartInfo` and `await launcher.LaunchTestHostProcessAsync(...)`. Otherwise keep the default `process.Start`. The returned `ITestHostProcessHandle` is adapted to the internal `IProcess` monitoring contract — which already only uses `Id` / `Exited` / `WaitForExitAsync` / `ExitCode` / `Kill`. -2. **Force the controller host.** Add the launcher to `TestHostControllerConfiguration` and make `RequireProcessRestart` `true` when a launcher is registered. Host selection is gated on `RequireProcessRestart` (`TestHostControllersManager.BuildAsync` → checked in `TestHostBuilder.Modes.cs`); without this, a run with *only* a launcher (no dump/lifetime extension) would stay in-process and there would be nothing to launch. -3. **Singleton.** Validate that at most one launcher is registered, matching the orchestrator's "Multiple … not supported" behavior. A duplicate registration fails fast at build time. -4. **Preserve ordering and services.** Because the call stays at the same point, `ITestHostEnvironmentVariableProvider`, the `MONITORTOHOST` IPC pipe, the PID handshake, and `ITestHostProcessLifetimeHandler` (and therefore hang dump and crash dump) all keep working with no changes. - -### Contract requirements on the launcher - -- The launched process **must** be a real OS process with a queryable PID, it **must** inherit/receive `context.EnvironmentVariables` (so it connects back on the controller pipe), and it **must** be passed `context.Arguments`. -- The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `Exited`) and support `Kill()` (hang dump terminates the host through it). -- If the launcher cannot start the process it should throw; the platform surfaces it as a platform-setup failure. - -### Minimal-surface variant - -If we prefer not to expose a handle type initially, the launcher can instead return `Task` (the PID) and the platform wraps it internally via the existing `IProcessHandler.GetProcessById(pid)`, which already provides `Id` / `Exited` / `WaitForExit` / `ExitCode` / `Kill`. That reduces the public surface to the interface + context only, at the cost of requiring the launched thing to be a local, query-able OS process (true for AUMID-activated packaged apps; not true for a process living inside a container or on a remote host). Because the goal is also to support container/remote, the RFC proposes the **handle-returning** shape as primary. - -## Examples - -All examples assume the extension is registered on the builder, e.g. from a `…Extensions` helper: - -```csharp -builder.TestHostControllers.AddTestHostProcessLauncher(sp => new MyLauncher(sp)); -``` - -### 1. Packaged WinUI / MSIX (the motivating case) - -Activate the packaged app by AUMID instead of starting an exe. The activated app self-hosts MTP (as the `MSTestRunnerWinUI` sample already does) and connects back on the env-provided pipe. - -```csharp -internal sealed class WinUiAppxLauncher(IServiceProvider sp) : ITestHostProcessLauncher -{ - public string Uid => nameof(WinUiAppxLauncher); - public string Version => "1.0.0"; - public string DisplayName => "WinUI MSIX launcher"; - public string Description => "Deploys and AUMID-activates a packaged WinUI test app."; - public Task IsEnabledAsync() => Task.FromResult(true); - - public Task LaunchTestHostProcessAsync( - TestHostProcessLaunchContext context, CancellationToken cancellationToken) - { - // 1. Parse the .appxrecipe / AppxManifest.xml next to context.FileName to get the AUMID - // and (in Developer Mode) register the loose layout: - // new PackageManager().RegisterPackageByUriAsync(manifestUri, options); - string aumid = AppxManifest.ResolveAumid(context.FileName); - - // 2. Activate, passing the SAME args the platform prepared. - var aam = (IApplicationActivationManager)new ApplicationActivationManager(); - aam.ActivateApplication(aumid, string.Join(' ', context.Arguments), - ACTIVATEOPTIONS.AO_NONE, out uint pid); - - // 3. Wrap the returned PID. The app inherits context.EnvironmentVariables - // (incl. the MONITORTOHOST pipe name) via its activation/launch profile. - return Task.FromResult(new ProcessIdHandle((int)pid)); - } -} -``` - -> Note: enabling the controller→host pipe across the AppContainer sandbox requires a loopback/pipe-ACL step (e.g. `CheckNetIsolation LoopbackExempt` or granting the package SID on the pipe). That belongs to the WinUI extension, not the platform. - -### 2. Launch under a debugger - -Start the host suspended and attach a debugger before it runs (useful for `--debug`-style flows or CI repro). - -```csharp -public async Task LaunchTestHostProcessAsync( - TestHostProcessLaunchContext context, CancellationToken cancellationToken) -{ - var psi = new ProcessStartInfo(context.FileName) { UseShellExecute = false }; - foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); - foreach (var kvp in context.EnvironmentVariables) psi.Environment[kvp.Key] = kvp.Value; - psi.Environment["DOTNET_DefaultDiagnosticPortSuspend"] = "1"; // start suspended - - Process p = Process.Start(psi)!; - await DebuggerLauncher.AttachAsync(p.Id, cancellationToken); // e.g. vsdbg / WinDbg / dlv - await DebuggerLauncher.ResumeAsync(p.Id, cancellationToken); - return new ProcessHandleAdapter(p); -} -``` - -### 3. Elevated (run as administrator) - -```csharp -public Task LaunchTestHostProcessAsync( - TestHostProcessLaunchContext context, CancellationToken cancellationToken) -{ - var psi = new ProcessStartInfo(context.FileName) - { - UseShellExecute = true, // required for the UAC "runas" verb - Verb = "runas", - }; - foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); - // NOTE: UseShellExecute = true cannot pass per-process env vars; an elevated launcher - // must forward context.EnvironmentVariables another way (e.g. a temp response file the - // host reads, or a broker that sets them) so the host still finds the controller pipe. - Process p = Process.Start(psi)!; - return Task.FromResult(new ProcessHandleAdapter(p)); -} -``` - -This example deliberately shows a sharp edge: elevation via the shell loses per-process environment variables, so the launcher is responsible for re-delivering them. The platform contract only requires that the host ends up with `context.EnvironmentVariables`. - -### 4. Container - -Run the test host inside a container and bridge the pipe. The returned handle tracks the `docker run` client process; killing it tears down the container. - -```csharp -public Task LaunchTestHostProcessAsync( - TestHostProcessLaunchContext context, CancellationToken cancellationToken) -{ - var args = new List { "run", "--rm", "--init" }; - foreach (var kvp in context.EnvironmentVariables) { args.Add("-e"); args.Add($"{kvp.Key}={kvp.Value}"); } - // Map the controller pipe into the container (Windows named pipe / Unix domain socket mount). - args.Add("test-image:latest"); - args.Add(context.FileName); - args.AddRange(context.Arguments); - - var psi = new ProcessStartInfo("docker") { UseShellExecute = false }; - foreach (string a in args) psi.ArgumentList.Add(a); - Process p = Process.Start(psi)!; - return Task.FromResult(new ProcessHandleAdapter(p)); -} -``` - -### 5. Remote (SSH) - -```csharp -public Task LaunchTestHostProcessAsync( - TestHostProcessLaunchContext context, CancellationToken cancellationToken) -{ - string env = string.Join(' ', context.EnvironmentVariables - .Where(kv => kv.Value is not null) - .Select(kv => $"{kv.Key}={Quote(kv.Value!)}")); // values are nullable; skip unset vars - string remoteCmd = $"{env} {Quote(context.FileName)} {string.Join(' ', context.Arguments.Select(Quote))}"; - - var psi = new ProcessStartInfo("ssh") { UseShellExecute = false }; - psi.ArgumentList.Add("user@remote-host"); - psi.ArgumentList.Add(remoteCmd); - Process ssh = Process.Start(psi)!; // tunnel the controller pipe over the SSH connection - return Task.FromResult(new ProcessHandleAdapter(ssh)); -} -``` - -## Alternatives considered - -### Reuse `ITestHostExecutionOrchestrator` - -MTP already ships an experimental `ITestHostExecutionOrchestrator` (`ITestHostOrchestratorManager.AddTestHostOrchestrator`). It was rejected as the vehicle because it sits **above** the controller: `OrchestrateTestHostExecutionAsync` runs in `TestHostOrchestratorHost` and replaces the *entire* execution, returning only an exit code. An implementation would have to re-create everything `TestHostControllersTestHost` provides — environment-variable providers, the `MONITORTOHOST` IPC/PID handshake, and the `ITestHostProcessLifetimeHandler` fan-out that **hang dump and crash dump depend on**. That is the wrong granularity for "launch the process differently." The orchestrator remains the right tool for whole-run concerns (e.g. retry/repeat that re-runs the host). - -### Make the internal `IProcessHandler` replaceable via DI - -`IProcessHandler` / `IProcess` are `internal` and surface `Process`-specific members (e.g. `MainModule`). Exposing them publicly would leak implementation detail and over-commit the surface. A purpose-built, minimal `ITestHostProcessHandle` is cleaner and evolvable. - -### Do nothing (keep `Process.Start`) - -Leaves [#2784](https://github.com/microsoft/testfx/issues/2784) unsolvable on MTP for packaged apps and blocks the debugger/elevation/container/remote scenarios, all of which today require forking the platform. - -## Compatibility and conventions - -- **Experimental.** All new types and methods are gated behind `[Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")]`, consistent with the other test-host-controller-era experimental APIs. -- **Public API tracking.** New members are added to `PublicAPI.Unshipped.txt` with the `[TPEXP]` prefix. -- **No `init` accessors** on any new public API, per repo policy. -- **No behavior change when unused.** If no launcher is registered, the platform behaves exactly as today (`Process.Start`), and the controller host is selected only when it already would be. - -## Open questions - -- **Handle vs PID surface.** Ship the `ITestHostProcessHandle` shape (supports container/remote) or start with the PID-only minimal variant and grow later? -- **CLI/debug integration.** Should the platform expose a built-in `--launcher`-style selector, or is builder/MSBuild registration sufficient for v1? -- **Cancellation semantics.** Define precisely what `Kill()` must guarantee for remote/container launchers (best-effort teardown vs. synchronous termination). -- **Multiple launchers.** Singleton for v1; is there ever a composition story (e.g. debugger + elevation), or do implementers compose manually? From 9a62128b2d494c04a1d0b332bb326ecbdd85e04a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 26 Jun 2026 13:42:06 +0200 Subject: [PATCH 5/7] Address review feedback and fix markdown lint in RFC 017 - 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> --- docs/RFCs/017-TestHost-Launcher.md | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) diff --git a/docs/RFCs/017-TestHost-Launcher.md b/docs/RFCs/017-TestHost-Launcher.md index 0281e2f567..96acd0e0f1 100644 --- a/docs/RFCs/017-TestHost-Launcher.md +++ b/docs/RFCs/017-TestHost-Launcher.md @@ -58,7 +58,7 @@ universally.** Several real scenarios need a different launch mechanism: over SSH/WinRM, then bridge the pipe — neither of which exposes a local, query-able PID. Today none of these is possible without forking the platform. The existing experimental -`ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives)). This +`ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives-considered)). This RFC adds the *minimal* hook at exactly the launch site. ## Goals @@ -203,8 +203,11 @@ public interface ITestHostControllersManager ### Contract requirements on the launcher -- The launched host **must** receive `context.EnvironmentVariables` (so it connects back on the - controller pipe) and **must** be passed `context.Arguments`. +- The launched host **must** end up with the values in `context.EnvironmentVariables` (so it connects + back on the controller pipe) and **must** receive `context.Arguments`. *How* those values reach the + host is left to the launcher — they can be inherited from the environment, passed as activation + arguments, or bridged through a broker. AUMID activation in particular cannot set per-launch + environment variables, so a packaged-app launcher must transfer them another way. - The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `HasExited`, `Exited`) and support `Terminate()` (hang dump terminates the host through it). - `ProcessId` may be `null` when there is no local, query-able process (container/remote). It is @@ -223,8 +226,10 @@ builder.TestHostControllers.AddTestHostLauncher(sp => new MyLauncher(sp)); ### 1. Packaged WinUI / MSIX (the motivating case) Deploy the loose layout (Developer Mode) and activate the packaged app by AUMID instead of starting -an exe. The activated app self-hosts MTP (as the `MSTestRunnerWinUI` sample already does) and -connects back on the env-provided pipe. +an exe. The activated app self-hosts MTP (as the `MSTestRunnerWinUI` sample already does for the +in-process case); the launcher is responsible for getting the controller pipe name and correlation +id to it so it can connect back, since AUMID activation does not flow per-launch environment +variables on its own. ```csharp public Task LaunchTestHostAsync( @@ -235,12 +240,16 @@ public Task LaunchTestHostAsync( // new PackageManager().RegisterPackageByUriAsync(manifestUri, options); string aumid = AppxManifest.ResolveAumid(context.FileName); - // 2. Activate, passing the SAME args the platform prepared. + // 2. Activate, passing the SAME args the platform prepared. AUMID activation takes a single + // command-line string, so the launcher must escape/quote context.Arguments (e.g. with a + // PasteArguments-style helper) to preserve what ProcessStartInfo.ArgumentList would have done. var aam = (IApplicationActivationManager)new ApplicationActivationManager(); - aam.ActivateApplication(aumid, string.Join(' ', context.Arguments), ACTIVATEOPTIONS.AO_NONE, out uint pid); + aam.ActivateApplication(aumid, PasteArguments(context.Arguments), ACTIVATEOPTIONS.AO_NONE, out uint pid); - // 3. Wrap the returned PID. The app inherits context.EnvironmentVariables - // (incl. the MONITORTOHOST pipe name) via its activation/launch profile. + // 3. Wrap the returned PID. AUMID activation cannot set per-launch environment variables, so the + // launcher must bridge the values the host needs from context.EnvironmentVariables (the + // MONITORTOHOST pipe name, correlation id, etc.) another way — e.g. activation arguments or a + // broker process the activated app reads on startup. return Task.FromResult(new ProcessIdHandle((int)pid)); } ``` From 278c3e443697241567c4879954c3c9674104c2c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 26 Jun 2026 14:16:09 +0200 Subject: [PATCH 6/7] Refine ITestHostHandle in RFC 017 per review - 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> --- docs/RFCs/017-TestHost-Launcher.md | 59 +++++++++++++++++------------- 1 file changed, 33 insertions(+), 26 deletions(-) diff --git a/docs/RFCs/017-TestHost-Launcher.md b/docs/RFCs/017-TestHost-Launcher.md index 96acd0e0f1..94be217c6f 100644 --- a/docs/RFCs/017-TestHost-Launcher.md +++ b/docs/RFCs/017-TestHost-Launcher.md @@ -18,7 +18,7 @@ The hook is deliberately **agnostic of the launch mechanism**: the launcher does local OS process. It can deploy and activate a packaged application, launch a container, or start the host on a remote machine. To make this explicit, the launcher returns an `ITestHostHandle` that exposes only the lifecycle the platform needs (`WaitForExitAsync`, `ExitCode`, `HasExited`, -`Exited`, `Terminate`); a process id is *optional* and used purely for diagnostics. +`Terminate`) plus an optional free-form `Identifier` string used purely for diagnostics. The motivating scenario is **packaging and deployment of MSIX-packaged applications — both UWP and WinUI** (see [#2784](https://github.com/microsoft/testfx/issues/2784)): packaged/MSIX apps cannot be @@ -41,8 +41,8 @@ using IProcess testHostProcess = process.Start(processStartInfo); ``` Everything downstream only needs a handful of things from the returned handle — a way to observe -exit (`Exited`, `WaitForExitAsync()`, `ExitCode`, `HasExited`), optionally a PID for logging, and a -way to tear it down (`Kill`) — plus the child connecting back on the named pipe whose name was +exit (`WaitForExitAsync()`, `ExitCode`, `HasExited`), optionally an identifier for logging, and a +way to tear it down — plus the child connecting back on the named pipe whose name was injected via an environment variable. **`Process.Start` is the only assumption that does not hold universally.** Several real scenarios need a different launch mechanism: @@ -55,7 +55,7 @@ universally.** Several real scenarios need a different launch mechanism: `vsdbg` / `WinDbg` / `dlv`) and only then resume. - **Elevation**: run the test host as administrator (UAC) or as another user. - **Container / remote**: launch the host inside a container (`docker run`) or on a remote device - over SSH/WinRM, then bridge the pipe — neither of which exposes a local, query-able PID. + over SSH/WinRM, then bridge the pipe — neither of which exposes a local, queryable PID. Today none of these is possible without forking the platform. The existing experimental `ITestHostExecutionOrchestrator` sits at the wrong layer (see [Alternatives](#alternatives-considered)). This @@ -149,10 +149,11 @@ monitoring contract): [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] public interface ITestHostHandle { - event EventHandler Exited; - - /// The OS process id, when available. Null for container/remote launches. Logging only. - int? ProcessId { get; } + /// + /// Free-form diagnostic identifier (a PID, container id, remote host:pid, …) or null. The + /// platform never relies on it for control flow. + /// + string? Identifier { get; } int ExitCode { get; } bool HasExited { get; } @@ -186,10 +187,11 @@ public interface ITestHostControllersManager env-var providers ran), if a launcher is registered, build a `TestHostLaunchContext` from the `ProcessStartInfo` and `await launcher.LaunchTestHostAsync(...)`. Otherwise keep the default `process.Start`. The returned `ITestHostHandle` is adapted to the internal `IProcess` monitoring - contract — which only uses `Id` / `Exited` / `WaitForExitAsync` / `ExitCode` / `HasExited` / - `Kill`. Because `ProcessId` is optional, the premature-exit check is gated on `HasExited` only - (not on PID availability), so a launcher that returns no PID (container/remote/AUMID) is - monitored purely through the handle lifecycle and the IPC PID handshake. + contract — which only uses `WaitForExitAsync` / `ExitCode` / `HasExited` / `Kill` (and an + internal `Exited` event synthesized from the exit task for an informational log). The + premature-exit check is gated on `HasExited` only (not on whether an identifier is available), so + a launcher that returns no identifier (container/remote/AUMID) is monitored purely through the + handle lifecycle and the IPC PID handshake. 2. **Force the controller host.** A launcher makes `RequireProcessRestart` `true` when one is registered (computed in `TestHostControllersManager.BuildAsync`, checked in `TestHostBuilder.Modes.cs`); without this, a run with *only* a launcher (no dump/lifetime @@ -208,10 +210,11 @@ public interface ITestHostControllersManager host is left to the launcher — they can be inherited from the environment, passed as activation arguments, or bridged through a broker. AUMID activation in particular cannot set per-launch environment variables, so a packaged-app launcher must transfer them another way. -- The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `HasExited`, - `Exited`) and support `Terminate()` (hang dump terminates the host through it). -- `ProcessId` may be `null` when there is no local, query-able process (container/remote). It is - used only for diagnostics. +- The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `HasExited`) and + support `Terminate()` (hang dump terminates the host through it). `WaitForExitAsync` may be awaited + more than once. +- `Identifier` is an optional free-form diagnostic string (PID, container id, remote `host:pid`, …) + and may be `null`. The platform never relies on it for control flow. - If the launcher cannot start the host it should throw; the platform surfaces it as a platform-setup failure. @@ -246,11 +249,12 @@ public Task LaunchTestHostAsync( var aam = (IApplicationActivationManager)new ApplicationActivationManager(); aam.ActivateApplication(aumid, PasteArguments(context.Arguments), ACTIVATEOPTIONS.AO_NONE, out uint pid); - // 3. Wrap the returned PID. AUMID activation cannot set per-launch environment variables, so the + // 3. Wrap the activated app. AUMID activation cannot set per-launch environment variables, so the // launcher must bridge the values the host needs from context.EnvironmentVariables (the // MONITORTOHOST pipe name, correlation id, etc.) another way — e.g. activation arguments or a - // broker process the activated app reads on startup. - return Task.FromResult(new ProcessIdHandle((int)pid)); + // broker process the activated app reads on startup. The handle surfaces the activated PID as + // its (diagnostic-only) Identifier. + return Task.FromResult(new ActivatedAppHandle(pid)); } ``` @@ -266,7 +270,8 @@ public async Task LaunchTestHostAsync( { var psi = new ProcessStartInfo(context.FileName) { UseShellExecute = false }; foreach (string arg in context.Arguments) psi.ArgumentList.Add(arg); - foreach (var kvp in context.EnvironmentVariables) psi.Environment[kvp.Key] = kvp.Value; + foreach (var kvp in context.EnvironmentVariables.Where(kv => kv.Value is not null)) + psi.Environment[kvp.Key] = kvp.Value; // skip unset (null) vars psi.Environment["DOTNET_DefaultDiagnosticPortSuspend"] = "1"; // start suspended Process p = Process.Start(psi)!; @@ -310,7 +315,7 @@ public Task LaunchTestHostAsync( TestHostLaunchContext context, CancellationToken cancellationToken) { var args = new List { "run", "--rm", "--init" }; - foreach (var kvp in context.EnvironmentVariables) { args.Add("-e"); args.Add($"{kvp.Key}={kvp.Value}"); } + foreach (var kvp in context.EnvironmentVariables.Where(kv => kv.Value is not null)) { args.Add("-e"); args.Add($"{kvp.Key}={kvp.Value}"); } // skip unset (null) vars // Map the controller pipe into the container (Windows named pipe / Unix domain socket mount). args.Add("test-image:latest"); args.Add(context.FileName); @@ -338,7 +343,7 @@ public Task LaunchTestHostAsync( psi.ArgumentList.Add("user@remote-host"); psi.ArgumentList.Add(remoteCmd); Process ssh = Process.Start(psi)!; // tunnel the controller pipe over the SSH connection - // The handle tracks the local ssh client; ProcessId here is the ssh client PID (diagnostic only). + // The handle tracks the local ssh client; its Identifier could be the ssh client PID (diagnostic only). return Task.FromResult(new ProcessHandleAdapter(ssh)); } ``` @@ -367,10 +372,12 @@ bake in the "it's always a local process" assumption. A purpose-built, minimal, ### A process-centric `ITestHostProcessLauncher` returning a `ProcessId` An earlier draft of this RFC named the hook `ITestHostProcessLauncher` and returned an -`ITestHostProcessHandle` whose `ProcessId` was mandatory. That over-commits to "the test host is a -local OS process," which is false for container and remote launches and awkward for AUMID-activated -apps. The current design renames the types to drop "Process", makes `ProcessId` optional, and names -the teardown `Terminate()` instead of `Kill()`. +`ITestHostProcessHandle` whose `ProcessId` was a mandatory `int`. That over-commits to "the test host +is a local OS process," which is false for container and remote launches and awkward for +AUMID-activated apps. The current design renames the types to drop "Process", replaces the `int` +process id with an optional free-form `string Identifier` (diagnostic only), drops the redundant +`Exited` event in favour of `WaitForExitAsync`, and names the teardown `Terminate()` instead of +`Kill()`. ### Do nothing (keep `Process.Start`) From fbce074c5430318e2f3e148e6ef2bcbbf8568c9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Amaury=20Lev=C3=A9?= Date: Fri, 26 Jun 2026 16:06:41 +0200 Subject: [PATCH 7/7] Adopt CancellationToken + IDisposable on ITestHostHandle; doc nits - 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> --- docs/RFCs/017-TestHost-Launcher.md | 46 ++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 8 deletions(-) diff --git a/docs/RFCs/017-TestHost-Launcher.md b/docs/RFCs/017-TestHost-Launcher.md index 94be217c6f..a77aab427a 100644 --- a/docs/RFCs/017-TestHost-Launcher.md +++ b/docs/RFCs/017-TestHost-Launcher.md @@ -147,7 +147,7 @@ monitoring contract): ```csharp [Experimental("TPEXP", UrlFormat = "https://aka.ms/testingplatform/diagnostics#{0}")] -public interface ITestHostHandle +public interface ITestHostHandle : IDisposable { /// /// Free-form diagnostic identifier (a PID, container id, remote host:pid, …) or null. The @@ -155,15 +155,25 @@ public interface ITestHostHandle /// string? Identifier { get; } + /// Only valid once is true (reading it earlier is undefined). int ExitCode { get; } bool HasExited { get; } - Task WaitForExitAsync(); + + /// Waits for exit, or for the token to be canceled. May be awaited more than once. + Task WaitForExitAsync(CancellationToken cancellationToken); /// Best-effort teardown (e.g. when hang dump aborts the run). void Terminate(); } ``` +`ITestHostHandle` extends `IDisposable`: the platform owns the handle for the whole lifetime of the +test host and disposes it once the host has exited, so implementations release any OS resources they +hold (process objects, sockets, container clients, …) in `Dispose`. `WaitForExitAsync` takes a +`CancellationToken` so the platform can stop waiting on cancellation; the controller host still +reconciles the real OS exit code afterwards (on cancellation it terminates the host and waits for it +to fully exit). + Registration mirrors the existing methods on `ITestHostControllersManager`: ```csharp @@ -212,12 +222,23 @@ public interface ITestHostControllersManager environment variables, so a packaged-app launcher must transfer them another way. - The returned handle must report exit reliably (`WaitForExitAsync`, `ExitCode`, `HasExited`) and support `Terminate()` (hang dump terminates the host through it). `WaitForExitAsync` may be awaited - more than once. + more than once, and must honor its `CancellationToken`. `ExitCode` is only required to be valid + once `HasExited` is `true` (or after `WaitForExitAsync` completes); reading it on a still-running + handle is undefined and implementations are not required to throw. +- The handle is `IDisposable`; the platform disposes it once the host has exited, so the launcher + should release any OS resources it holds (process object, sockets, container client, …) in + `Dispose`. - `Identifier` is an optional free-form diagnostic string (PID, container id, remote `host:pid`, …) and may be `null`. The platform never relies on it for control flow. - If the launcher cannot start the host it should throw; the platform surfaces it as a platform-setup failure. +> The `Quote` and `PasteArguments` helpers used in the examples below are placeholders for whatever +> argument/shell quoting the target mechanism needs (e.g. `PasteArguments` from dotnet/runtime for +> Windows command lines, POSIX single-quoting for a shell). Implement them carefully to avoid +> argument-injection bugs; the reference `Microsoft.Testing.Extensions.PackagedApp` extension (see the +> implementation PR) shows a concrete approach. + ## Examples All examples assume the extension is registered on the builder, e.g. from a `…Extensions` helper: @@ -307,8 +328,11 @@ requires that the host ends up with `context.EnvironmentVariables`. ### 4. Container -Run the test host inside a container and bridge the pipe. The returned handle tracks the -`docker run` client process; `Terminate()` tears down the container. +Run the test host inside a container and bridge the pipe. `docker run --rm` is used so the container +is removed when it stops. The returned handle tracks the `docker run` client process; note that +killing the client alone does not reliably stop the container, so a real implementation should make +`Terminate()` run `docker stop`/`docker rm` (or run with `--init` and rely on `--rm`) rather than +just killing the local client. ```csharp public Task LaunchTestHostAsync( @@ -317,6 +341,9 @@ public Task LaunchTestHostAsync( var args = new List { "run", "--rm", "--init" }; foreach (var kvp in context.EnvironmentVariables.Where(kv => kv.Value is not null)) { args.Add("-e"); args.Add($"{kvp.Key}={kvp.Value}"); } // skip unset (null) vars // Map the controller pipe into the container (Windows named pipe / Unix domain socket mount). + args.Add("--name"); + string containerName = $"mtp-{Guid.NewGuid():N}"; + args.Add(containerName); args.Add("test-image:latest"); args.Add(context.FileName); args.AddRange(context.Arguments); @@ -324,7 +351,9 @@ public Task LaunchTestHostAsync( var psi = new ProcessStartInfo("docker") { UseShellExecute = false }; foreach (string a in args) psi.ArgumentList.Add(a); Process p = Process.Start(psi)!; - return Task.FromResult(new ProcessHandleAdapter(p)); + // Wrap so Terminate() runs `docker stop ` (which tears down the container), not + // just Kill() on the local docker client. + return Task.FromResult(new DockerRunHandle(p, containerName)); } ``` @@ -376,8 +405,9 @@ An earlier draft of this RFC named the hook `ITestHostProcessLauncher` and retur is a local OS process," which is false for container and remote launches and awkward for AUMID-activated apps. The current design renames the types to drop "Process", replaces the `int` process id with an optional free-form `string Identifier` (diagnostic only), drops the redundant -`Exited` event in favour of `WaitForExitAsync`, and names the teardown `Terminate()` instead of -`Kill()`. +`Exited` event in favour of `WaitForExitAsync`, gives `WaitForExitAsync` a `CancellationToken` and +makes the handle `IDisposable` (so the platform can honor cancellation and deterministically release +handle resources), and names the teardown `Terminate()` instead of `Kill()`. ### Do nothing (keep `Process.Start`)