Add MSTest.Windows.AppTesting thin-layer for Windows desktop E2E testing

[Evangelink]

Summary

Introduces a new MSTest.Windows.AppTesting NuGet package that provides base classes for end-to-end testing of Windows desktop applications (WinForms, WPF, Win32). Follows the same thin-layer pattern as Microsoft.Playwright.MSTest — a lifecycle adapter over the built-in System.Windows.Automation API with zero external dependencies.

Base class hierarchy

AutomationTest          ← PlaywrightTest equivalent (hierarchy root, extensible)
  └─ ApplicationTest    ← BrowserTest equivalent (process launch/teardown)
       └─ WindowTest    ← PageTest equivalent (MainWindow as AutomationElement)

• AutomationTest — root of the hierarchy, extension point for future configuration
• ApplicationTest — launches the app via Process.Start, waits for main window handle, manages teardown with CloseMainWindow/Kill. Uses [STATestClass] for COM/STA threading. Configurable via virtual properties (ApplicationPath, ApplicationArguments, ApplicationStartTimeout)
• WindowTest — exposes MainWindow as System.Windows.Automation.AutomationElement. Users can layer FlaUI or other libraries on top for richer element interaction

SDK integration

Mirrors the Playwright/Aspire SDK pattern exactly:

• <EnableWindowsAppTesting>true</EnableWindowsAppTesting> MSBuild property in MSTest.Sdk
• WindowsAppTesting.targets feature file wired into ClassicEngine, VSTest, and NativeAOT targets
• Package version templated through Sdk.props.template
• Works as both an SDK feature flag and a standalone <PackageReference>
• Includes a build-time error when targeting non-Windows TFMs

User experience

<Project Sdk="MSTest.Sdk/4.3.0">
  <PropertyGroup>
    <TargetFramework>net8.0-windows</TargetFramework>
    <EnableWindowsAppTesting>true</EnableWindowsAppTesting>
  </PropertyGroup>
</Project>

[TestClass]
public class MyAppTests : WindowTest
{
    public override string ApplicationPath => @"C:\MyApp\MyApp.exe";

    [TestMethod]
    public void MainWindow_IsVisible()
    {
        Assert.IsNotNull(MainWindow);
    }
}

What's included

• New project: src/TestFramework/TestFramework.Windows.AppTesting/ — the package source
• SDK wiring: Feature targets, props template, NativeAOT guard
• Sample: samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ — Calculator E2E test
• Acceptance tests: WindowsAppTestingSdkTests.cs — tests both MTP runner and VSTest modes using notepad.exe

Design decisions

• System.Windows.Automation over FlaUI: Zero external dependencies. Uses the managed UIA wrapper from the Windows Desktop runtime. Users who want a richer API can layer FlaUI on top.
• [STATestClass]: Required for COM interop with Windows UI Automation
• Separate package + SDK flag: Ships from the same repo, usable both ways
• EnableWindowsTargeting: Always sets Windows TFMs with EnableWindowsTargeting for non-Windows CI compatibility
• Race-safe teardown: ApplicationTearDown uses try/finally with InvalidOperationException handling for process exit races

[Copilot]

Pull request overview

Adds a new Windows-desktop E2E testing thin layer (MSTest.DesktopTesting) and wires it into MSTest.Sdk behind an <EnableDesktopTesting> feature flag, plus samples and acceptance coverage.

Changes:

• Introduces Microsoft.MSTest.DesktopTesting base classes (AutomationTest → ApplicationTest → WindowTest) built on System.Windows.Automation.
• Adds MSTest.Sdk feature wiring (DesktopTesting.targets import + version plumbing + NativeAOT guard).
• Adds a public sample and an acceptance test that exercises the SDK flag in both MSTest runner and VSTest modes.

Show a summary per file

