[release/11.0.1xx-preview7] Allow UI tests to apply resolution after test failure - #37044
[release/11.0.1xx-preview7] Allow UI tests to apply resolution after test failure#37044kubaflo wants to merge 3 commits into
Conversation
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 37044Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 37044" |
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
This PR adjusts the Windows screen-resolution helper script used by UI test runs so that a CDS_TEST probe failure (DISP_CHANGE_FAILED) no longer aborts the script before attempting the actual resolution change, improving resilience on hosted agents.
Changes:
- Downgrade
CDS_TESTreturningDISP_CHANGE_FAILEDfrom a hard error/exit to a warning so the script proceeds to apply the requested resolution.
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
The DISP_CHANGE_FAILED branch logged a redundant "failed (FAILED)" message without the return code or the resolution being attempted. Log the specific CDS_TEST result and the target resolution instead. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
|
/azp run maui-pr-uitests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
| @@ -174,8 +174,7 @@ namespace DisplaySettings | |||
| return $false | |||
| } | |||
| ([DisplaySettings.NativeMethods]::DISP_CHANGE_FAILED) { | |||
There was a problem hiding this comment.
💡 Testing — The new DISP_CHANGE_FAILED → real-apply transition is covered only by AST parsing, so a later control-flow refactor could restore the early exit or stop propagating a failed real apply. The native static call is not mockable in the current script structure; consider a follow-up that extracts the probe-result decision into a pure helper and tests FAILED = continue and BADMODE = fail, while preserving nonzero exit for real-apply failures.
Flagged by: 3/3 reviewers after severity dispute
There was a problem hiding this comment.
Agreed this is worth a follow-up. Extracting the probe-result decision (SUCCESSFUL → proceed, BADMODE → fail, FAILED → continue) into a pure helper would make the warn-and-continue transition unit-testable without the non-mockable ChangeDisplaySettings P/Invoke, and would lock in that a real-apply failure still returns nonzero.
I'm keeping it out of this PR intentionally — this change is a narrowly-scoped preview7 hot-fix (only the non-mutating CDS_TEST DISP_CHANGE_FAILED path), and restructuring the resolution flow on a Windows-only script I can't exercise on macOS is better done as its own change with the Pester coverage you describe. I'll track it as a follow-up. Leaving this thread open so it isn't lost.
There was a problem hiding this comment.
Addressed in 4eb296e1ed: the CDS_TEST and real-apply result decisions are now pure helpers used by the native flow. The focused Pester suite pins FAILED = continue, BADMODE = reject, and every non-success/restart real apply = failure. All 14 focused tests and the complete 1,513-test PowerShell suite pass with Pester 5.9.0.
PureWeen
left a comment
There was a problem hiding this comment.
Adversarial review — low risk
No correctness, security, or failure-propagation issue survived consensus. The change narrowly converts only the non-mutating CDS_TEST DISP_CHANGE_FAILED result to warn-and-continue; DISP_CHANGE_BADMODE and every failed real apply still return false and produce exit code 1. The title and description accurately match the implementation.
- 💡 Testing: One non-blocking coverage suggestion was retained at the lower consensus severity. AST parsing does not exercise the new branch, but direct end-to-end stubbing would require disproportionate refactoring around static P/Invoke and unconditional process exits; a pure decision helper is the feasible follow-up.
- What looks right: The warning includes the probe result and target resolution, while the unchanged real-apply switch preserves diagnosable hard failures.
- Prior review status: The earlier warning-detail comment was addressed in
c82d600; no unresolved correctness finding remains.
Methodology: 3 independent reviewers with adversarial consensus + repo domain specialist. The testing note reached 3/3 agreement only after severity dispute and is non-blocking. CI status was intentionally out of scope.
|
CI evidence from the exact-head UI run: maui-pr-uitests 1537883 executed 23 I inspected every resolution-task log. None emitted the new |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16
|
/azp run |
|
Azure Pipelines: Successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
Suppressed comments (1)
.github/scripts/Set-ScreenResolution.Tests.ps1:15
- Guard against $parseErrors being $null and include just the parse error messages for consistency with other Pester AST-based tests in this repo. This avoids a potential null-reference and makes failures easier to read.
if ($parseErrors.Count -gt 0) {
throw "Set-ScreenResolution.ps1 has parse errors: $($parseErrors -join '; ')"
}
|
@PureWeen this is ready to merge at unchanged head |
|
Consolidated into #37081, which is now the single rebased test-only stabilization PR for Preview 7. |
<!-- Please let the below note in for people that find this PR --> > [!NOTE] > Are you waiting for the changes in this PR to be merged? > It would be very helpful if you could [test the resulting artifacts](https://github.com/dotnet/maui/wiki/Testing-PR-Builds) from this PR and let us know in a comment if this change resolves your issue. Thank you! ### Description of Change Bundles independently valid Preview 7 test and test-infrastructure stabilizations into one merge path. Changes are cherry-picked or extracted from their source PRs without modifying MAUI runtime or product behavior. The aggregate currently covers: - exact DragEvents label polling and shared Appium text-wait behavior; - waiting for the iOS Entry keyboard before its snapshot; - adaptive ListView and CollectionView screenshot stabilization; - RefreshView gesture retry and SwipeView callback-result waiting; - stable initial SafeArea layout measurement; - deterministic iOS Picker-to-Entry keyboard transition handling; - a bounded Android screenshot tolerance for stable post-rotation antialiasing variance; - independent iOS and MacCatalyst Graphics image-scaling device coverage; - a bounded UIKit cleanup wait for the CarouselView leak assertion; - Android-only scoping for the Android modal-animation leak regression test; - an exact detached-page transition with a synchronous alert-request return signal; and - resilient Windows test-machine resolution setup, with focused Pester coverage for each fallback and failure path. Eligible UI tests, device tests, unit tests, test infrastructure, and screenshot baselines can all be included here when they are independently valid without a corresponding product-code change. ### Source PRs The standalone source PRs below are closed in favor of this aggregate. For #37057, only the independently valid test subset is included; its product-code rewrite remains excluded. | PR | Included test-side fix | |---|---| | #37044 | Windows screen-resolution test infrastructure and Pester coverage | | #37045 | DragEvents exact-text waits and Appium helper | | #37053 | iOS Entry keyboard readiness | | #37055 | ListView screenshot retry window | | #37057 | Graphics image-scaling device tests that pass without the runtime rewrite | | #37058 | RefreshView pull-to-refresh retry | | #37059 | SwipeView result-label wait | | #37066 | SafeArea initial-layout retry | | #37069 | Picker keyboard transition stabilization | | #37074 | Empty CollectionView footer screenshot retry/tolerance | | #37086 | iOS/MacCatalyst CarouselView leak-test cleanup wait | | #37088 | Detached-page alert request synchronization | | #37091 | Android modal-animation leak-test platform scope | | #37092 | Android Issue22306 post-rotation screenshot tolerance | ### Scope exclusions Mixed runtime/test fixes are included only when their test-side changes pass independently against the unmodified product code. The current device tests in #37052, the four nonpositive-size cases in #37057, and the device test plus screenshots in #37062 remain excluded because they expose or describe behavior that requires those PRs' functional fixes. #37070 has no test-side changes. Closed ineffective or unsafe fixes are also excluded. ### Validation - All 25 directly reusable source commits were cherry-picked in their original order. - The #37057 device-test subset was extracted into one additional test-only commit after proving it against the unmodified product implementation. - Every directly cherry-picked aggregate file matches the corresponding included source PR head. - The aggregate diff contains no MAUI runtime or product files. - Targeted Release builds pass for `UITest.Appium`, `Controls.TestCases.iOS.Tests`, `Controls.TestCases.Mac.Tests`, and `Controls.TestCases.Android.Tests`. - The focused screen-resolution Pester suite passes 14/14. - Exact local xUnit XML reports 46/46 Graphics device tests passing on both iOS 26 and MacCatalyst, including all 13 extracted cases. - The Graphics deadlock regression now asserts the bounded five-second completion result before awaiting the scaling task. - The CarouselView category passes 6 consecutive MacCatalyst runs at 4/4 each and passes 4/4 on iOS 26 with the final 100 ms cleanup wait. - The Android modal-animation leak test passes in both discovered Android variants, while the iOS device-test assembly excludes that Android-specific regression test. - The Issue22306 Android failure's three retries were byte-for-byte identical at a 1.85% visual difference, below the new Android-only 2% tolerance. - The unloaded-page alert probe now signals immediately after `DisplayAlertAsync` returns, avoiding a teardown-order dependency on the detached page task. - Rebased the aggregate onto release head `dee83edd121`; the resulting diff remains limited to tests and test infrastructure. - The Pester workflow now triggers when `eng/scripts/Set-ScreenResolution.ps1` changes. - A standalone `/azp run` was posted for aggregate head `83da465424b`; exact merge `bad9e4e9a57` builds `1539860`, `1539861`, and `1539862` are required before merge. ### Issues Fixed Contributes to stabilizing the .NET 11 Preview 7 test branch. --------- Co-authored-by: Vally Fixture <vally-fixture@example.invalid> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: kubaflo <34349119+kubaflo@users.noreply.github.com> Copilot-Session: 9be49656-7117-4235-9d96-404779ab6b16 Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d
Note
Are you waiting for the changes in this PR to be merged?
It would be very helpful if you could test the resulting artifacts from this PR and let us know in a comment if this change resolves your issue. Thank you!
Description of Change
The Windows UI-test resolution helper is designed to fall back to applying 1920x1080 when display enumeration or the non-mutating
CDS_TESTprobe is unreliable on a hosted agent. However, the genericDISP_CHANGE_FAILEDprobe result was treated as a hard error, so the script exited before attempting the actual resolution change.Treat that generic probe result as a warning and continue to the real
ChangeDisplaySettingscall. Unsupported modes (DISP_CHANGE_BADMODE) still fail immediately, and a failed actual resolution change still fails the script. The native probe/apply result decisions are now isolated in pure helpers so those transitions can be tested without invoking Windows P/Invoke APIs.CI Evidence
In maui-pr-uitests build 1535264, a Windows leg could not enumerate the current display and the
CDS_TESTprobe returnedDISP_CHANGE_FAILED. The build stopped inSet screen resolutionbefore any tests ran, even though the helper logged that it would attempt the fallback.Testing
CDS_TESTdecisions (FAILEDcontinues,BADMODErejects), successful/restart apply results, and nonzero real-apply failures..github/scriptsPester 5.9 suite: 1,513 passed, 0 failed.Issues Fixed
N/A