Skip to content

[Testing] Fix flaky EntryClearButtonShouldBeVisibleOnDarkTheme test #34341

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.

Sign up for GitHub

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

Closed
PureWeen wants to merge 1 commit into from
+20 −3
Closed

[Testing] Fix flaky EntryClearButtonShouldBeVisibleOnDarkTheme test #34341

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion src/Controls/tests/TestCases.HostApp/Issues/Issue32886.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ namespace Maui.Controls.Sample.Issues;
[Issue(IssueTracker.Github, 32886, "[Android, iOS, Mac] Entry ClearButton not visible on dark theme", PlatformAffected.Android | PlatformAffected.iOS | PlatformAffected.macOS)]
public class Issue32886 : TestContentPage
{
Label _themeLabel;

protected override void Init()
{
Title = "Issue32886";
Expand All @@ -25,9 +27,18 @@ protected override void Init()
};
button.Clicked += Button_Clicked;

_themeLabel = new Label
{
Text = "Light",
AutomationId = "ThemeLabel",
HeightRequest = 0,
Opacity = 0
};

var layout = new VerticalStackLayout();
layout.Children.Add(entry);
layout.Children.Add(button);
layout.Children.Add(_themeLabel);

Content = layout;

Expand All @@ -39,7 +50,9 @@ private void Button_Clicked(object sender, EventArgs e)
{
if (Application.Current is not null)
{
Application.Current.UserAppTheme = Application.Current.UserAppTheme != AppTheme.Dark ? AppTheme.Dark : AppTheme.Light;
var newTheme = Application.Current.UserAppTheme != AppTheme.Dark ? AppTheme.Dark : AppTheme.Light;
Application.Current.UserAppTheme = newTheme;
_themeLabel.Text = newTheme == AppTheme.Dark ? "Dark" : "Light";
Comment on lines +53 to +55
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The _themeLabel.Text = "Dark" update in Button_Clicked happens synchronously right after Application.Current.UserAppTheme = newTheme, but on Windows the actual WinUI visual theme propagation uses DispatcherQueue.TryEnqueue (see Application.Windows.cs:105), which is an asynchronous dispatch. So WaitForTextToBePresentInElement("ThemeLabel", "Dark") only confirms the MAUI-level property assignment occurred; it does not guarantee that the WinUI RequestedTheme update and the subsequent re-render of child controls (e.g., the Entry's clear button color) have completed.

As a result, the test can still take a screenshot mid-theme-render on Windows. The retryTimeout: TimeSpan.FromSeconds(3) on VerifyScreenshot is the real mechanism compensating for this. To make the label a stronger synchronization point, its text should be updated after awaiting the theme change, or inside a RequestedThemeChanged callback rather than synchronously after setting UserAppTheme. As an alternative, a RequestedThemeChanged event on Application.Current could set the label text when the theme change is actually applied.

Note: If the retryTimeout alone is considered sufficient, the ThemeLabel approach is still useful as it reduces unnecessary wait time on fast machines—but in that case the comment "deterministic signal" in the PR description slightly overstates the guarantee provided.

Copilot uses AI. Check for mistakes.
}
}
}
8 changes: 6 additions & 2 deletions src/Controls/tests/TestCases.Shared.Tests/Tests/Issues/Issue32886.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,16 +40,20 @@ public void EntryClearButtonShouldBeVisibleOnDarkTheme()
{
App.WaitForElement("TestEntry");
App.Tap("ThemeButton");

// Wait for the theme change to propagate through the UI
App.WaitForTextToBePresentInElement("ThemeLabel", "Dark");

#if WINDOWS // On Windows, the clear button isn't visible when Entry loses focus, so manually focused to check its icon color.
App.Tap("TestEntry");
#endif

#if IOS
// On iOS, the virtual keyboard appears inconsistent with keyboard characters casing, can cause flaky test results. As this test verifying only the entry clear button color, crop the bottom portion of the screenshot to exclude the keyboard.
// Using DismissKeyboard() would unfocus the control in iOS, so we're using cropping instead to maintain focus during testing.
VerifyScreenshot(cropBottom: 1550);
VerifyScreenshot(cropBottom: 1550, retryTimeout: TimeSpan.FromSeconds(3));
#else
VerifyScreenshot();
VerifyScreenshot(retryTimeout: TimeSpan.FromSeconds(3));
Comment on lines +54 to +56
Copy link

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The AppThemeFeatureTests class (and most other screenshot tests in the codebase) consistently uses tolerance: 0.5 alongside retryTimeout for theme-change screenshots (e.g., VerifyScreenshot(tolerance: 0.5, retryTimeout: TimeSpan.FromSeconds(2))). This PR uses retryTimeout: TimeSpan.FromSeconds(3) without tolerance, which deviates from the established pattern for theme-change screenshot tests. Adding tolerance: 0.5 would be consistent with how similar tests handle minor cross-machine rendering differences (as seen in AppThemeFeatureTests.cs:31,40,55,67 etc.).

Copilot uses AI. Check for mistakes.
#endif
}
}
Loading