File	Description
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/DesktopTestingSdkTests.cs	Adds acceptance tests that generate a project enabling the DesktopTesting SDK feature.
src/TestFramework/TestFramework.DesktopTesting/AutomationTest.cs	Adds hierarchy root base class for desktop automation tests.
src/TestFramework/TestFramework.DesktopTesting/ApplicationTest.cs	Adds process lifecycle management for desktop app E2E tests.
src/TestFramework/TestFramework.DesktopTesting/WindowTest.cs	Exposes main window as AutomationElement for tests.
src/TestFramework/TestFramework.DesktopTesting/GlobalUsings.cs	Adds MSTest global using for the new package.
src/TestFramework/TestFramework.DesktopTesting/TestFramework.DesktopTesting.csproj	Defines the new packable Windows-desktop testing project/package.
src/TestFramework/TestFramework.DesktopTesting/PublicAPI/PublicAPI.Shipped.txt	Initializes shipped API baseline for the new package.
src/TestFramework/TestFramework.DesktopTesting/PublicAPI/PublicAPI.Unshipped.txt	Declares new public API surface for DesktopTesting.
src/Package/MSTest.Sdk/Sdk/Features/DesktopTesting.targets	Implements the SDK feature target that injects the DesktopTesting package + implicit using.
src/Package/MSTest.Sdk/Sdk/VSTest/VSTest.targets	Imports DesktopTesting feature targets in VSTest mode.
src/Package/MSTest.Sdk/Sdk/Runner/ClassicEngine.targets	Imports DesktopTesting feature targets in ClassicEngine mode.
src/Package/MSTest.Sdk/Sdk/Runner/NativeAOT.targets	Adds a NativeAOT validation error for the feature flag.
src/Package/MSTest.Sdk/Sdk/Sdk.props.template	Adds EnableDesktopTesting default + version property.
src/Package/MSTest.Sdk/MSTest.Sdk.csproj	Plumbs DesktopTesting version into SDK props templating.
samples/public/DemoMSTestSdk/ProjectUsingDesktopTesting/ProjectUsingDesktopTesting.csproj	Adds sample project demonstrating the SDK flag.
samples/public/DemoMSTestSdk/ProjectUsingDesktopTesting/CalculatorTests.cs	Adds sample Calculator tests using WindowTest + UIA.
Directory.Packages.props	Introduces MSTestDesktopTestingVersion and a declared PackageVersion entry.

Copilot's findings

Comments suppressed due to low confidence (1)

samples/public/DemoMSTestSdk/ProjectUsingDesktopTesting/CalculatorTests.cs:45

• The sample locates buttons by AutomationElement.NameProperty with values "One"/"Two". These names are localized in many Windows locales, so the sample can be flaky or fail outside English. Prefer a locale-independent locator (e.g., AutomationId) or note the limitation explicitly in comments.

        AutomationElement? button1 = MainWindow.FindFirst(
            TreeScope.Descendants,
            new PropertyCondition(AutomationElement.NameProperty, "One"));

        AutomationElement? button2 = MainWindow.FindFirst(
            TreeScope.Descendants,
            new PropertyCondition(AutomationElement.NameProperty, "Two"));

• Files reviewed: 17/17 changed files
• Comments generated: 5

[Copilot]

Copilot's findings

• Files reviewed: 17/17 changed files
• Comments generated: 2

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

1. [Coverage] MTP runner test has no test-count assertion (WindowsAppTestingSdkTests.cs:80) — Only AssertExitCodeIs is called. If 0 tests ran, this silently passes. The VSTest counterpart correctly asserts "Passed: 2". AspireSdkTests and PlaywrightSdkTests both use AssertOutputContainsSummary for the MTP case.

2. [Structure] NopAssetFixture + per-test GenerateAssetAsync diverges from repo pattern (WindowsAppTestingSdkTests.cs:11) — Every test independently compiles the test asset from source. The repo's AspireSdkTests / PlaywrightSdkTests use an inner TestAssetFixture : TestAssetFixtureBase that generates the asset once in ClassInitialize, then both tests locate and run against the pre-built asset.

3. [Flakiness] CalculatorTests.cs automation IDs are broken on modern Windows (CalculatorTests.cs:47) — calc.exe on Windows 10/11 is a UWP app whose element tree does not contain num1Button / num2Button. The sample test will always fail as written on any modern Windows installation, which undermines its value as reference documentation.

Recommendations

