-
-
Notifications
You must be signed in to change notification settings - Fork 973
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
ci(android_alarm_manager_plus): Add matrix of versions for integration tests #1184
ci(android_alarm_manager_plus): Add matrix of versions for integration tests #1184
Conversation
# Required for Android builds | ||
- uses: actions/setup-java@v1 | ||
# Required for Android builds | ||
- uses: actions/setup-java@v2 |
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 experimented with removing this step as suggested in one of other PRs (because of https://github.com/actions/runner-images/blob/main/images/macos/macos-12-Readme.md#java saying that such runners already have Java 11), but couldn't build the project due to missing Java 11. Thus, returned the step again.
with: | ||
distribution: "temurin" |
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.
setup-java in v2 requires to specify the distribution type, thus, this addition.
api-level: 30 | ||
arch: x86_64 | ||
api-level: ${{ matrix.android-api-level }} | ||
cores: 3 |
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.
MacOS
runner has 3 cores available, while android-emulator-runner
uses 2 by default. Specifying this value in effort to get higher performance.
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, let's see if the CI gods keep everything green ;)
strategy: | ||
matrix: | ||
android-api-level: [21, 26, 32] |
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.
Ohh that's cool, simpler than what I thought.
Description
Introduced build matrix so tests run against multiple Android versions. It should help us to be sure that functionality of the plugin isn't broken by some change.
This PR has matrix only for Android alarm manager plus to get initial feedback. If other maintainers agree with suggested strategies in the next PR I will add similar changes for all plugins at once.
Initially I wanted to have 19 as a minimal version, but on 19 I saw such error:
So it seems that
integration_test
doesn't work well with old APIs.Mid version corresponds to Android O, which is something in the middle of the suggested range.
Max version is 32 as 33 acted strange during my tests on fork.
Checklist
CHANGELOG.md
nor thepubspec.yaml
files.flutter analyze
) does not report any problems on my PR.Breaking Change
Does your PR require plugin users to manually update their apps to accommodate your change?
!
in the title as explained in Conventional Commits).