[Testing] Fix for flaky UITests in CI - 4 #33215
Conversation
dotnet-policy-service
bot
added
community ✨
There was a problem hiding this comment.
Pull request overview
This PR addresses flaky UI tests in CI by implementing retry mechanisms and disabling animations in CarouselView tests. The changes focus on improving test reliability for CarouselView scroll operations on iOS and drag-and-drop coordinate detection across all platforms.
Key changes:
- Added iOS-specific retry logic for CarouselView ScrollTo operations that occasionally scroll to the wrong item in CI
- Implemented a retry mechanism with up to 3 attempts for drag-and-drop operations that may not fire drop events consistently
- Moved the "Apply" button from toolbar to main content area for better accessibility and relocated animation disabling to ScrollTo calls
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
src/Controls/tests/TestCases.Shared.Tests/Tests/FeatureMatrix/CarouselViewFeatureTests.cs |
Added iOS-specific retry mechanism to handle flaky ScrollTo behavior in CI |
src/Controls/tests/TestCases.Shared.Tests/Tests/DragAndDropUITests.cs |
Implemented retry loop for drag-and-drop operations with validation of drop event firing |
src/Controls/tests/TestCases.HostApp/FeatureMatrix/CarouselView/CarouselViewOptionsPage.xaml |
Migrated Apply button from toolbar to main content for better accessibility |
src/Controls/tests/TestCases.HostApp/FeatureMatrix/CarouselView/CarouselViewControlPage.xaml.cs |
Disabled animation in ScrollTo method to ensure immediate, deterministic navigation |
| for(int i = 0; i < 3; i++) | ||
| { | ||
| App.DragAndDrop("Blue", "Green"); | ||
| Thread.Sleep(500); |
There was a problem hiding this comment.
Using Thread.Sleep(500) is a code smell in UI tests. This hard-coded delay can cause tests to be slower than necessary or still fail if the UI isn't ready. Consider using App.WaitForElement or similar waiting mechanisms that poll for the expected state instead of arbitrary waits.
| Thread.Sleep(500); |
| } | ||
| } | ||
|
|
||
| if(!dragDropSuccess) |
There was a problem hiding this comment.
Missing space after if keyword. The code should be if (!dragDropSuccess) instead of if(!dragDropSuccess) to follow C# style conventions.
| if(!dragDropSuccess) | |
| if (!dragDropSuccess) |
| App.WaitForElement("DropRelativeLabel"); | ||
| App.WaitForElement("DragStartRelativeScreen"); | ||
| bool dragDropSuccess = false; | ||
| for(int i = 0; i < 3; i++) |
There was a problem hiding this comment.
The magic number 3 for retry attempts should be extracted to a named constant (e.g., const int MaxRetryAttempts = 3) to improve code readability and maintainability.
| for(int i = 0; i < 3; i++) | ||
| { | ||
| App.DragAndDrop("Blue", "Green"); | ||
| Thread.Sleep(500); |
There was a problem hiding this comment.
The magic number 500 (milliseconds) should be extracted to a named constant (e.g., const int DragDropDelayMs = 500) to improve code readability and make it easier to adjust timing if needed.
| App.WaitForElement("Item 4"); | ||
| } | ||
| catch (TimeoutException) | ||
| { |
There was a problem hiding this comment.
The retry logic silently catches and retries on failure without logging. If this iOS-specific flakiness continues, there will be no diagnostic information in test logs to understand why the retry was needed or if the retry mechanism itself is working. Consider adding a log statement or test output when the retry is triggered.
| { | |
| { | |
| TestContext.WriteLine("VerifyCarouselViewWithScrollTo: initial ScrollTo did not reach 'Item 4' within the timeout on iOS; retrying ScrollToButton tap."); |
| App.WaitForElement("DropRelativeLabel"); | ||
| App.WaitForElement("DragStartRelativeScreen"); | ||
| bool dragDropSuccess = false; | ||
| for(int i = 0; i < 3; i++) |
There was a problem hiding this comment.
Missing space after for keyword. The code should be for (int i = 0; i < 3; i++) instead of for(int i = 0; i < 3; i++) to follow C# style conventions.
| for(int i = 0; i < 3; i++) | |
| for (int i = 0; i < 3; i++) |
4 participants
This pull request introduces improvements to the CarouselView feature and enhances the reliability of drag-and-drop UI tests. The most significant changes include updating the way the "Apply" action is triggered in the CarouselView options page, making CarouselView scroll operations non-animated, and improving the robustness of UI tests for both CarouselView and drag-and-drop scenarios.
CarouselView improvements:
CarouselViewOptionsPage.xamlfor better accessibility and user experience.ScrollTomethod inCarouselViewControlPage.xaml.csto disable animation, ensuring immediate navigation to the selected item.UI test reliability enhancements:
DragAndDropUITests.csby retrying the drag-and-drop operation up to three times and asserting failure if the drop event does not fire, increasing test robustness against flakiness.VerifyCarouselViewWithScrollTotest on iOS, re-attempting the scroll action if the expected item does not appear, to handle platform-specific flakiness.