• Add AssertOutputContainsSummary(0, 2, 0) (or equivalent) after the AssertExitCodeIs in the MTP runner test.
• Refactor WindowsAppTestingSdkTests to use the TestAssetFixture pattern to match Aspire/Playwright tests and avoid redundant per-test compilation.
• Replace the broken Calculator_NumberButtons_AreVisible test in the sample with either a stable Win32 app (e.g., notepad.exe) or add a clear comment that the automation IDs are app-version-specific and must be adapted.

---

Generated by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Expert Code Reviewer
Date: 2026-05-11
Repository: microsoft/testfx

Key Findings

🔴 Correctness — Critical

[TestClass] on WindowTest silently drops STA threading for the entire hierarchy (WindowTest.cs line 42)

STATestClassAttribute is declared with [AttributeUsage(Inherited = false)]. Because of this:

• GetCustomAttributes(inherit: true) on WindowTest does not return [STATestClass] from ApplicationTest
• MSTest resolves ClassAttribute as a plain TestClassAttribute, so ClassAttribute is STATestClassAttribute is false
• GetTestMethodAttribute returns a regular TestMethodAttribute — tests run on MTA, not STA

The same logic applies to every user class that writes [TestClass] class MyTests : WindowTest. All of them will run on MTA, causing Windows UI Automation COM interop to fail with InvalidOperationException or COMException (0x80010106) at runtime.

Fix needed: Change [TestClass] → [STATestClass] on WindowTest (same for ApplicationTest is already correct). Document clearly that user test classes must also use [STATestClass]. Longer term, consider changing STATestClassAttribute to Inherited = true so the constraint propagates automatically.

---

🟡 Defensive — Important

AutomationElement.FromHandle can return null (WindowTest.cs line 58)

If the window handle becomes invalid between ApplicationSetup and WindowSetup, FromHandle returns null, which is silently stored in the non-nullable MainWindow property. Tests then crash with an uninformative NullReferenceException. A null check with a descriptive InvalidOperationException is needed.

---

🟡 Resources — Minor

Process.Kill() without entireProcessTree: true (ApplicationTest.cs line 107)

Desktop applications often spawn child processes. Killing only the top-level process leaves children alive (handles, file locks), which can cause subsequent tests to fail. Use Kill(entireProcessTree: true) — available on both net8.0-windows and net9.0-windows.

---

🟡 Correctness — Minor

Early process exit not detected in setup polling loop (ApplicationTest.cs line 69)

If the app crashes at startup, the loop spins for the full ApplicationStartTimeout and then throws a misleading TimeoutException. Adding an AppProcess.HasExited check inside the loop and surfacing the exit code gives developers actionable diagnostics.

---

Positive Observations

• The thin-layer pattern (mirroring Playwright/Aspire) is a clean and familiar design for MSTest users.
• The teardown try/finally with InvalidOperationException handling correctly handles the race between HasExited checks and process operations.
• SDK integration (ClassicEngine, VSTest, NativeAOT guard, Sdk.props.template) is done consistently with the Playwright/Aspire pattern.
• EnableWindowsTargeting for non-Windows CI builds is a thoughtful touch.
• PublicAPI.Unshipped.txt is properly maintained.

---

Generated by Expert Code Reviewer

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

• Files reviewed: 17/17 changed files
• Comments generated: 3

[Evangelink]

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

All four issues flagged in the previous expert review (run 25696190614) have been fully addressed in the latest commits:

Previous Finding	Status
[TestClass] on WindowTest silently drops STA threading for the entire hierarchy	✅ Fixed — [STATestClass] applied; docs and samples updated
AutomationElement.FromHandle null return not handled	✅ Fixed — null-coalescing throw with descriptive message added
Process.Kill() without entireProcessTree: true	✅ Fixed — child processes now terminated on force-kill
Early process exit not detected in polling loop	✅ Fixed — HasExited check with exit code in error message

Additionally, both Test Expert findings have been addressed: AssertOutputContainsSummary is now called in the MTP runner test, and the TestAssetFixture pattern has been adopted to match AspireSdkTests/PlaywrightSdkTests.

Positive Observations

