-
Notifications
You must be signed in to change notification settings - Fork 29.7k
Remove timeout from add2app test for iOS #28746
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
Conversation
|
Upstream change has landed, will just need to get it rolled into framework |
|
Once #28688 lands this can be rebased and should work. |
xster
left a comment
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.
LGTM
|
|
||
| NS_ASSUME_NONNULL_BEGIN | ||
|
|
||
| @interface DualFlutterViewController : UIViewController |
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.
Semi unrelated but just for easier maintenance in the future
Can you, from README.md, call out the related filenames of the tests for each entry so we can associate which test is done in which file?
| [[NSNotificationCenter defaultCenter] removeObserver:observer]; | ||
| }]; | ||
| [viewController.engine ensureSemanticsEnabled]; | ||
| while (semanticsAvailable == NO && tries != 0) { |
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.
Add one line saying what is synchronous and what isn't in a comment here and in the ensureSemanticsEnabled objective-c doc
Description
Fixes the flake in ios_add2app and removes the timeout. We still have retry logic in here - eventually, EarlGrey will be able to do this without us needing this logic.
This PR will fail until the related engine PR lands in the framework.
Related Issues
flutter/engine#8005
#24519 (related, but for something slightly different).
Checklist
Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes (
[x]). This will ensure a smooth and quick review process.///).flutter analyze --flutter-repo) does not report any problems on my PR.Breaking Change
Does your PR require Flutter developers to manually update their apps to accommodate your change?