Add MSTest.Windows.AppTesting for desktop UI tests - #10862
Add MSTest.Windows.AppTesting for desktop UI tests#10862Amaury Levé (Evangelink) wants to merge 14 commits into
Conversation
Revive the MSTest.Windows.AppTesting work from PR #7810 on current main, including SDK integration, package metadata, a deterministic sample, and MTP/VSTest acceptance coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
🧪 Expert test review — PR #10862
Note: Mutation analysis reported N/A for all four tests — the acceptance tests spawn out-of-process test hosts against a Windows-only physical UI (Character Map), and the sample tests are documentation demos, so the production entry points (native app automation) could not be resolved/exercised for pseudo-mutation in this review. All four tests are gated correctly ( This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Suggestions on the Files changed tab can be applied with one click. Re-run with
|
This comment has been minimized.
This comment has been minimized.
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Note
🤖 Automated review by GitHub Copilot. Generated by the Expert Code Review workflow. To request a follow-up action, reply by tagging @copilot directly.
Review Summary — MSTest.Windows.AppTesting
Clean PR that adds a new MSTest.Windows.AppTesting package with application lifecycle and Windows UI Automation base classes, wired through MSTest.Sdk. Two inline comments posted; neither is blocking.
| # | Dimension | Verdict |
|---|---|---|
| 1 | Algorithmic Correctness | ✅ OK — startup loop handles edge cases (process exit, timeout) correctly |
| 2 | Threading & Concurrency | AutomationTest uses [TestClass] (MTA) while subclasses use [STATestClass]; see inline comment |
| 3 | Security & IPC | ✅ N/A |
| 4 | Public API & Binary Compat | ✅ OK — all public API in PublicAPI.Unshipped.txt, no init accessors, types unsealed by design (base-class hierarchy), XML docs present |
| 5 | Performance & Allocations | ✅ OK — not a hot path |
| 6 | Cross-TFM Compatibility | ✅ OK — targets net8.0-windows / net9.0-windows only, validated in .targets |
| 7 | Resource & IDisposable | ✅ OK — Process disposed in finally; CloseMainWindow → Kill cascade is correct |
| 8 | Defensive Coding | ✅ OK — boundary checks for process exit, null handle, and timeout present |
| 9 | Naming & Code Style | ✅ OK |
| 10 | Logging & Observability | ✅ N/A |
| 11 | Error Messages & UX | ✅ OK — clear exception messages with variable names |
| 12 | Tests & Test Quality | ✅ OK — acceptance tests cover MTP and VSTest, OS-gated |
| 13 | Documentation | ✅ OK — PACKAGE.md present, XML docs thorough |
| 14 | Localization | ✅ N/A — no user-facing resx strings |
| 15 | Backward Compatibility | ✅ N/A — new package |
| 16 | Build & Pack Integration | ✅ OK — verify-nupkgs.ps1 updated, MSBuild integration mirrors Aspire/Playwright patterns |
| 17 | Serialization | ✅ N/A |
| 18 | Tracing & Telemetry | ✅ N/A |
| 19 | Nullability | ✅ OK — null! used for lifecycle-initialized properties, consistent with rest of codebase |
| 20 | TODOs & Leftovers | ✅ None found |
| 21 | Scope Discipline | ✅ Single concern |
| 22 | Script & Tooling Hygiene | ✅ OK — PowerShell hash table update is correct |
Overall: Looks good. The two inline comments are non-blocking suggestions for future hardening.
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 2
New issues introduced by this change (4)
| Severity | Finding |
|---|---|
 |
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj — This project is not listed in samples/public/DemoMSTestSdk/DemoMSTestSdk.sln. Because… |
|
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj — The non-SDK example cannot restore as written: MSTestWindowsAppTestingVersion is only defined by… |
 |
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — No test exercises this validation path: every generated acceptance TFM appends -windows. Add a… |
|
src/TestFramework/TestFramework.Windows.AppTesting/ApplicationTest.cs — The acceptance tests exercise only a responsive Character Map process. Add coverage for an app that… |
What changed in this PR
Adds Windows desktop UI automation support to MSTest through a new package, SDK integration, sample, and acceptance coverage.
Changes:
- Adds application lifecycle and UI Automation base classes.
- Integrates the package into MSTest.Sdk for MTP and VSTest.
- Adds Character Map samples, acceptance tests, and package verification.
| File | Description |
|---|---|
TestFx.slnx |
Includes the new package project. |
WindowsAppTestingSdkTests.cs |
Tests MTP and VSTest integration. |
WindowTest.cs |
Exposes the main UI Automation window. |
TestFramework.Windows.AppTesting.csproj |
Defines the Windows package. |
PublicAPI.Unshipped.txt |
Tracks new public APIs. |
PublicAPI.Shipped.txt |
Initializes the shipped baseline. |
PACKAGE.md |
Documents package usage. |
GlobalUsings.cs |
Imports MSTest APIs globally. |
AutomationTest.cs |
Adds the hierarchy root. |
ApplicationTest.cs |
Manages application lifecycle. |
VSTest.targets |
Enables the feature under VSTest. |
Sdk.props.template |
Defines feature and version properties. |
NativeAOT.targets |
Rejects unsupported NativeAOT usage. |
ClassicEngine.targets |
Enables the feature under MTP. |
WindowsAppTesting.targets |
Adds validation, package reference, and using. |
MSTest.Sdk.csproj |
Packs and versions the feature target. |
ProjectUsingWindowsAppTesting.csproj |
Adds the SDK sample configuration. |
CharacterMapTests.cs |
Demonstrates desktop UI tests. |
verify-nupkgs.ps1 |
Verifies package contents. |
💡 Configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Run the base hierarchy on STA threads, honor cooperative cancellation during app startup, include the package in MSTest.slnf, and make package verification detect missing outputs. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
This comment has been minimized.
This comment has been minimized.
🧪 Expert test review — PR #10862Reviewed 4 new test methods introduced by this PR across the new
This advisory comment was generated automatically. Grades are heuristic
|
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 3
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
|
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/WindowsAppTestingSdkTests.cs — This VSTest leg omits the roll-forward setting used by the MTP leg above. The repo-local toolchain… |
|
src/Package/MSTest.Sdk/Sdk/Sdk.props.template — These new user-facing SDK properties are missing from the SDK documentation tables.… |
Pre-existing issues (4)
| Severity | Finding |
|---|---|
|
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj — The non-SDK example cannot restore as written: MSTestWindowsAppTestingVersion is only defined by… View comment |
|
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj — This project is not listed in samples/public/DemoMSTestSdk/DemoMSTestSdk.sln. Because… View comment |
|
src/TestFramework/TestFramework.Windows.AppTesting/ApplicationTest.cs — The acceptance tests exercise only a responsive Character Map process. Add coverage for an app that… View comment |
|
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — No test exercises this validation path: every generated acceptance TFM appends -windows. Add a… View comment |
Suppressed comments (3)
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj:1
- This public sample is not included in
samples/public/DemoMSTestSdk/DemoMSTestSdk.sln.eng/build-samples.ps1:60-64builds discovered solutions rather than standalone projects, so the new sample is currently omitted from sample CI and can silently break. Add the project and its Debug/Release configurations to that solution.
<Project Sdk="MSTest.Sdk">
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets:11
- The added acceptance asset always targets
*-windows, and no other test exercises this validation message, so removing or breaking this target would leave all current tests green. Add a non-Windows-target build case that enables the feature and asserts this diagnostic.
<Target Name="_ValidateWindowsAppTestingPlatform" BeforeTargets="Restore;Build"
Condition=" '$(TargetFramework)' != '' and '$(_IsWindowsTfm)' != 'true' ">
<Error Text="MSTest.Windows.AppTesting requires a Windows target framework (e.g. net8.0-windows). Current TargetFramework: '$(TargetFramework)'." />
src/TestFramework/TestFramework.Windows.AppTesting/ApplicationTest.cs:124
- The Character Map acceptance path closes normally and does not assert that cleanup terminated the process, so this forced-kill branch is untested; a regression here can leave desktop processes running after the suite. Add a test application that ignores
CloseMainWindowand verify that cleanup terminates its process tree.
_ = appProcess.CloseMainWindow();
if (!appProcess.WaitForExit(5000))
{
appProcess.Kill(entireProcessTree: true);
_ = appProcess.WaitForExit(5000);
Cover invalid target frameworks, early process exit, startup timeout, and forced cleanup while correcting the standalone sample package version. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
This comment has been minimized.
This comment has been minimized.
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
 |
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — This check is case-sensitive, although the SDK accepts target framework monikers… |
Pre-existing issues (2)
| Severity | Finding |
|---|---|
|
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/WindowsAppTestingSdkTests.cs — This VSTest leg omits the roll-forward setting used by the MTP leg above. The repo-local toolchain… View comment |
|
src/Package/MSTest.Sdk/Sdk/Sdk.props.template — These new user-facing SDK properties are missing from the SDK documentation tables.… View comment |
Issues resolved since last review (4)
| Severity | Finding |
|---|---|
|
src/TestFramework/TestFramework.Windows.AppTesting/ApplicationTest.cs — The acceptance tests exercise only a responsive Character Map process. Add coverage for an app that… View resolved comment |
|
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — No test exercises this validation path: every generated acceptance TFM appends -windows. Add a… View resolved comment |
|
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj — The non-SDK example cannot restore as written: MSTestWindowsAppTestingVersion is only defined by… View resolved comment |
|
samples/public/DemoMSTestSdk/ProjectUsingWindowsAppTesting/ProjectUsingWindowsAppTesting.csproj — This project is not listed in samples/public/DemoMSTestSdk/DemoMSTestSdk.sln. Because… View resolved comment |
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
src/TestFramework/TestFramework.Windows.AppTesting/PACKAGE.md:12
- This standalone getting-started snippet does not pin
MSTest.Sdk, and the README does not provide aglobal.jsonpin, so users copying it from the NuGet package page cannot resolve the project SDK. Include an SDK version here, consistent withsrc/Package/MSTest.Sdk/PACKAGE.md:9-16.
<Project Sdk="MSTest.Sdk">
src/Package/MSTest.Sdk/Sdk/Sdk.props.template:45
- The repository's complete MSTest.Sdk property reference in
docs/glossary.md:312-343still lists only Aspire and Playwright and omits both new controls. Add anEnableWindowsAppTestingcompatibility row (including its NativeAOT error and VSTest support) and documentMSTestWindowsAppTestingVersion, otherwise the advertised complete reference is stale as soon as this feature ships.
<EnableWindowsAppTesting Condition=" '$(EnableWindowsAppTesting)' == '' ">false</EnableWindowsAppTesting>
<EnablePlaywright Condition=" '$(EnablePlaywright)' == '' ">false</EnablePlaywright>
<AspireHostingTestingVersion Condition=" '$(AspireHostingTestingVersion)' == '' ">${AspireHostingTestingVersion}</AspireHostingTestingVersion>
<MicrosoftNETTestSdkVersion Condition=" '$(MicrosoftNETTestSdkVersion)' == '' ">${MicrosoftNETTestSdkVersion}</MicrosoftNETTestSdkVersion>
<MSTestWindowsAppTestingVersion Condition=" '$(MSTestWindowsAppTestingVersion)' == '' ">${MSTestWindowsAppTestingVersion}</MSTestWindowsAppTestingVersion>
Apply desktop runtime roll-forward consistently to the VSTest acceptance leg and document the feature and version override properties. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
🧪 Expert test review — PR #10862The pre-step's extract data (test-file/line-range TSV) was unavailable in this run, so this review was derived directly from the PR's file diffs. Two new test files were added: a sample demo (
Note on mutation column: production code for the new No inline suggestions were posted — findings above are minor reliability/coverage notes rather than concrete, safely-anchorable mechanical fixes. This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Re-run with
|
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
Pre-existing issues (1)
| Severity | Finding |
|---|---|
|
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — This check is case-sensitive, although the SDK accepts target framework monikers… View comment |
Issues resolved since last review (2)
| Severity | Finding |
|---|---|
|
src/Package/MSTest.Sdk/Sdk/Sdk.props.template — These new user-facing SDK properties are missing from the SDK documentation tables.… View resolved comment |
|
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/WindowsAppTestingSdkTests.cs — This VSTest leg omits the roll-forward setting used by the MTP leg above. The repo-local toolchain… View resolved comment |
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
test/IntegrationTests/MSTest.Acceptance.IntegrationTests/WindowsAppTestingSdkTests.cs:150
- The generated asset imports
Microsoft.MSTest.Windows.AppTestingexplicitly, so these acceptance tests do not exercise the implicitUsingadded byWindowsAppTesting.targets. Since the public sample relies on that implicit import and is intentionally excluded from sample CI, removing this using is needed to make a regression in the SDK feature fail automated coverage.
using Microsoft.MSTest.Windows.AppTesting;
This comment has been minimized.
This comment has been minimized.
Use MSBuild's target-platform parser, cover uppercase target framework monikers, and pin the package README SDK example. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
🧪 Expert test review — PR #10862
This advisory comment was generated automatically. Grades are heuristic
|
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
Include the test SDK and default CodeCoverage and TrxReport extensions in the non-MSTest.Sdk equivalent configuration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
|
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — On a non-Windows build agent, enabling this feature still leaves EnableWindowsTargeting unset.… |
This comment has been minimized.
This comment has been minimized.
Default EnableWindowsTargeting on non-Windows hosts without overriding consumers and cover the cross-targeted SDK build path. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
|
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — The consumer-side targets still do not enable Windows targeting on non-Windows hosts. The similarly… |
Issues resolved since last review (1)
| Severity | Finding |
|---|---|
|
src/Package/MSTest.Sdk/Sdk/Features/WindowsAppTesting.targets — On a non-Windows build agent, enabling this feature still leaves EnableWindowsTargeting unset.… View resolved comment |
🧪 Expert test review — PR #10862
Overall this is a strong acceptance-test suite for the new Windows App Testing feature: it exercises MTP and VSTest runners, both the invalid- and valid-TFM guard paths, and deterministic early-exit/timeout scenarios with real process-cleanup verification, all under This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Suggestions on the Files changed tab can be applied with one click. Re-run with
|
There was a problem hiding this comment.
🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 135 AIC · ⌖ 1.03 AIC · ⊞ 16.9K · ◷
There was a problem hiding this comment.
Copilot review overview
Review tier: Balanced
Findings: None
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
src/Package/MSTest.Sdk/Sdk/VSTest/VSTest.targets:56
- The new VSTest SDK import is not exercised by
EnableWindowsAppTesting_WhenUsingVSTest_RunsDesktopTests: that test runs the already-built ClassicEngine executable through the VSTest front end, whileUseVSTestremains at its defaultfalse. Consequently this import could be removed and the test would still pass. Add a Windows App Testing asset that sets<UseVSTest>true</UseVSTest>and build/run that project, as the existing VSTest SDK coverage does inSdkTests.cs:66-75.
<Import Project="$(MSBuildThisFileDirectory)../Features/WindowsAppTesting.targets" Condition=" '$(EnableWindowsAppTesting)' == 'true' " />
Build and run a dedicated UseVSTest project and assert stable VSTest output instead of relying on a version banner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
🧪 Expert test review — PR #10862Reviewed the single new test file in this PR:
This advisory comment was generated automatically. Grades are heuristic
|
There was a problem hiding this comment.
🤖 Automated content by GitHub Copilot. Generated by the Test Reviewer on PR (on open / sync) workflow. · auto · 89.9 AIC · ⌖ 1.44 AIC · ⊞ 16.9K · ◷
This comment has been minimized.
This comment has been minimized.
Evaluate EnableWindowsTargeting directly for simulated non-Windows hosts and explicit consumer opt-outs while retaining real non-Windows build coverage. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
🧪 Expert test review — PR #10862This PR has already been through multiple review iterations (14 review threads, all addressed) covering STA threading, cancellation, SDK guard casing, cross-targeting defaults, and negative-path coverage. Re-reviewing the current HEAD, the test suite is now in solid shape. No new high-confidence actionable findings beyond what is already resolved in prior threads.
This advisory comment was generated automatically. Grades are heuristic and informational — they do not block merging. Re-run with
|
Include MSTest.Windows.AppTesting in NonWindowsTests.slnf so Linux and macOS acceptance tests can restore the locally built package. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 006cf5a8-6525-4ecb-829f-e83a99c4de05
🧵 Parallel-safety audit — PR #10862Parallelization — assembly audited:
Only one test-relevant file changed in this PR: Findings: A (global-state) Top actions (by expected value):
Warning
No category A/B/C findings: environment variables passed via Advisory only — heuristic, non-blocking. Re-run with
|
Summary
Revives the unmerged work from #7810 on current
main.MSTest.Windows.AppTestingpackage with application lifecycle and Windows UI Automation base classesMSTest.Sdkwith<EnableWindowsAppTesting>true</EnableWindowsAppTesting>for MTP and VSTestMSTest.TestFrameworkdependency conventionsValidation
build.cmd -packWindowsAppTestingSdkTests: 4 passed across MTP and VSTest (net8.0-windowsandnet10.0-windows)Supersedes #7810.