• Correctness: ApplicationSetup/WindowSetup [TestInitialize] ordering is correct — MSTest calls base-class initializers first, so the process is guaranteed to be running before the AutomationElement is obtained.
• Teardown: try/finally with InvalidOperationException guard correctly handles the race between HasExited and process operations; WaitForExit(5000) after Kill avoids zombie handles.
• SDK integration: WindowsAppTesting.targets correctly validates the Windows TFM at build time and is guarded from NativeAOT; the Sdk.props.template versioning aligns with the Playwright/Aspire pattern.
• Acceptance test: NotepadTests uses a stable Win32 app and validates both a non-null MainWindow and a non-empty title — robust against Notepad UI changes.

No New Issues

No correctness, threading, performance, API compatibility, resource management, or security issues were found in the updated code.

---

Generated by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

The PR has been significantly improved since the previous review (the AssertOutputContainsSummary gap and TestAssetFixture pattern have both been addressed ✅). Three issues remain:

1. [Assertion] Notepad_MainWindow_IsNotNull is a vacuous assertion (WindowsAppTestingSdkTests.cs embedded NotepadTests, line 79) — WindowSetup already guarantees MainWindow is non-null (throws if not). The assertion can never be the actual cause of a test failure; it gives false confidence of coverage.

2. [Flakiness] Calculator_NumberButtons_AreVisible always fails on modern Windows (CalculatorTests.cs lines 44-59) — The UWP Calculator (calc.exe on Windows 10/11) does not expose num1Button/num2Button in its automation tree. The class-level XML doc notes this, but a broken sample test is more confusing than helpful. Using notepad.exe (already stable in the acceptance tests) or adding [Ignore] would prevent users from running into an immediate, unexplained failure.

3. [Coverage] Single build artifact used for both MTP and VSTest acceptance tests (WindowsAppTestingSdkTests.cs line 25) — Both tests share an asset configured with runner: VSTest in global.json. The MTP runner path (TestHost.ExecuteAsync) works because it bypasses dotnet test, but no acceptance test covers a default/MTP-native build, so an MSTest.Sdk regression in that mode would go undetected.

Recommendations

• Replace the vacuous Assert.IsNotNull(MainWindow) with a meaningful assertion (e.g., Assert.AreEqual(ControlType.Window, MainWindow.Current.ControlType)), or remove the redundant test and rely on the title test alone.
• Replace calc.exe in the sample with notepad.exe (or apply [Ignore] to the broken test) so the shipped sample is immediately runnable.
• Add a second test asset variant (or parameterise the existing one) that builds without runner: VSTest to cover the default MTP runner path end-to-end.

---

🧪 Test quality reviewed by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

The PR has addressed all issues from the two previous test reviews ✅:

• AssertOutputContainsSummary(0, 2, 0) is now present in the MTP runner test
• Vacuous Assert.IsNotNull(MainWindow) was replaced with the meaningful ControlType.Window assertion in NotepadTests
• Calculator_NumberButtons_AreVisible now carries [Ignore] with an explanatory message

Two minor issues remain:

1. [Assertion] EnableWindowsAppTesting_WhenUsingVSTest_RunsDesktopTests does not assert "VSTest version" (WindowsAppTestingSdkTests.cs line 42) — Both AspireSdkTests and PlaywrightSdkTests include dotnetTestResult.AssertOutputContains("VSTest version") to confirm VSTest was actually invoked. Without it, the test passes even if the run silently fell back to the MTP runner, making the WhenUsingVSTest claim unverifiable.

2. [Assertion] Assert.IsNotNull(MainWindow) in the sample CalculatorTests.cs is vacuous (line 35) — WindowSetup() already throws before the test body if MainWindow cannot be obtained, so this assertion can never catch a real failure. The same fix applied to the embedded NotepadTests (using Assert.AreEqual(ControlType.Window, ...)) should be applied here for consistency.

Recommendations

• Add dotnetTestResult.AssertOutputContains("VSTest version"); in EnableWindowsAppTesting_WhenUsingVSTest_RunsDesktopTests.
• Replace Assert.IsNotNull(MainWindow) in CalculatorTests.Calculator_MainWindow_HasExpectedTitle with Assert.AreEqual(ControlType.Window, MainWindow.Current.ControlType, ...), consistent with the acceptance test.

---

🧪 Test quality reviewed by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

