Migrate flavors_test_ios to Simulators and add UIScene support - #189442
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates several iOS device lab targets in .ci.yaml to run on arm64 hosts, and refactors flavors_test_ios.dart and related integration test utilities to run on a newly created iOS simulator using a provided device ID. It also adds scene manifest configurations to the iOS plist files. A review comment points out that the manual simulator cleanup in flavors_test_ios.dart is redundant and will cause failures because testWithNewIOSSimulator already manages the simulator lifecycle.
There was a problem hiding this comment.
Code Review
This pull request updates several iOS devicelab tasks to run on a simulator instead of a physical device, introducing a deviceIdOverride parameter to target specific simulator instances, and configures UIApplicationSceneManifest in the plist files. Feedback highlights two issues: first, wrapping testWithNewIOSSimulator in an outer try-finally block to delete the simulator is redundant and will cause errors; second, when deviceIdOverride is provided, selectedDevice remains null, which will break the withTalkBack check.
|
This is a prototype PR to propose and discuss the architectural strategy for moving iOS UI/Functional tests off physical lab devices and onto Mac VM simulators (resolving #186414) before applying the pattern broadly. The Approach: Why keep them in Devicelab instead of rewriting them?
Trade-offs:
|
|
Here is the comprehensive list of Devicelab targets I plan to migrate using this strategy. Compile-Only TasksThese tasks only verify that an iOS project can successfully build (running
Functional / UI TestsThese tasks verify UI layout, platform channels, and debugging workflows. While they do require launching an app, they do not measure raw device performance (unlike our benchmarking tasks, which must remain on physical hardware to prevent skewed metrics). We will migrate these by injecting the
|
vashworth
left a comment
vashworth
left a comment
There was a problem hiding this comment.
LGTM after one request
Can you move this comment to the tracking issue? |
| task_name: route_test_ios | ||
|
|
||
| - name: Mac_ios flavors_test_ios | ||
| - name: Mac_arm64 flavors_test_ios |
There was a problem hiding this comment.
| - name: Mac_arm64 flavors_test_ios | |
| - name: Mac flavors_test_ios |
I don't think there's a reason this needs to run exclusively on arm
There was a problem hiding this comment.
not wrong, I had added it because we are deprecating intel(not that it affects per se) and also intel is slower
okorohelijah
added
the
autosubmit
auto-submit
Bot
removed
the
autosubmit
|
autosubmit label was removed for flutter/flutter/189442, because - The status or check suite Dashboard Checks has failed. Please fix the issues identified (or deflake) before re-applying this label. |
okorohelijah
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
…r#190775) This PR adds the missing `ruby` (CocoaPods) dependency to the `flavors_test_ios` target in `.ci.yaml`. Following the migration of this test from the physical device pool (`Mac_ios`) to the generic `Mac` simulator pool (in PR flutter#189442), the target began failing in post-submit bringup with the [error `CocoaPods not installed or not in valid state`](https://ci.chromium.org/ui/p/flutter/builders/staging/Mac%20flavors_test_ios/1/overview). Because the dummy `flavors` app uses the `integration_test` package and contains an `ios/Podfile`, it requires CocoaPods to compile. Generic Mac CI bots do not have CocoaPods installed globally, so it must be explicitly requested in the LUCI `.ci.yaml` dependencies list. *List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.* Fixes flutter#189442 *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 - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
Flavors test has been passing, remove from bringup Fixes flutter#189442 *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 - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [AI contribution guidelines] and understand my responsibilities, or I am not using AI tools. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement]. - [x] I signed the [CLA]. - [x] I listed at least one issue that this PR fixes in the description above. - [ ] I updated/added relevant documentation (doc comments with `///`). - [ ] I added new tests to check the change I am making, or this PR is [test-exempt]. - [ ] I followed the [breaking change policy] and added [Data Driven Fixes] where supported. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. If this change needs to override an active code freeze, provide a comment explaining why. The code freeze workflow can be overridden by code reviewers. See pinned issues for any active code freezes with guidance. **Note**: The Flutter team is currently trialing the use of [Gemini Code Assist for GitHub](https://developers.google.com/gemini-code-assist/docs/review-github-code). Comments from the `gemini-code-assist` bot 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. <!-- Links --> [Contributor Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview [AI contribution guidelines]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#ai-contribution-guidelines [Tree Hygiene]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md [test-exempt]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests [Flutter Style Guide]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md [Features we expect every widget to implement]: https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md [Data Driven Fixes]: https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
2 participants
Part of #186414. This PR introduces the
testWithNewIOSSimulatorhelper pattern to execute theflavors_test_ioson macOS VMs rather than physical lab devices. This also migrates the old flavors test app to adopt the UIScene lifecycle usingFlutterSceneDelegate.