-
Notifications
You must be signed in to change notification settings - Fork 3.6k
[ci] Add emulator tests on older Android version #4616
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
Merged
auto-submit
merged 8 commits into
flutter:main
from
stuartmorgan-g:android-legacy-emulator
Oct 19, 2023
Merged
Changes from 6 commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
39a6b0f
Test 21 with existing tasks
stuartmorgan-g e62bc42
Try 22, which should be available
stuartmorgan-g 2701cf2
Merge branch 'main' into android-legacy-emulator
stuartmorgan-g a5c1a48
Add a new exclusion file and test file
stuartmorgan-g 3b8c115
More exclusions
stuartmorgan-g a7f5cbe
Add as actual new tasks
stuartmorgan-g 4c5de9b
Typo fix
stuartmorgan-g 997e2a9
Move emulator definition to new base config
stuartmorgan-g File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| tasks: | ||
| - name: prepare tool | ||
| script: .ci/scripts/prepare_tool.sh | ||
| infra_step: true # Note infra steps failing prevents "always" from running. | ||
| - name: download Dart and Android deps | ||
| script: script/tool_runner.sh | ||
| infra_step: true | ||
| args: ["fetch-deps", "--android", "--supporting-target-platforms-only", "--exclude=script/configs/exclude_integration_android_legacy_emulator.yaml"] | ||
| # Only these two tests are run because they are the only ones that use the | ||
| # emulator. Other tests would just duplicate android_platform_test.yaml tests. | ||
| - name: native integration tests | ||
| script: script/tool_runner.sh | ||
| args: ["native-test", "--android", "--no-unit", "--exclude=script/configs/exclude_integration_android_legacy_emulator.yaml"] | ||
| - name: drive examples | ||
| script: script/tool_runner.sh | ||
| args: ["drive-examples", "--android", "--exclude=script/configs/exclude_integration_android_legacy_emulator.yaml,script/configs/exclude_integration_android.yaml,script/configs/exclude_integration_android_emulator.yaml"] |
13 changes: 13 additions & 0 deletions
13
script/configs/exclude_integration_android_legacy_emulator.yaml
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
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| # Tihs plugin no-ops below API 25, which is newer than the legacy emulator. | ||
stuartmorgan-g marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| # This can be removed once the legacy emulator test is using at least | ||
| # API level 25. | ||
| - quick_actions | ||
| - quick_actions_android | ||
| # Fails on the legacy emulator. See https://github.com/flutter/flutter/issues/136823 | ||
| # TODO(stuartmorgan): Remove this if the limitation can be worked around. | ||
| - video_player | ||
| - video_player_android | ||
| # Hangs on the legacy emulator. See https://github.com/flutter/flutter/issues/136824 | ||
| # TODO(stuartmorgan): Remove this if the hang can be fixed. | ||
| - webview_flutter | ||
| - webview_flutter_android | ||
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.
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.
Out of scope for this pr but I wish we could use variables for these shards since they cant actually have different dependencies.
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.
I could make a new config for legacy emulator Android tests, but without a way to have one
platform_propertiesentry inherit from another one that sort of just moves the problem (since I'd have to instead duplicate all the core Android dependencies, which can't really be different either). Would you prefer that? I can see an argument for it being less error-prone.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.
I think it is out of scope for this change but anyplace we can make it clear in code that dependencies that have to be the same are the same I think it is a win for clarity and maintainability.
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.
Since this is a whole new set of shards with configs, it seems in scope to have it start the way you'd prefer it; I'll leave the existing ones alone for now though (especially so I don't mess up Camille's WIP PR).
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.
Let me know what you think of this updated version :)