The four issues from the first expert review (run 25696190614) are all correctly addressed in the current HEAD. Three new minor observations remain:

🟡 Minor — Correctness

AppProcess.Refresh() missing before final timeout check (ApplicationTest.cs:93)
The cached MainWindowHandle value is up to one poll interval (≤50 ms) stale when the while loop exits due to timeout. A final Refresh() before the post-loop check prevents a false TimeoutException at the boundary.

🟡 Minor — Resource management

Win32Exception not caught in ApplicationTearDown (ApplicationTest.cs:121)
CloseMainWindow() and Kill() are both documented to throw Win32Exception (e.g., different desktop session, insufficient permissions). The current catch (InvalidOperationException) misses these, causing the process to run orphaned when teardown is triggered against an elevated or session-isolated process.

🔵 Informational — Cross-TFM dependency

UseWPF=true forces WPF as transitive dependency (TestFramework.Windows.AppTesting.csproj:6)
NuGet propagates framework references, so all consumers (Win32 or WinForms test projects) silently acquire WPF. The package only needs UIAutomation assemblies, which are available without WPF via <FrameworkReference Include="Microsoft.WindowsDesktop.App" />.

Positive Observations

• Process teardown is robust: [TestCleanup] runs even when [TestInitialize] throws, the finally block always disposes the .NET handle, and Kill(entireProcessTree: true) correctly handles child processes.
• [STATestClass] placement is correct: ApplicationTest and WindowTest both carry it; AutomationTest intentionally uses [TestClass] as the extension-point root. STA is inherited by derived user classes.
• Startup polling loop correctly calls Refresh() before each HasExited and MainWindowHandle check; the Thread.Sleep(min(remaining, 50ms)) avoids overshooting the timeout significantly.
• Null safety in WindowSetup — the null-coalescing throw on AutomationElement.FromHandle gives an actionable error message.
• SDK wiring mirrors the Playwright/Aspire patterns precisely: feature targets, props template, NativeAOT guard, and the build-time TFM error are all consistent with the existing infrastructure.

---

Generated by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

• Files reviewed: 19/19 changed files
• Comments generated: 1

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

1. ✅ [Coverage] MTP runner test count assertion — FIXED (WindowsAppTestingSdkTests.cs:24) — The previously missing AssertOutputContainsSummary(failed: 0, passed: 2, skipped: 0) call is now present. Both the MTP and VSTest tests correctly validate test counts, consistent with AspireSdkTests and PlaywrightSdkTests.

2. [Coverage] Only NetCurrent-windows TFM tested (WindowsAppTestingSdkTests.cs:13) — PlaywrightSdkTests (the closest equivalent) uses [DynamicData(nameof(TargetFrameworks.AllForDynamicData))] across all TFMs. The Windows AppTesting tests only exercise net10.0-windows. If the package supports net8.0-windows, regressions on that TFM are invisible to CI. See inline comment for a concrete fix using TargetFrameworks.Net mapped to Windows TFMs.

Observations

• All 25 review threads from the previous cycle are resolved or outdated — no open items carried over.
• Test isolation is clean: ApplicationSetup/ApplicationTearDown bracketing is correct. The Thread.Sleep in the polling loop is bounded to ≤50 ms per iteration and protected by ApplicationStartTimeout — acceptable for a UI automation framework.
• The embedded NotepadTests.cs assertions (ControlType.Window, non-empty title) are adequate for an acceptance test verifying the library plumbing. No false-positive risk.
• [OSCondition(OperatingSystems.Windows)] correctly guards both test methods, so non-Windows CI will report skips rather than failures.

Recommendations

• Extend TFM coverage to all TargetFrameworks.Net entries suffixed with -windows (see inline comment on line 13).

---

Generated by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

The third round of fixes (commit c9ac13f) resolved all previously open correctness issues:

• ✅ AppProcess.Refresh() now called before the post-timeout MainWindowHandle check
• ✅ Win32Exception now caught in ApplicationTearDown alongside InvalidOperationException
• ✅ UseWPF=true replaced with an explicit FrameworkReference

🟡 One remaining concern

