Fix race condition in RemoveInnerPage unit test #32787
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The RemoveInnerPage test was failing randomly on CI due to a race condition: - The test removed a page but didn't wait for navigation to complete - TestNavigationHandler simulates async navigation with a 10ms delay - If the test completed before navigation finished, Appearing/Disappearing events could fire after test completion, throwing uncaught exceptions Fixed by: 1. Changed nav declaration from NavigationPage to var (TestNavigationPage) 2. Added await nav.NavigatingTask after RemovePage call This matches the pattern used in the RemoveLastPage test and ensures the test waits for async navigation to complete before finishing.
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR fixes a race condition in the RemoveInnerPage unit test that was causing intermittent failures on CI. The test was completing before asynchronous navigation finished, causing event handlers to fire after test completion and throw uncaught exceptions.
Key changes:
- Modified variable declaration to preserve
TestNavigationPagetype access - Added proper await for navigation completion to ensure deterministic test behavior
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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!
Description of Change
Fixes a race condition in the
RemoveInnerPageunit test that was causing random failures on CI.Issue
The test was failing randomly on CI because it didn't wait for the async navigation to complete after calling
nav.Navigation.RemovePage(pageToRemove).Root Cause
TestNavigationHandlersimulates async navigation with a 10ms delayRemovePageAppearing/Disappearingevent handlers would throw exceptions that weren't caught by the test frameworkChanges
navdeclaration fromNavigationPagetovarto preserve theTestNavigationPagetypeawait nav.NavigatingTask;afterRemovePagecall to ensure the test waits for navigation to completeThis matches the pattern used in the
RemoveLastPagetest in the same file (line 122).Testing
API Changes
None
Behavioral Changes
None - this only fixes test infrastructure
PR Checklist