Fix WinUI MTP samples and per-sample test runner selection - #11103
Fix WinUI MTP samples and per-sample test runner selection#11103Amaury Levé (Evangelink) wants to merge 5 commits into
Conversation
- Default WinUI sample projects (packaged/unpackaged MTP, VSTest) to x64 so directory-based dotnet test invocations no longer build/launch an ARM64 executable on x64 Windows. - Remove ARM64 from the WinUI MTP .slnx default configurations. - Mark WinUI MTP sample projects as test projects for consistent discovery. - Add AnyCPU fallback architecture/RID/publish-profile settings for direct WinUI project builds. - Configure samples/public/global.json to select the Microsoft.Testing.Platform runner for directory-based dotnet test invocations, with local global.json opt-outs/opt-ins for the VSTest-only and MTP-only samples so each keeps using its intended runner. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
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
Clean samples-only PR that fixes real usability issues (ARM64 binary launch on x64 hosts, missing IsTestProject, missing test runner configuration). No production code, public API, or test changes.
Verdict Table
| # | Dimension | Severity | Result |
|---|---|---|---|
| 1 | Algorithmic Correctness | MAJOR | N/A — no logic code |
| 2 | Threading & Concurrency | BLOCKING | N/A |
| 3 | Security & IPC | BLOCKING | N/A |
| 4 | Public API & Binary Compat | BLOCKING | N/A — samples only |
| 5 | Performance & Allocations | MAJOR | N/A |
| 6 | Cross-TFM Compatibility | MAJOR | N/A |
| 7 | Resource Management | MAJOR | N/A |
| 8 | Error Handling | MAJOR | N/A |
| 9 | Localization | MINOR | N/A |
| 10 | Testing Adequacy | MAJOR | ✅ Manual testing documented in PR body |
| 11 | Code Style & Conventions | MINOR | ✅ Clean |
| 12 | Documentation | MINOR | ✅ PR body is thorough |
| 13 | MSBuild & Build Infra | MAJOR | |
| 14 | Naming | MINOR | N/A |
| 15 | Logging & Telemetry | MINOR | N/A |
| 16 | Configuration | MINOR | ✅ global.json hierarchy is correct |
| 17 | Cancellation | MAJOR | N/A |
| 18 | Backward Compat | BLOCKING | ✅ Samples only, no shipping code |
| 19 | Dependency Management | MINOR | N/A |
| 20 | Scope Discipline | MINOR | ✅ Single concern |
| 21 | Experimental API | MINOR | N/A |
| 22 | PowerShell Hygiene | MINOR | N/A |
Findings
One minor MSBuild concern (inline comment): The AnyCPU → x64 fallback in the WinUI .csproj files only handles PROCESSOR_ARCHITECTURE == 'AMD64'. An ARM64 Windows user doing a default dotnet test would still hit the original failure since no ARM64 fallback is provided. This is low severity for sample projects — a comment or matching ARM64 condition would suffice.
Overall this is a well-scoped improvement to sample usability. No blocking issues found.
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The root runner selection unintentionally converts the documented VSTest performance comparison to MTP.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 1
New issues introduced by this change (1)
| Severity | Finding |
|---|---|
samples/public/global.json — This root setting changes the documented VSTest leg in `samples/public/mstest-runner/runner_vs_vstes… |
What changed in this PR
Updates public WinUI samples for x64 defaults and configures directory-scoped test runners.
Changes:
- Adds x64/AnyCPU fallbacks to WinUI projects.
- Removes ARM64 from MTP solution defaults.
- Selects MTP or VSTest through scoped
global.jsonfiles.
| File | Description |
|---|---|
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj |
Adds x64 build fallbacks. |
samples/public/WinUIVSTestApp/global.json |
Selects VSTest. |
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.slnx |
Removes ARM64 solution configuration. |
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj |
Adds x64 fallbacks and test discovery. |
samples/public/WinUIMtpPackagedApp/WinUIMtpPackagedApp.slnx |
Removes ARM64 solution configuration. |
samples/public/WinUIMtpPackagedApp/WinUIMtpPackagedApp.csproj |
Adds x64 fallbacks and test discovery. |
samples/public/UwpVSTestApp/global.json |
Selects VSTest. |
samples/public/global.json |
Selects MTP for public samples. |
samples/public/DemoMSTestSdk/ProjectWithNativeAOT/global.json |
Selects MTP and pins MSTest SDK. |
samples/public/DemoMSTestSdk/ProjectUsingVSTest/global.json |
Selects VSTest and pins MSTest SDK. |
samples/public/DemoMSTestSdk/ProjectUsingPlaywright/global.json |
Selects MTP and pins MSTest SDK. |
samples/public/DemoMSTestSdk/ProjectUsingMSTestRunner/global.json |
Selects MTP and pins MSTest SDK. |
samples/public/DemoMSTestSdk/ProjectUsingAspire/global.json |
Selects MTP and pins MSTest SDK. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Co-authored-by: Evangelink <11340282+Evangelink@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The default runner breaks a documented VSTest comparison, and two WinUI projects have incomplete ARM64 AnyCPU fallbacks.
Review tier: Balanced
Findings: 1
Pre-existing issues (1)
| Severity | Finding |
|---|---|
samples/public/global.json — This root setting changes the documented VSTest leg in `samples/public/mstest-runner/runner_vs_vstes… View comment |
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj:11
- The AnyCPU fallback handles only AMD64, so a direct build on native ARM64 leaves this WinUI executable as
AnyCPUwith no RID despite ARM64 being an advertised project platform. Add the correspondingARM64/win-arm64assignments, as the packaged MTP project already does.
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj:14 - The AnyCPU fallback is incomplete on native ARM64. In that case none of these AMD64-only assignments runs, leaving
PlatformTarget/RuntimeIdentifierunresolved andPublishProfileset to the nonexistentwin-AnyCPU.pubxml, even though this project advertises ARM64 and containswin-arm64.pubxml. Mirror the ARM64 mappings used by the packaged MTP sample.
samples/public/global.json:4
- This repository-wide runner default changes the documented VSTest leg of
samples/public/mstest-runner/runner_vs_vstest: that directory has no nearerglobal.json, and its README invokesdotnet testspecifically to measure VSTest. On .NET 10+, this setting makes that command use Microsoft.Testing.Platform instead, so the sample no longer performs the comparison it advertises. Add a VSTest-scopedglobal.jsonfor that sample (or narrow this default's scope).
"test": {
"runner": "Microsoft.Testing.Platform"
},
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Two WinUI samples omit ARM64 AnyCPU fallbacks, leaving native ARM64 direct builds incompletely configured.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 2
New issues introduced by this change (2)
| Severity | Finding |
|---|---|
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj — On a native ARM64 host, a direct invocation that evaluates Platform as AnyCPU matches none of… |
|
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj — The AnyCPU fallback only covers AMD64 here. With a native ARM64 SDK, Platform=AnyCPU leaves… |
Pre-existing issues (1)
| Severity | Finding |
|---|---|
samples/public/global.json — This root setting changes the documented VSTest leg in `samples/public/mstest-runner/runner_vs_vstes… View comment |
Pipeline Test Triage — Final AnalysisBuild: [1586775]((dev.azure.com/redacted) (result: failed, completed) · Check: microsoft.testfx Conclusion: No test-level defect — root cause is a build/compilation failure (out of scope for this agent)The only two "failed" test records in the 31,742-test result set are:
Both fail with the same message: This is a downstream consequence, not an independent test defect: the sourced from Confidence: High. The RS0017 analyzer errors are deterministic public-API-tracking mismatches (uwp History: In the 12 prior completed builds sampled (last 30 days, Scope note: Per this agent's charter, ordinary compilation/PublicAPI-analyzer build failures are owned by Build Failure Analysis, not Pipeline Test Triage. No issue is being created here; the actionable next step is to reconcile No issue created — this is a build-configuration/API-tracking problem specific to this PR's change, not a recurring or flaky test defect.
|
Build failure analysisResult: Genuine build failure — not a non-build/test failure. Root causeThe Windows Debug and Release build legs fail with a Roslyn public-API analyzer error ( This is exactly the risk the PR description itself calls out — the legacy Why it happens:
Confirmed via binlog: the failing Suggested fixSplit the shared Scope checkAll 18 build legs were inspected (
|
There was a problem hiding this comment.
🤖 Automated content by GitHub Copilot. Generated by the Build Failure Analysis workflow. · auto · 176 AIC · ⌖ 2.03 AIC · ⊞ 13.4K · ◷
e4bd531 to
4201d57
Compare
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The public-samples runner default unintentionally routes the documented VSTest benchmark through MTP.
Review tier: Balanced
Findings: 2
Pre-existing issues (3)
| Severity | Finding |
|---|---|
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj — The AnyCPU fallback only covers AMD64 here. With a native ARM64 SDK, Platform=AnyCPU leaves… View comment |
|
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj — On a native ARM64 host, a direct invocation that evaluates Platform as AnyCPU matches none of… View comment |
|
samples/public/global.json — This root setting changes the documented VSTest leg in `samples/public/mstest-runner/runner_vs_vstes… View comment |
Suppressed comments (1)
samples/public/global.json:3
- This directory-wide setting also changes the documented VSTest benchmark under
samples/public/mstest-runner/runner_vs_vstest: its README runsdotnet test <FOLDER_TO_TEST>specifically as the VSTest side of the comparison, but .NET 10 resolves this parentglobal.jsonand will now route that command through Microsoft.Testing.Platform. Add a nearer VSTest runner configuration for that benchmark (or otherwise scope this default away from it) so the sample still measures the two different runners.
"runner": "Microsoft.Testing.Platform"
…comparison Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The three WinUI AnyCPU fallback paths omit the supported native x86 architecture.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review tier: Balanced
Findings: 3
New issues introduced by this change (3)
| Severity | Finding |
|---|---|
samples/public/WinUIMtpPackagedApp/WinUIMtpPackagedApp.csproj — The AnyCPU fallback omits native x86. When PROCESSOR_ARCHITECTURE=x86, the target/RID remain… |
|
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj — The AnyCPU fallback still has no native x86 branch. On a 32-bit Windows host,… |
|
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj — The AnyCPU fallback omits native x86. When PROCESSOR_ARCHITECTURE=x86, the target/RID remain… |
Issues resolved since last review (3)
| Severity | Finding |
|---|---|
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj — The AnyCPU fallback only covers AMD64 here. With a native ARM64 SDK, Platform=AnyCPU leaves… View resolved comment |
|
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj — On a native ARM64 host, a direct invocation that evaluates Platform as AnyCPU matches none of… View resolved comment |
|
samples/public/global.json — This root setting changes the documented VSTest leg in `samples/public/mstest-runner/runner_vs_vstes… View resolved comment |
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The VSTest solution still defaults directory-based invocations to ARM64.
Review tier: Balanced
Findings: None
Issues resolved since last review (3)
| Severity | Finding |
|---|---|
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj — The AnyCPU fallback omits native x86. When PROCESSOR_ARCHITECTURE=x86, the target/RID remain… View resolved comment |
|
samples/public/WinUIMtpUnpackagedApp/WinUIMtpUnpackagedApp.csproj — The AnyCPU fallback still has no native x86 branch. On a 32-bit Windows host,… View resolved comment |
|
samples/public/WinUIMtpPackagedApp/WinUIMtpPackagedApp.csproj — The AnyCPU fallback omits native x86. When PROCESSOR_ARCHITECTURE=x86, the target/RID remain… View resolved comment |
Suppressed comments (1)
Previously missed (1) — in code that hasn't changed since the last review.
samples/public/WinUIVSTestApp/WinUIVSTestApp.csproj:8
- This project-level default does not fix directory-based
dotnet testfor the VSTest sample. That invocation discoversWinUIVSTestApp.sln, whose first/default configuration is stillDebug|ARM64(WinUIVSTestApp.sln:9-15); the solution then suppliesPlatform=ARM64as a global property, so this empty-value condition cannot replace it and x64 Windows can still build/launch the ARM64 app. Update the solution configurations so x64 is the default (as done for both MTP.slnxfiles), while retaining ARM64 as an explicit project platform.
Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Good catch, and confirmed. I reproduced it with Fixed in b8ff3a1 by reordering |


Summary
x64so directory-baseddotnet testinvocations no longer build/launch an ARM64 executable on x64 Windows (e.g.dotnet test .\WinUIMtpUnpackagedApp\was failing withSystem.ComponentModel.Win32Exception (216): ... is not a valid application for this OS platform).ARM64from the WinUI MTP.slnxdefault configurations so the default solution platform matches the host architecture; ARM64 remains available as an explicit project platform.IsTestProject=true) for consistent discovery.AnyCPUfallback architecture/RID/publish-profile settings for direct WinUI project builds.samples/public/global.jsonto select theMicrosoft.Testing.Platformrunner for directory-baseddotnet testinvocations, with localglobal.jsonopt-outs/opt-ins for the VSTest-only and MTP-only samples so each keeps using its intended runner.Testing
dotnet test .\WinUIMtpUnpackagedApp\(with binlog) - passes (3/3 tests).dotnet test .\WinUIMtpPackagedApp\(with binlog) - passes (2/2 tests).dotnet build .\WinUIVSTestApp\WinUIVSTestApp.csproj(with binlog) - succeeds.dotnet testinDemoMSTestSdk\ProjectUsingMSTestRunner- uses MTP, passes.dotnet testinDemoMSTestSdk\ProjectUsingVSTest- remains on VSTest, passes.Follow-up
A UWP + Microsoft.Testing.Platform sample isn't possible yet because the adapter compiles its MTP integration layer out for UWP targets. That is being addressed separately in its own PR; a UWP MTP sample can be added here (or in a follow-up) once it lands.