-
Notifications
You must be signed in to change notification settings - Fork 121
Replace Task.Delay with CompositionTargetHelper in RichSuggestBox tests #753
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
base: main
Are you sure you want to change the base?
Conversation
…ittent failure Co-authored-by: michael-hawker <24302614+michael-hawker@users.noreply.github.com>
|
Ironically also failed on Add Tokens too. @copilot can you ignore that intermittent failing test as well? |
…ent failure Co-authored-by: michael-hawker <24302614+michael-hawker@users.noreply.github.com>
Added Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Hmm... now it's in CustomizeToken. I feel like there's some underlying issue with RichTextBox on Windows App SDK that is just tripping us up here, as I think we only see these fail in WinUI 3 regularly vs. WinUI 2? @copilot instead of ignoring the tests, can you replace the Task delays across all the RichSuggestBox tests with the CompositionTargetHelper as suggested here: #589 (comment) Please also provide any further analysis into the problem and potential solutions |
Co-authored-by: michael-hawker <24302614+michael-hawker@users.noreply.github.com>
Replaced all Analysis: The intermittent failures appear to be caused by race conditions where Warning Firewall rules blocked me from connecting to one or more addresses (expand for details)I tried to connect to the following addresses, but was blocked by firewall rules:
If you need me to access, download, or install something from one of these locations, you can either:
|
|
Re-running all the checks again to see how it holds up, optimistic that first pass passed. |
|
Re-running all checks again while waiting for @Arlodotexe review. |
Fixes #589
Fixes the intermittent test failures in RichSuggestBox tests by replacing
Task.Delay(10)withCompositionTargetHelper.ExecuteAfterCompositionRenderingAsync()to properly wait for UI updates to complete.PR Type
What is the current behavior?
Test_RichSuggestBox_DeleteTokens,Test_RichSuggestBox_AddTokens, and other RichSuggestBox tests intermittently fail on WinUI 3 due to race conditions. The tests useawait Task.Delay(10)to wait for UI updates, which isn't always sufficient for the WinUI 3 RichTextBox to complete its text changes and fire events.What is the new behavior?
All
Task.Delay(10)calls in RichSuggestBox tests have been replaced withCompositionTargetHelper.ExecuteAfterCompositionRenderingAsync(() => { }), which properly waits for the composition rendering pass to complete before assertions run.Analysis: The intermittent failures appear to be caused by differences in the WinUI 3 platform's text control event timing compared to WinUI 2. The
CompositionTargetHelperapproach is already used in other tests across the toolkit (e.g.,SwitchPresenterTests,ConstrainedBox,TokenizingTextBox) and provides a more reliable synchronization mechanism.PR Checklist
Other information
Replaced 9 occurrences of
Task.Delay(10)across all RichSuggestBox tests:Test_RichSuggestBox_DeleteTokensTest_RichSuggestBox_ReplaceTokenTest_RichSuggestBox_FormatResetTest_RichSuggestBox_LoadAddTokenAsync(used by multiple tests)Original prompt
This pull request was created as a result of the following prompt from Copilot chat.
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.