skip interactive keyboard tests#183757
Conversation
github-actions
bot
added
a: text input
![gemini-code-assist[bot]](proxy.php?url=https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to skip several interactive keyboard tests on iOS versions 26 and above due to SDK bugs. However, there appears to be a typo in the specified iOS version (26.0), which will prevent the tests from being skipped as intended. Additionally, one test case is removed entirely, while others are conditionally skipped, leading to an inconsistency that should be addressed.
I am having trouble creating individual review comments. Click here to see my feedback.
engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm (3464-3467)
The version iOS 26.0 appears to be a typo, as this iOS version has not been released. As a result, this condition will never be met, and the test will not be skipped as intended. Please verify the correct iOS version (e.g., iOS 16.0 or iOS 17.0).
This check is duplicated across several tests. To improve maintainability, consider extracting this logic into a helper macro. This would centralize the version check and skip message, making future updates easier.
For example:
#define SKIP_INTERACTIVE_KEYBOARD_TEST_IF_NEEDED() \
if (@available(iOS 17.0, *)) { /* Or the correct version */ \
XCTSkip(@"Interactive keyboard tests broken on this iOS version due to SDK bugs. See: " \
"https://github.com/flutter/flutter/issues/183473"); \
}
// Usage in test:
- (void)testInteractiveKeyboardAfterUserScrollWillResignFirstResponder {
SKIP_INTERACTIVE_KEYBOARD_TEST_IF_NEEDED();
// ...
}engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterTextInputPluginTest.mm (3487-3528)
This test, testInteractiveKeyboardAfterUserScrollToTopOfKeyboardWillTakeScreenshot, is being removed entirely, while other related tests are being conditionally skipped. For consistency and to preserve the test for older iOS versions where it might still be valid, please consider conditionally skipping this test instead of deleting it.
| - (void)testInteractiveKeyboardAfterUserScrollWillResignFirstResponder { | ||
| if (@available(iOS 26.0, *)) { | ||
| XCTSkip(@"Interactive keyboard tests broken on iOS 26+ due to SDK bugs. See: " | ||
| "https://github.com/flutter/flutter/issues/183473"); |
There was a problem hiding this comment.
Let's remove it for iOS 17+ (as explained in the link above)
There was a problem hiding this comment.
why is this removed instead of skipped? (like in the other tests)
2 participants
Skip Interactive keyboard tests that no longer apply to iOS 26+
Fixes #183407
Tracking #183473 for full logic refactoring
If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.
Note: The Flutter team is currently trialing the use of Gemini Code Assist for GitHub. Comments from the
gemini-code-assistbot should not be taken as authoritative feedback from the Flutter team. If you find its comments useful you can update your code accordingly, but if you are unsure or disagree with the feedback, please feel free to wait for a Flutter team member's review for guidance on which automated comments should be addressed.