FrameworkReference Include="Microsoft.WindowsDesktop.App" (csproj line 8) — the switch from UseWPF=true replaced one WPF-only reference with a broader reference that includes both WPF and WinForms. Since System.Windows.Automation resides in the WPF subsystem, Microsoft.WindowsDesktop.App.WPF is the correct minimal reference. See inline comment for details.

✅ What was verified

• Lifecycle ordering: ApplicationSetup → WindowSetup (base-first init), single ApplicationTearDown cleanup — correct MSTest ordering
• Process teardown: CloseMainWindow → WaitForExit(5s) → Kill(entireProcessTree) → Dispose in try/finally — no leaks in any error path
• Win32Exception handling: correctly catches post-HasExited races in teardown
• Post-loop Refresh() guard: main window handle is freshly validated before WindowSetup consumes it
• SDK integration: NativeAOT guard, feature-flag pattern, nuspec wiring, version templating — all correct
• Public API surface: PublicAPI.Unshipped.txt is complete and accurate

Recommendations

Change <FrameworkReference Include="Microsoft.WindowsDesktop.App" /> to <FrameworkReference Include="Microsoft.WindowsDesktop.App.WPF" /> to avoid pulling WinForms assemblies into consumers that don't need them.

---

Generated by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot's findings

• Files reviewed: 19/19 changed files
• Comments generated: 2

[Evangelink]

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

All open findings from previous expert reviews are resolved in the current HEAD (76e6f0b):

Previous finding	Status
AppProcess.Refresh() missing before final timeout check	✅ Fixed — Refresh() now called unconditionally before the TimeoutException check
Win32Exception not caught in ApplicationTearDown	✅ Fixed — catch (Exception ex) when (ex is InvalidOperationException or Win32Exception)
FrameworkReference overly broad (Microsoft.WindowsDesktop.App)	✅ Fixed — now scoped to Microsoft.WindowsDesktop.App.WPF, which is the minimum required for System.Windows.Automation

No new correctness, threading, performance, API-compat, cross-TFM, resource, or security issues were found in this pass.

Positive Observations

• Startup polling loop is correct: each iteration calls Refresh() and HasExited before sleeping, avoiding a busy-spin; the trailing Refresh() + handle check after the loop exits is clean.
• Teardown is robust: try/finally guarantees Dispose() even if CloseMainWindow()/Kill() throw; Kill(entireProcessTree: true) handles child processes; WaitForExit(5000) post-kill avoids zombie handles.
• [STATestClass] placement is correct: ApplicationTest and WindowTest carry it, and since Inherited = true (the default for AttributeUsageAttribute), all user-derived classes automatically run on the STA thread without needing to redeclare the attribute — while AutomationTest deliberately stays [TestClass] as the extension-point root.
• WindowSetup ordering: [TestInitialize] methods run base-to-derived, so ApplicationSetup (which sets AppProcess) always precedes WindowSetup (which reads AppProcess.MainWindowHandle). If ApplicationSetup throws, WindowSetup is skipped by the MSTest runner, so no NRE path exists.
• PublicAPI.Unshipped.txt correctly declares the full public surface for the new package.
• SDK integration mirrors the Playwright/Aspire pattern exactly: feature targets, NativeAOT.targets guard, Sdk.props.template version injection, Central Package Management support.

Overall Assessment

The PR is ready to merge. The implementation is clean, safe, and consistent with the existing thin-layer pattern used by Playwright and Aspire.

---

Generated by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

All issues from previous test reviews have been addressed ✅, and the latest commit (76e6f0b) adds clean multi-TFM coverage:

• AssertOutputContainsSummary(0, 2, 0) is present in the MTP runner test ✓
• AssertOutputContains("VSTest version") confirms VSTest invocation in the VSTest test ✓
• AssertOutputContains("Passed! - Failed: 0, Passed: 2, ...") verifies test counts in VSTest mode ✓
• [DynamicData(nameof(DesktopTargetFrameworksForDynamicData))] now runs both tests across all supported Windows TFMs (consistent with PlaywrightSdkTests approach for platform-specific TFMs) ✓
• PatchTargetFrameworks(DesktopTargetFrameworks) correctly builds the asset for all Windows TFMs in a single pass ✓
• TestAssetFixture pattern matches the repo's Aspire/Playwright convention — asset built once, reused by all test cases ✓
• Assertions in the embedded NotepadTests.cs and CalculatorTests.cs sample are meaningful (ControlType.Window check, not vacuous IsNotNull) ✓

