[ci-fix-net11] De-flake EditorRuntimeTextAlignmentChanged UI test (NoSuchElementException race on iOS) - #36395
Conversation
… tapping EditorRuntimeTextAlignmentChanged (Issue10987) intermittently failed on the iOS latest simulator with NoSuchElementException because its first action, App.Tap(LTREditor), ran before the editor had rendered. The sibling test EditorPlaceholderRuntimeTextAlignmentChanged already waits for its target element first; this applies the same synchronization. Adds App.WaitForElement(LTREditor) before the first interaction so the test is deterministic. No assertion is weakened and VerifyScreenshot is unchanged. Refs: #36393 Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
MauiBot
left a comment
There was a problem hiding this comment.
AI Review Summary
@github-actions[bot] — new AI review results are available based on this last commit:
e2fcaac. To request a fresh review after new comments or commits, comment/review rerun.
🗂️ Review Sessions — click to expand
🚦 Gate — Test Before & After Fix
Gate Result: ⚠️ SKIPPED
Gate skipped (SkipUITests fast test mode). No UI/device tests were run for this pipeline invocation.
📋 Pre-Flight — Context & Validation
Issue: #36393 - [ci-scan-net11] Flaky: EditorRuntimeTextAlignmentChanged — NoSuchElementException on iOS (vlatest) UITests (net11.0)
PR: #36395 - [ci-fix] De-flake EditorRuntimeTextAlignmentChanged UI test (NoSuchElementException race on iOS)
Platforms Affected: iOS reported; android requested for candidate testing
Files Changed: 0 implementation, 1 test
Key Findings
- The linked issue reports intermittent
NoSuchElementExceptioninIssue10987.EditorRuntimeTextAlignmentChangedbefore the first interaction withLTREditor. - PR #36395 adds
App.WaitForElement(LTREditor);beforeApp.Tap(LTREditor);; it does not mute the test, weakenVerifyScreenshot(), add retry attributes, or change baselines. - The changed file is
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue10987.cs; the affected UI category isEditor. - Local
ghis unauthenticated, so PR metadata/comments were gathered through GitHub's public REST API. The local worktree also contains unrelated script edits, so public PR data was treated as authoritative for PR context.
Code Review Summary
Verdict: NEEDS_DISCUSSION
Confidence: low
Errors: 0 | Warnings: 0 | Suggestions: 0
Key code review findings:
- ℹ No code findings. The PR fix matches MAUI UI-test guidance to use
WaitForElementbefore interacting with an element that may not be rendered yet. - ℹ Coverage/verification gap: required-check state could not be authenticated locally, and Android device validation is blocked by missing
adb.
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #36395 | Add App.WaitForElement(LTREditor); before App.Tap(LTREditor); |
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue10987.cs |
Original PR; one-line deterministic wait before first interaction |
🔬 Code Review — Deep Analysis
Code Review — PR #36395
Independent Assessment
What this changes: Adds App.WaitForElement(LTREditor); before tapping LTREditor in EditorRuntimeTextAlignmentChanged.
Inferred motivation: De-flake an Appium timing race where the editor is not yet present when first tapped.
Reconciliation with PR Narrative
Author claims: Fixes intermittent iOS NoSuchElementException without muting or weakening the test.
Agreement/disagreement: Agrees. The fix matches MAUI UI-test guidance: wait for an element before interacting.
Prior Review Reconciliation
No prior ❌ Error findings found. Inline review comments were empty via the public PR comments API.
Blast Radius Assessment
- Runs for all instances: No — isolated to one UI test method.
- Startup impact: No.
- Static/shared state: No.
CI Status
- Required-check result:
gh pr checks --requiredunavailable because the local GitHub CLI is unauthenticated. Public combined status for heade2fcaacc5217ec7bb40a89908bfebff03b3a029creturnedpendingwith zero contexts. - Classification: undetermined.
- Action taken: confidence capped low.
Findings
No ❌ Errors,
Failure-Mode Probing
- If
LTREditoris not rendered yet:WaitForElementblocks until present, addressing the reported first-interactionNoSuchElementException. - If the page is not fully settled but
LTREditorexists: the PR fix proves target presence but does not prove adjacent control readiness; laterApp.WaitForElement(RTLEditor)still guards the second editor. - Cross-platform impact: safe; test-only change, no product code, no assertion weakening, no screenshot baseline change.
Verdict: NEEDS_DISCUSSION
Confidence: low
Summary: The code change is sound and minimal, with no code-review findings. Because required-check state could not be authenticated and the Android validation environment lacks adb, this cannot be promoted to a verified LGTM from this session alone.
🛠️ Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| 1 | try-fix-1 | Replace wait-then-second-lookup with App.WaitForElement(LTREditor).Tap() |
1 file | Potentially stronger than PR by tapping the waited element; Android SDK/adb missing prevented execution |
|
| 2 | try-fix-2 | Wait for RTLEditor as a whole-page readiness barrier before tapping LTREditor |
1 file | Uses sibling-test pattern but is more indirect than the PR fix; Android SDK/adb missing prevented execution |
|
| 3 | try-fix-3 | Remove the first tap and let EnterText(LTREditor, ...) focus after page readiness |
1 file | Meaningfully different but riskier because the tap is documented as Mac focus setup; Android SDK/adb missing prevented execution |
|
| PR | PR #36395 | Add App.WaitForElement(LTREditor); before App.Tap(LTREditor); |
1 file | Original PR; simplest and best-aligned with MAUI UITest guidance |
Cross-Pollination
| Model/Reviewer | Round | New Ideas? | Details |
|---|---|---|---|
| code-review skill + MAUI expert reviewer | 1 | Yes | Generated alternatives: reuse waited element for tap, wait for later page element as readiness barrier, require hittable/non-zero geometry, remove initial tap, add host-page readiness sentinel. |
| orchestrator | 1 | No further viable tested candidates | Geometry/hittable custom wait and host-page sentinel were not run because they are more invasive/verbose for a simple element-not-found race and not clearly better than candidates 1-3 or the PR fix. |
Exhausted: Yes
Selected Fix: PR #36395 — No alternative passed tests, and all candidate executions were blocked by missing Android SDK/adb. Among unverified options, try-fix-1 is the only candidate that may be stronger than the PR fix, but it is not demonstrably better without device execution. The PR fix remains the simplest deterministic synchronization matching .github/instructions/uitests.instructions.md.
Environment Blocker
All Android candidate tests used:
pwsh .github/scripts/BuildAndRunHostApp.ps1 -Platform android -TestFilter "FullyQualifiedName~Issue10987.EditorRuntimeTextAlignmentChanged"Each attempt failed before running tests with:
Android SDK (adb) not found. Please install Android SDK and ensure 'adb' is in PATH.
Failed to start or detect device
🏁 Report — Final Recommendation
Comparative Candidate Report — PR #36395
Candidates compared
| Rank | Candidate | Result | Assessment |
|---|---|---|---|
| 1 | pr |
Gate skipped; no regression failure observed | Adds the exact missing readiness wait before the first interaction with LTREditor. This is the simplest targeted synchronization fix and matches MAUI UI-test guidance for element-not-found races. |
| 2 | pr-plus-reviewer |
Same as pr |
Expert review produced no actionable findings, so this is identical to pr and not a distinct improvement. |
| 3 | try-fix-1 |
Blocked before test execution | Uses App.WaitForElement(LTREditor).Tap() to tap the waited element. This is plausible and slightly stronger against a theoretical wait-then-relookup race, but it was not empirically validated and is not necessary for the observed failure. |
| 4 | try-fix-2 |
Blocked before test execution | Waits for RTLEditor as a page-ready barrier before tapping LTREditor. It is deterministic but indirect because it waits for a neighboring/later element instead of the element being tapped. |
| 5 | try-fix-3 |
Blocked before test execution | Removes the initial tap and relies on EnterText to focus. This is riskiest because the existing tap is explicitly documented as needed for Mac focus behavior, while the reported flake is an iOS element-readiness race. |
No candidate failed regression tests. The Step 5a try-fix candidates were all blocked before UI test execution because Android SDK/adb was unavailable, so they cannot outrank the simpler PR fix on empirical grounds. Candidates that failed regression tests would be ranked below passing candidates; none are present here.
Winning candidate
Winner: pr
The raw PR fix is the best candidate because it directly addresses the root cause with the smallest safe change: wait for LTREditor before tapping it. It does not mute the test, add retry behavior, weaken assertions, remove platform-specific focus setup, or change screenshot verification. Expert review found no actionable improvements, making pr-plus-reviewer equivalent rather than superior.
🧭 Next Steps — review latest findings
No alternative fix was selected for this run. Review the session findings and CI results before merging.
|
/azp run maui-pr-uitests |
|
/azp run maui-pr-devicetests |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
1 similar comment
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
|
♻️ Attempt 1/10 — red is an unrelated CI flake, not caused by this PR (head The current reds are UI-test categories unrelated to this PR's A maintainer re-run (
|
|
Target test EditorRuntimeTextAlignmentChanged validated green on head e2fcaac across every platform it runs on (iOS vlatest, Android API 30, MacCatalyst ×2 — build 1511735, all Editor legs succeeded). The de-flake adds only an App.WaitForElement guard; no assertion weakened or test muted. Remaining red is unrelated pre-existing flake. Ready for maintainer review.
|
|
🎯 Target de-flake validated green — marking ready for review
Editor is not run on the WinUI legs, so there is no other-platform coverage gap. The diff is a genuine de-flake — it adds a single The remaining red checks are unrelated pre-existing UI flakes (Image / cancelled Shell-Layout categories + Build Analysis + devicetests) not attributable to this test-only change. Transitioning from draft → ready for review and adding Automated by the CI Failure Fixer (net11.0). A maintainer should confirm before merge.
|
|
Azure Pipelines: Successfully started running 1 pipeline(s). There may be pipelines that require an authorized user to comment /azp run to run. |
kubaflo
left a comment
There was a problem hiding this comment.
🔍 AI-generated review — automated multi-model ensemble (Claude Opus 4.8 · GPT-5.5 · Gemini 3.1 Pro), posted on behalf of @kubaflo. Not a human approval.
✅ LGTM — one-line UITest de-flake, verified correct
Three models reviewed this change independently, then cross-pollinated. Consensus: LGTM. The single added line — App.WaitForElement(LTREditor) before the first App.Tap(LTREditor) in EditorRuntimeTextAlignmentChanged — is the canonical, convention-compliant fix for the reported NoSuchElementException race.
Why the fix is correct & complete
- Root cause: the test's first Appium interaction was a bare
App.Tap(LTREditor)with no preceding query. A bareTapdoesn't retry element lookup, so if the page isn't queryable yet Appium throwsNoSuchElementExceptionimmediately.WaitForElementpolls until present, then taps — exactly the remedy documented in.github/instructions/uitests.instructions.md("UseWaitForElementbefore interacting with elements"). - Consistent with the file's own pattern: the sibling test
EditorPlaceholderRuntimeTextAlignmentChangedalreadyWaitForElement(RTLEditor)before its first interaction. - No remaining race: the HostApp page builds all four controls synchronously in
Init()in a single non-scrollingVerticalStackLayout(ResetAlignmentButtonis created before both editors), so the later unguardedTap("ResetAlignmentButton")is guaranteed in-tree by the time it runs. - No regression risk:
WaitForElementhas a timeout — it fails on a genuine non-render, so it doesn't mask real bugs; it's a read-only query that adds no artificial delay and doesn't shift theVerifyScreenshot()baseline (captured at the end).
CI classification (required checks red/pending — all PR-unrelated)
| Leg | State | Classification |
|---|---|---|
maui-pr-uitests |
fail | The modified Issue10987 / Editor shard is SUCCESS on all 4 platforms including iOS vlatest (the exact platform where the race was reported). The red rolls up from unrelated shards (Image/ImageButton/IndicatorView, Cells/CheckBox/DragAndDrop) + fail-fast-cancelled MacCatalyst legs. |
maui-pr-devicetests |
fail | Different surface — device tests never compile the TestCases.Shared.Tests UITest project, so this change can't affect them. Failures are the known FileSystem/Essentials NRE flake (CheckFileResultOpenReadAsyncMultipleTimes, OpenAppPackageFileAsync_Can_Load_File = dotnet/runtime#129813). |
maui-pr |
pending | Rollup build in progress; nil risk for a one-line UITest change. |
Verdict: LGTM (Opus high · Gemini high · GPT "high confidence the code change is correct," with a mechanical pending-CI caution that the CI classification above resolves). No code findings from any model. A maintainer /azp run maui-pr re-run should clear the unrelated flakes.
🔍 Automated ensemble review · no human approval implied · reds classified via AzDO test results at head e2fcaac.
Visual Failure Comparisons
Visual comparisonsFull-resolution CI snapshot evidence is preserved below. These images supplement the failure classification and do not change the deterministic verdict ceiling.
|
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_FillWithImageSourceFromUri - macos - visual comparison
CI reported 15.06% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_CenterWithImageSourceFromUri - macos - visual comparison
CI reported 12.58% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_AspectFitWithImageSourceFromUri - macos - visual comparison
CI reported 8.50% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_AspectFillWithImageSourceFromUri - macos - visual comparison
CI reported 14.42% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_FillWithImageSourceFromUri - macos - visual comparison
CI reported 13.04% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_CenterWithImageSourceFromUri - macos - visual comparison
CI reported 12.24% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_AspectFitWithImageSourceFromUri - macos - visual comparison
CI reported 8.10% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_AspectFillWithImageSourceFromUri - macos - visual comparison
CI reported 12.58% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
ShellFlyoutIconShouldNotBeBlack - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
NavigationBarShouldRemainHiddenAfterNavigatingBack - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyShellMenuItemsAlignedInRTL - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyShellFlyoutContentAlignedInRTL - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
TabBarShouldBeVisibleOnMacCatalyst - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyFlyoutSelectedCurrentItem - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
SearchHandlerShouldNotOverlap - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyTabBarIconIsLoaded - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
ShouldHideHeaderWhenTitleEmpty - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
CharacterSpacingShouldApply - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
TabBarIconsShouldAutoscaleShell - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
VerifySearchHandlerPlaceholderText - macos - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
DownSizeImageAppearProperly - ios - visual comparison
CI reported 21.79% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_FillWithImageSourceFromUri - ios - visual comparison
CI reported 28.46% difference in build 1511735.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
AI-generated visual evidence by GitHub Copilot.
Tests Failure Analysis
Test Failure Review: Not ready - click to expandOverall verdict: Not ready CI is not ready because Coverage: 139 checks · 127 passing · 12 failing · 0 pending · 0 inaccessible · 1 unmapped · 24 unexplained build legs · 0 unaccounted failing checks · 5 aborted failing checks · 0 canceled-build checks · 7 device-test unverified · 42 unattributed · 1 regressed-vs-base. Deterministic ceiling: Not ready — Build Analysis unmapped; 24 unexplained failed legs; 42 unattributed failures; 5 aborted checks; 7 unverified device-test checks;
Recommended actionTreat the PR as CI-blocked: investigate/fix the Evidence details
Visual failure comparisonsFull-resolution CI baseline, actual, and diff images are embedded below. They supplement the failure classification and do not change the deterministic verdict ceiling.
|
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_FillWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 15.06% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_CenterWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 12.58% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_AspectFitWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 8.50% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageAspect_AspectFillWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 14.42% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_FillWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 13.04% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_CenterWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 12.24% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_AspectFitWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 8.10% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
VerifyImageButtonAspect_AspectFillWithImageSourceFromUri - macos - Needs human investigation - visual comparison
CI reported 12.58% difference in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
ShellFlyoutIconShouldNotBeBlack - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
NavigationBarShouldRemainHiddenAfterNavigatingBack - macos - Needs human investigation - visual comparison
CI reported size differs - baseline is 789x563 pixels, actual is 1920x1051 pixels in build 1511735.
Relationship to PR: Needs human investigation - No decisive exact test-and-platform baseline attribution was available.
| CI baseline | Fresh PR actual | CI diff |
|---|---|---|
![]() |
![]() |
![]() |
|
The test fix contained in this PR is not related to the reported issue (a NoSuchElementException race on iOS). Analysis of the reported issue:Previously, the The PR #36270 includes the required changes to tap the "Selected" button on iOS 26 as well. Therefore, the fix in this PR #36395 is not related to the reported issue. This failure was already fixed by the #36270 PR changes @PureWeen / @kubaflo |
kubaflo
left a comment
There was a problem hiding this comment.
LGTM — approving. Multi-model consensus (Opus 4.8 / GPT-5.5 / Gemini 3.1 Pro): LGTM on this [ci-fix-net11] UI-test de-flake.
The change is a single line in one test method: App.WaitForElement(LTREditor) before App.Tap(LTREditor) in EditorRuntimeTextAlignmentChanged. It fixes the NoSuchElementException race where the test tapped the editor before it had rendered (notably on iOS/mac). Test-only, no product code touched.
CI: the 12 red legs are unrelated flakes, not caused by this PR.
- The changed test's own category —
[Category(UITestCategories.Editor)]— passed; no "Editor" leg is red. - All failures are in other categories (Image/ImageButton, Layout, CollectionView, SearchBar, Shell) plus net11 iOS/MacCatalyst Helix device tests + Build Analysis — the known-flaky MacCatalyst/iOS/net11 surface. An independent maintainer review on this PR also classified the CI failures as unrelated to the change.
<!-- 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 Automates visual snapshot evidence in `/review tests`. When public AzDO results contain failed screenshot comparisons, the command now emits exactly one test-failure analysis comment containing bounded, expandable baseline/actual/diff panels. Visual evidence remains supplementary: it does not change `gate.verdictCeiling`, deterministic attribution, or the merge-readiness verdict. #### One-comment flow 1. Trusted pre-activation code discovers failed visual results through the public AzDO `resultsbybuild` API, including retry-suffixed attachments such as `Snapshot[1].png` and `Snapshot-diff[1].png`. 2. It resolves baselines from the exact source version tested by AzDO and maps runtime evidence to the correct snapshot directory (`ios-26`, `android-notch-36`, `mac`, or `windows`). 3. It streams and validates bounded PNG assets, then stores them on `review-tests-assets` using immutable commit-pinned `raw.githubusercontent.com` URLs. 4. The Copilot agent emits the normal single `add_comment` analysis payload with a trusted insertion marker. 5. A sealed post-step validates the published asset manifest and injects as many expandable comparison panels as fit into that same comment. Each collapsed panel shows a conservative relationship label: - `Likely PR-caused` for an exact test/platform base regression or directly changed snapshot/test; - `Likely unrelated` for an exact base/known-issue match without direct visual scope; - `Needs human investigation` for unmatched or mixed evidence. 6. Excess comparisons are summarized as omitted instead of creating a companion comment. The local `.github/scripts/Review-Tests.ps1 -PostComment` path uses the same merger. It also recognizes complete reports returned in Copilot's final response, preserving nested evidence code fences without wrapping a second title or badge section. ### Security and Failure Safety - PR text, logs, test names, attachment metadata, changed files, and visual labels remain untrusted input. - The merger script and visual context are copied to a root-owned location before the workflow checks out the untrusted PR branch. - The post-step runs without `COPILOT_GITHUB_TOKEN`, `GH_TOKEN`, or `GITHUB_TOKEN`. - AzDO attachment URLs must match the expected public project and attachment route. - Published assets are size-bounded, signature-checked PNGs with validated dimensions and repository paths. - Raw image URLs must match the exact repository, asset commit, PR directory, and safe filename. - Labels are HTML-escaped and `@` is neutralized before insertion. - Relationship labels use fixed trusted text. Untrusted attribution values are never rendered, and same-named snapshots changed on another platform do not count as PR scope. - The final body is checked panel-by-panel against conservative limits of 45 URLs, 10 mentions, and 60,000 UTF-16 characters, below gh-aw's throwing limits. - The analysis JSON update is atomic (written to a temp file, then renamed over the original). Invalid context, malformed output, missing analysis payloads, limit failures, and dry-run/noop output leave the original analysis unchanged. - The publisher never creates or patches PR comments; only the existing gh-aw `add_comment` payload is mutated. ### What NOT to Do - Do not use the ordinary anonymous AzDO test-runs listing for discovery; it redirects to sign-in. Use the public failed-results endpoint. - Do not resolve baselines from the current PR head; use the source version actually tested by the selected AzDO build. - Do not let the agent construct or trust visual asset URLs. - Do not publish a second companion comment; merge bounded panels into the single analysis payload. ### Validation - 98 focused Pester tests pass. - Changed PowerShell scripts parse successfully. - `gh aw compile copilot-review-tests --approve` completes without errors or warnings. - A real `agent_output.json` from gh-aw run [29674953402](https://github.com/dotnet/maui/actions/runs/29674953402) was replayed through the post-step: - one `add_comment` item remained one item; - five visual panels were inserted; - the final body contained 26 URLs, one mention, and 9,585 characters. ### Live Single-Comment Examples The exact local `/review tests` path from this branch posted or repaired these merged comments after the PRs' `/azp run` pipelines completed: | PR | Single merged result | Included evidence | Relationship labels | Final limits | | --- | --- | --- | --- | --- | | #36413 | [Test-failure analysis with visual panels](#36413 (comment)) | 5 panels / 15 images | 1 PR-caused, 4 investigate | 23 URLs, 13,156 chars | | #36631 | [Test-failure analysis with visual panels](#36631 (comment)) | 6 panels / 18 images | 2 PR-caused, 4 investigate | 31 URLs, 21,367 chars | | #36395 | [Test-failure analysis with visual panels](#36395 (comment)) | 11 panels / 33 images; 19 omitted | 11 investigate | 43 URLs, 19,858 chars | | #36404 | [Test-failure analysis with visual panels](#36404 (comment)) | 14 panels / 40 images; 81 omitted | 14 investigate | 45 URLs, 23,779 chars | | #35846 | [Test-failure analysis with visual panels](#35846 (comment)) | 10 panels / 30 images; 9 omitted | 10 investigate | 43 URLs, 22,496 chars | | #36277 | [Test-failure analysis with visual panels](#36277 (comment)) | 7 panels / 19 images | 3 PR-caused, 4 investigate | 31 URLs, 18,355 chars | | #36170 | [Test-failure analysis with visual panels](#36170 (comment)) | 11 panels / 33 images; 8 omitted | 11 investigate | 44 URLs, 23,180 chars | | #35578 | [Test-failure analysis with visual panels](#35578 (comment)) | 12 panels / 36 images; 50 omitted | 12 investigate | 44 URLs, 25,443 chars | | #36672 | [Test-failure analysis with visual panels](#36672 (comment)) | 14 panels / 40 images; 9 omitted | 14 investigate | 45 URLs, 25,915 chars | | #31755 | [Test-failure analysis with visual panels](#31755 (comment)) | 12 panels / 36 images; 3 omitted | 12 investigate | 44 URLs, 22,728 chars | | #34637 | [Test-failure analysis with visual panels](#34637 (comment)) | 9 panels / 27 images; 81 omitted | 9 investigate | 43 URLs, 22,325 chars | | #35156 | [Test-failure analysis with visual panels](#35156 (comment)) | 2 panels / 6 images | 1 PR-caused, 1 investigate | 19 URLs, 11,911 chars | | #35885 | [Test-failure analysis with no visual failures](#35885 (comment)) | 0 panels / 0 images | No visual failures detected | 8 URLs, 4,239 chars | | #36577 | [Test-failure analysis with visual panels](#36577 (comment)) | 1 panel / 3 images | 1 investigate | 44 URLs, 21,768 chars | | #36212 | [Test-failure analysis with no visual failures](#36212 (comment)) | 0 panels / 0 images | No visual failures detected | 5 URLs, 5,982 chars | Each result contains one `Tests Failure Analysis` title and one merged review marker. Across 114 rendered panels, all 336 embedded image URLs returned HTTP 200. Seven panels were safely classified as likely PR-caused; no panel in this sample had enough exact evidence to be safely classified as likely unrelated, so the remaining 107 stayed at `Needs human investigation`. Another 260 comparisons were omitted safely by the comment limits. The latest eight-example batch was regenerated concurrently, and #36672, #31755, #34637, #35156, #35885, #36577, and #36212 were added afterward. The current [`review-tests-assets` head](f937993) retains the full asset history. The protected `copilot-pat-pool` environment rejects feature-branch `workflow_dispatch` runs before job execution. The live local-runner examples validate comment generation and asset publication, while the real gh-aw output replay validates the workflow post-step mutation without weakening that branch protection. ### Issues Fixed N/A - reviewer workflow enhancement. --------- Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: kubaflo <kubaflo@users.noreply.github.com> Copilot-Session: a280b482-e102-4ca0-9ff9-1cfe1946e21f Copilot-Session: d00747b7-96f3-4e7a-8dfb-e3a48db04b2d Copilot-Session: 478d195b-20f3-4bc6-aeed-f6aa88b55fda


























































































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!
Target branch: net11.0
Refs: #36393
Attempt: 1/5
Outcome: de-flake (test-quality flake — Step 4.7 bucket b)
Summary
EditorRuntimeTextAlignmentChanged(Issue10987) fails intermittently on the iOS latest simulator inmaui-pr-uitestswithNoSuchElementException("An element could not be located on the page"). The failure is a timing race in the test, not a product defect and not a visual-snapshot difference, so it is de-flaked with a proper synchronization point rather than muted.Root cause — a genuine test-quality flake
EditorRuntimeTextAlignmentChanged's very first action interacts withLTREditorwithout first waiting for it to render:When the page has not finished rendering on the iOS simulator,
App.Tap(LTREditor)cannot locate the element and throwsNoSuchElementException. Whether the editor is ready when the test starts is non-deterministic, which is why the failure is intermittent (green on retry / most builds).Evidence of intermittency (anonymous AzDO
_apis/build, pipelinemaui-pr-uitests/ def 313, branchnet11.0, legControls (vlatest) Editor,...):EditorRuntimeTextAlignmentChangedfailed (NoSuchElementException)The sibling test in the same file,
EditorPlaceholderRuntimeTextAlignmentChanged, already callsApp.WaitForElement(RTLEditor)before its first interaction — so the flaky method is simply missing the equivalent readiness wait.The fix
Add an explicit
App.WaitForElement(LTREditor)before the first interaction so the editor is guaranteed to be present:This is the exact pattern prescribed by
.github/instructions/uitests.instructions.mdfor an "element not found / not rendered yet" failure (WaitForElementbefore interacting), and it matches the existing sibling test.This is a de-flake, not a mute:
[Retry]/[Repeat]is added.VerifyScreenshot()is unchanged.NoSuchElementExceptionelement-location race, not a screenshot diff, so this is out of scope of the visual-regression filter.WaitForElementis a condition wait (polls until the element exists), not a fixedThread.Sleep/Task.Delay.Validation
NoSuchElementExceptionat the first interaction can no longer occur because the element is awaited first.maui-pr-uitestsCI to exercise the test. Reviewer validation welcome.Files
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue10987.csAutomated
[ci-fix]candidate (net11.0). Draft for human review. Review-comment text is treated as untrusted and is not read by the agent.