-
Notifications
You must be signed in to change notification settings - Fork 1.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Testing] Fix for flaky UITests in CI that occasionally fail - 5 #28269
[Testing] Fix for flaky UITests in CI that occasionally fail - 5 #28269
Conversation
Hey there @HarishKumarSF4517! Thank you so much for your PR! Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Overview
This PR improves the reliability of flaky UITests in CI by updating test interactions and timing for better platform-specific handling. Key changes include:
- Updating CarouselViewUITests to use gesture-based scrolling and switching from App.Click to App.Tap.
- Adding an extra WaitForElement in Issue20903 to ensure element readiness.
- Replacing the conditional Catalyst directive with a Windows-specific Thread.Sleep to handle scrollbar visibility issues.
Reviewed Changes
File | Description |
---|---|
Issue27418.cs | Introduces a Windows-specific delay to avoid flaky scrollbar tests. |
CarouselViewUITests.LoopNoFreeze.cs | Modifies scroll interactions to use gesture-based scrolling and updates button interaction. |
Issue20903.cs | Adds a WaitForElement before a tap to improve test robustness. |
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
Comments suppressed due to low confidence (1)
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27418.cs:19
- [nitpick] Consider replacing Thread.Sleep with an explicit wait mechanism to verify the scrollbar has disappeared, which would improve test robustness on Windows.
Thread.Sleep(2000); // Wait for scrollbar to disappear to avoid flaky test failures in CI
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can t we just hide the ScrollView bars? don t we have a API for that ?
@@ -18,8 +15,10 @@ public Issue27418(TestDevice device) : base(device) { } | |||
public void CarouselItemsShouldRenderProperly() | |||
{ | |||
App.WaitForElement("CarouselView"); | |||
#if WINDOWS | |||
Thread.Sleep(2000); // Wait for scrollbar to disappear to avoid flaky test failures in CI |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jsuarezruiz I have addressed this issue, resolved it, and committed the changes. Please review them and share any concerns you may have. Thank you!
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
/azp run |
Azure Pipelines successfully started running 3 pipeline(s). |
WinUI ScrollView parameters are broken so this code will get cleaned up later.
|
This pull request includes several updates to the test cases in the
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues
directory. The changes focus on improving test reliability and handling platform-specific issues.Improvements to test reliability:
src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/CarouselViewUITests.LoopNoFreeze.cs
: UpdatedIssue12574Test
to useScrollStrategy.Gesture
with a scroll percentage of0.99
and replacedApp.Click
withApp.Tap
for better interaction handling.src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue20903.cs
: Added aWaitForElement
call before the second tap onAddDoubleTapHandlerButton
to ensure the element is ready for interaction.src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue27418.cs
: Removed the#if TEST_FAILS_ON_CATALYST
directive and added a conditional sleep for Windows to prevent flaky test failures due to scrollbar visibility. [1] [2]TestCases