Recommendations

No further changes needed. Tests are well-isolated, structurally consistent with existing acceptance tests, and cover the expected behavior across all Windows TFMs.

---

Generated by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Test Expert Reviewer 🧪
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

Both issues flagged in the previous test review (2026-05-12T09:55) have been resolved ✅:

1. [Assertion] VSTest version assertion — dotnetTestResult.AssertOutputContains("VSTest version") is now present in EnableWindowsAppTesting_WhenUsingVSTest_RunsDesktopTests (line 44), confirming VSTest was actually invoked.

2. [Assertion] Vacuous Assert.IsNotNull(MainWindow) in sample — CalculatorTests.Calculator_MainWindow_HasExpectedTitle now uses Assert.AreEqual(ControlType.Window, MainWindow.Current.ControlType, ...), matching the meaningful assertion style established in the embedded NotepadTests.

The test suite is well-structured:

• Both MTP runner and VSTest invocation paths are covered
• Each test asserts exit code and explicit pass/fail/skip counts
• OSCondition correctly guards Windows-only tests for non-Windows CI
• [STATestClass] threading requirement is respected throughout
• [Ignore] with an explanatory message covers the AutomationId-specific sample test

Recommendations

None — all previously flagged issues have been addressed.

---

🧪 Test quality reviewed by Test Expert Reviewer 🧪

🧪 Test quality reviewed by Test Expert Reviewer 🧪

[Evangelink]

Summary

Workflow: Expert Code Reviewer 🧠
Date: 2026-05-12
Repository: microsoft/testfx

Key Findings

All three observations from the previous expert review (run 25732804835) are addressed in commit 76e6f0b:

Previous Finding	Status
AppProcess.Refresh() missing before final timeout check (ApplicationTest.cs:93)	✅ Refresh() call is present at line 94
Win32Exception not caught in ApplicationTearDown	✅ catch (Exception ex) when (ex is InvalidOperationException or Win32Exception) on line 123
UseWPF=true forces WPF as transitive dependency	✅ Narrowed to <FrameworkReference Include="Microsoft.WindowsDesktop.App.WPF" /> — correct minimal reference since UIAutomationClient.dll lives in the WPF framework subset

Fresh Analysis (No New Issues Found)

After reviewing all changed files in the current HEAD (66d35e0):

• Correctness: Lifecycle ordering is correct — MSTest calls base-class [TestInitialize] first, so AppProcess is always set before WindowSetup accesses it; [TestCleanup] on ApplicationTest runs even when [TestInitialize] throws, so process handles are always cleaned up.
• Threading: [STATestClass] is correctly applied to both ApplicationTest and WindowTest; the sample and acceptance test both redeclare [STATestClass] on the user test class (required because STATestClassAttribute has Inherited = false).
• Resources: try/finally in ApplicationTearDown guarantees disposal; Kill(entireProcessTree: true) handles child processes; WaitForExit(5000) after kill prevents zombie handles.
• API surface: PublicAPI.Unshipped.txt is complete and correct; all public types have XML docs; no init accessors in new public API.
• Cross-TFM: _ValidateWindowsAppTestingPlatform target errors at Restore/Build time for non-Windows TFMs; EnableWindowsTargeting guard in the .csproj handles non-Windows CI builds.
• Acceptance tests: Both MTP runner and VSTest paths now assert AssertOutputContainsSummary(failed: 0, passed: 2, skipped: 0) / "Passed: 2", consistent with AspireSdkTests and PlaywrightSdkTests; [DynamicData] loops over all desktop TFMs.

Overall Assessment

The implementation is clean, correct, and complete. The thin-layer pattern faithfully mirrors Playwright/Aspire, the teardown logic is race-safe, and all previously identified issues have been resolved. No correctness, threading, performance, API compatibility, resource management, or security concerns remain.

---

Generated by Expert Code Reviewer 🧠

🧠 Reviewed by Expert Code Reviewer 🧠

[Copilot]

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.