[Testing] Refactoring Feature Matrix UITest Cases for Entry Control#34632
[Testing] Refactoring Feature Matrix UITest Cases for Entry Control#34632LogishaSelvarajSF4525 wants to merge 7 commits intodotnet:mainfrom
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.sh | bash -s -- 34632Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/maui/main/eng/scripts/get-maui-pr.ps1) } 34632" |
|
Hey there @@LogishaSelvarajSF4525! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
dotnet-policy-service
bot
added
the
partner/syncfusion
NafeelaNazhir
added
area-testing
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR refactors the Entry Feature Matrix page and options UI to be more testable and easier to reset between scenarios, while updating platform snapshot baselines to reflect the UI/test behavior changes.
Changes:
- Add new bindable properties to the Entry feature matrix (WidthRequest, BackgroundColor, Opacity) and centralize state reset via
EntryViewModel.Reset(). - Simplify the options UI event handling (e.g., using
AutomationIdto select colors; new handlers for width/opacity/height inputs; updated font attributes handling). - Update WinUI/macOS/iOS snapshot images to match the updated UI/test baselines.
Reviewed changes
Copilot reviewed 6 out of 148 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/EntryControl/EntryViewModel.cs | Adds new properties (BackgroundColor/Opacity/WidthRequest) and a Reset() method to restore defaults for test runs. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/EntryControl/EntryOptionsPage.xaml.cs | Refactors option handlers (color selection by AutomationId, numeric input handlers, font-attributes checkbox aggregation). |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/EntryControl/EntryControlPage.xaml.cs | Uses _viewModel.Reset() instead of re-instantiating the view model when navigating to options. |
| src/Controls/tests/TestCases.HostApp/FeatureMatrix/EntryControl/EntryControlPage.xaml | Binds new properties to the Entry, updates Options AutomationId, and wires CursorPositionEntry TextChanged for better interactivity. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/VerifyClearButtonVisiblityWhenTextAlignedVertically.png | Updated iOS snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/VerifyClearButtonVisiblityWhenTextAlignedHorizontally.png | Updated iOS snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/ClearButtonVisiblityButton_TextPresent.png | Updated iOS snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios/ClearButtonVisiblityButton_TextEmpty.png | Updated iOS snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/VerifyTextWhenAlingnedVertically.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/VerifyTextWhenAlingnedHorizontally.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/VerifyClearVisiblityButtonWhenTextColorChanged.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/VerifyClearButtonVisiblityWhenTextAlignedVertically.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/VerifyClearButtonVisiblityWhenTextAlignedHorizontally.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/ClearButtonVisiblityButton_TextPresent.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.iOS.Tests/snapshots/ios-26/ClearButtonVisiblityButton_TextEmpty.png | Updated iOS 26 snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyTextWhenAlignedVertically.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyTextWhenAlignedHorizontally.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyIsPasswordWhenMaxLengthSetValue.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyClearVisibilityButtonWhenTextColorChanged.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyClearButtonVisibilityWhenTextAlignedVertically.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyClearButtonVisibilityWhenTextAlignedHorizontally.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/VerifyClearButtonVisibilityWhenIsPasswordTrueOrFalse.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/ClearButtonVisibilityButton_TextPresent.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.WinUI.Tests/snapshots/windows/ClearButtonVisibilityButton_TextEmpty.png | Updated WinUI snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/VerifyTextWhenAlignedVertically.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/VerifyTextWhenAlignedHorizontally.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/VerifyIsPasswordWhenMaxLengthSetValue.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/VerifyClearVisibilityButtonWhenTextColorChanged.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/VerifyClearButtonVisibilityWhenTextAlignedHorizontally.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/VerifyClearButtonVisibilityWhenIsPasswordTrueOrFalse.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/ClearButtonVisibilityButton_TextPresent.png | Updated macOS snapshot baseline. |
| src/Controls/tests/TestCases.Mac.Tests/snapshots/mac/ClearButtonVisibilityButton_TextEmpty.png | Updated macOS snapshot baseline. |
| private void HeightRequestEntry_TextChanged(object sender, TextChangedEventArgs e) | ||
| { | ||
| if (double.TryParse(e.NewTextValue, out double heightRequest)) | ||
| { | ||
| _viewModel.FontAttributes = FontAttributes.Italic; | ||
| _viewModel.HeightRequest = heightRequest; | ||
| } | ||
| } |
There was a problem hiding this comment.
HeightRequestEntry_TextChanged sets HeightRequest directly from user input without validating range. Given HeightRequest uses “-1 = unset, otherwise non-negative”, consider coercing values < 0 to -1 (or ignoring them) to avoid applying invalid heights via the options UI.
| <local:UITestEntry x:Name="CursorPositionEntry" | ||
| Text="{Binding CursorPosition}" | ||
| AutomationId="CursorPositionEntry" | ||
| TextChanged="CursorPositionButton_Clicked" | ||
| IsCursorVisible="False" |
There was a problem hiding this comment.
CursorPositionEntry is wired to TextChanged but the handler is named CursorPositionButton_Clicked (and takes EventArgs). Renaming the handler (and parameter type) to reflect it’s a TextChanged handler would make the intent clearer and reduce confusion when navigating the code.
| private void WidthRequestEntry_TextChanged(object sender, TextChangedEventArgs e) | ||
| { | ||
| if (sender == FontAttributesBold) | ||
| if (double.TryParse(e.NewTextValue, out double widthRequest)) | ||
| { | ||
| _viewModel.FontAttributes = FontAttributes.Bold; | ||
| _viewModel.WidthRequest = widthRequest; | ||
| } | ||
| else if (sender == FontAttributesNone) | ||
| } |
There was a problem hiding this comment.
WidthRequestEntry_TextChanged currently accepts any parsed double (including negative values other than -1). Since WidthRequest semantics are “-1 = unset, otherwise non-negative”, consider coercing values < 0 to -1 (or ignoring them) so invalid widths can’t be applied via the options UI.
🚦 Gate — Test Verification📊 Expand Full Gate —
|
| # | Type | Test Name | Filter |
|---|---|---|---|
| 1 | UITest | EntryFeatureTests | EntryFeatureTests |
Verification
| Step | Expected | Actual | Result |
|---|---|---|---|
| Without fix | FAIL | FAIL | ✅ |
| With fix | PASS | FAIL | ❌ |
Details
- ❌ Failed: VerifyTextWhenSelectionLengthSetValue [20 s]; VerifyTextWhenCursorPositionValueSet [21 s]; VerifyIsPasswordWhenCursorPositionValueSet [22 s]; VerifyCursorPositionWhenSelectionLengthSetValue [22 s]
- 📋 Error: This test is currently failing on All platforms. See issue link: Inconsistent Behavior of IsSpellCheckEnabled and IsTextPredictionEnabled on Entry Control #29833
Standard Output Messages:
This test is currently failing on All platforms. See issue lin...; This test is currently failing on All platforms. See issue link: Inconsistent Behavior of IsSpellCheckEnabled and IsTextPredictionEnabled on Entry Control #29833
Standard Output Messages:
This test is currently failing on All platforms. See issue lin...; System.TimeoutException : Timed out waiting for element...; System.TimeoutException : Timed out waiting for element...; System.TimeoutException : Timed out waiting for element...; System.TimeoutException : Timed out waiting for element...
Fix Files Reverted
eng/pipelines/ci-copilot.yml
Base Branch: main | Merge Base: 720a9d4
🤖 AI Summary📊 Expand Full Review —
|
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #34632 | Add TextChanged to CursorPositionEntry; add BackgroundColor/Opacity/WidthRequest bindings; rename snapshots; add Reset() to ViewModel | ❌ FAILED (Gate) | 6 files + ~100 snapshots | Cursor position tests (Order 36-39) timeout; TextChanged handler may cause interference |
🔧 Fix — Analysis & Comparison
Fix Candidates
| # | Source | Approach | Test Result | Files Changed | Notes |
|---|---|---|---|---|---|
| PR | PR #34632 | Rename "OptionsButton"→"Options"; add TextChanged to CursorPositionEntry; add intermediate assertions in cursor tests; add BackgroundColor/Opacity/WidthRequest bindings; add Reset() to ViewModel | ❌ FAILED (Gate) | 6 files + ~100 snapshots | Cursor position tests (Order 36-39) timeout: Focus() in OnUpdateCursorAndSelectionClicked raises Android keyboard, pushing elements off-screen |
| 1 | try-fix (claude-opus-4.6) | Test-only: Add App.DismissKeyboard() after App.Tap("UpdateCursorAndSelectionButton") in each of the 4 failing cursor position tests |
✅ PASS | EntryFeatureTests.cs (+4 lines) |
Simple, explicit, preserves all intermediate assertions; DismissKeyboard pattern already used throughout the file |
| 2 | try-fix (claude-sonnet-4.6) | HostApp: Add EntryControl.Unfocus() at end of OnUpdateCursorAndSelectionClicked |
❌ FAIL | EntryControlPage.xaml.cs (+1 line) |
Unfocus() calls ClearFocus() on Android EditText which doesn't reliably dismiss the IME; keyboard stayed visible for full 21s timeout |
| 3 | try-fix (gpt-5.3-codex) | HostApp: Remove both EntryControl.Focus() calls from OnUpdateCursorAndSelectionClicked |
✅ PASS | EntryControlPage.xaml.cs (-2 lines) |
Fixes root cause at HostApp level; but removes intentional focus behavior (user can't visually see cursor update without tapping Entry) |
| 4 | try-fix (gpt-5.4) | Test: Remove PR-added intermediate assertions (Is.EqualTo("5") checks), reverting to simpler main-branch pattern |
✅ PASS | EntryFeatureTests.cs (-8 lines) |
Simplifies tests but removes intermediate state verification added by PR |
| 5 | try-fix (gpt-5.3-codex cross-poll) | HostApp: async void + await EntryControl.HideSoftInputAsync(CancellationToken.None) at end of handler |
❌ FAIL | EntryControlPage.xaml.cs (+2 lines) |
HideSoftInputAsync after the fact doesn't reliably hide keyboard before Appium checks elements |
Cross-Pollination
| Model | Round | New Ideas? | Details |
|---|---|---|---|
| claude-opus-4.6 | 2 | NO | — |
| gpt-5.3-codex | 2 | YES | HideSoftInputAsync approach → ran as Attempt 5, FAILED |
| claude-sonnet-4.6 | 2 | YES | IsReadOnly=true before Focus — increasingly complex, not tried |
| claude-opus-4.6 | 3 | YES | Replace Focus() with text setting — not applicable to cursor position |
| gpt-5.3-codex | 3 | YES | Native EditText.ShowSoftInputOnFocus=false — overly complex/platform-specific |
Exhausted: Yes — max 3 rounds reached; remaining ideas are increasingly complex and impractical
Selected Fix: Candidate #1 (Attempt 1 — App.DismissKeyboard() in tests)
Reason:
- Minimal test-only change (+4 single lines across 4 tests)
- Preserves ALL intermediate assertions (more coverage than Attempt 4)
- Preserves intended HostApp UX (Entry visually shows cursor position update)
DismissKeyboard()pattern already widely used throughout the test class- Makes keyboard handling explicit in test — self-documenting
📋 Report — Final Recommendation
⚠️ Final Recommendation: REQUEST CHANGES
Phase Status
| Phase | Status | Notes |
|---|---|---|
| Pre-Flight | ✅ COMPLETE | Issue #34611, testing-only PR, 6 modified files + ~100 snapshot renames |
| Gate | ❌ FAILED | Android — cursor position tests (Order 36-39) timeout |
| Try-Fix | ✅ COMPLETE | 5 attempts (3 pass, 2 fail); cross-pollination exhausted Round 3 |
| Report | ✅ COMPLETE |
Summary
PR #34632 is a testing-only refactoring for the Entry control feature matrix. It adds BackgroundColor, Opacity, and WidthRequest bindings to the HostApp, renames ~100 snapshots to fix a typo (ClearButtonVisiblity* → ClearButtonVisibility*), renames the Options button AutomationId from "OptionsButton" to "Options", adds a Reset() method to EntryViewModel, and introduces new cursor position and selection length tests (Order 36-39).
The gate failed because the new cursor position tests (Order 36-39) timeout on Android. OnUpdateCursorAndSelectionClicked calls EntryControl.Focus() which shows the Android soft keyboard, pushing CursorPositionEntry and SelectionLengthEntry off-screen. The PR added intermediate App.WaitForElement() checks right after the button tap — these timeout because the elements are obscured by the keyboard.
Try-Fix found a clean fix: add App.DismissKeyboard() after each App.Tap("UpdateCursorAndSelectionButton") call in the 4 failing tests.
There are also 3 unresolved Copilot inline review comments about input validation and handler naming.
Root Cause
OnUpdateCursorAndSelectionClicked in EntryControlPage.xaml.cs calls EntryControl.Focus() to ensure the Entry is focused before setting CursorPosition and SelectionLength. On Android, Focus() triggers the soft keyboard to appear, which pushes the UI upward. Elements below the keyboard (including CursorPositionEntry and SelectionLengthEntry) become off-screen. The PR added new assertions immediately after tapping the button — these App.WaitForElement() calls timeout because those elements are no longer accessible to Appium while the keyboard is visible.
Fix Quality
PR's approach: ❌ Incomplete. The PR added TextChanged="CursorPositionButton_Clicked" to CursorPositionEntry and new intermediate assertions, but did not account for the keyboard appearing after UpdateCursorAndSelectionButton is tapped.
Selected alternative (Candidate #1 — App.DismissKeyboard()):
App.Tap("UpdateCursorAndSelectionButton");
+ App.DismissKeyboard();
App.WaitForElement("SelectionLengthEntry");
Assert.That(App.WaitForElement("SelectionLengthEntry").GetText(), Is.EqualTo("5"));Applied in 4 tests (Order 36, 37, 38, 39). Minimal, test-only, self-documenting, consistent with existing patterns in the test class.
Additional Issues (not blocking, should be addressed)
-
Copilot inline comments (3 unresolved):
HeightRequestEntry_TextChanged: should reject/coerce negative values forHeightRequestWidthRequestEntry_TextChanged: should reject/coerce negative values forWidthRequestCursorPositionButton_Clickednaming: handler is used asTextChangedbut named as_Clicked— should be renamed
-
VerifyEntryBackgroundColorResetToDefaultguard:#if TEST_FAILS_ON_CATALYST && TEST_FAILS_ON_IOScorrectly excludes this test from iOS/macCatalyst where issue [iOS, Maccatalyst] Entry & Editor BackgroundColor not reset to Null #34611 exists (conventional codebase pattern). No change needed.
Required Change
Add App.DismissKeyboard() after App.Tap("UpdateCursorAndSelectionButton") in the 4 failing cursor position tests in EntryFeatureTests.cs.
MauiBot
added
s/agent-review-incomplete
MauiBot
added
the
s/agent-changes-requested
|
/azp run maui-pr-uitests |
|
Azure Pipelines successfully started running 1 pipeline(s). |
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!
This pull request updates the
EntryControlPageand its code-behind to improve testability and maintainability of the Entry control's feature matrix. The main changes include adding new bindable properties, improving event handling, and simplifying the options navigation logic.Enhancements to Entry control testability and UI:
WidthRequest,BackgroundColor, andOpacityto the Entry control inEntryControlPage.xamlto allow more comprehensive property testing.AutomationIdof the options button from"OptionsButton"to"Options"for consistency with test automation.TextChangedevent handler (CursorPositionButton_Clicked) to theCursorPositionEntrycontrol, enabling dynamic updates to cursor position via UI interaction.Code simplification and maintainability:
NavigateToOptionsPage_Clickedwith a call to the new_viewModel.Reset()method, making the reset logic more maintainable and centralized.Issue Identified: