-
Notifications
You must be signed in to change notification settings - Fork 3.3k
[various] Scrubs pre-SDK-21 Android code #9112
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
[various] Scrubs pre-SDK-21 Android code #9112
Conversation
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.
approve % comment about @config with minsdk set.
|
||
@RunWith(RobolectricTestRunner.class) | ||
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP) |
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 the @config annotation should stay and you should use the new minimum.
minSdk= is supposed to run the tests for every version of android after that version.
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.
Maybe I misunderstood @Config
. Does it restrict the APIs that this test can be run under, or does it cause the test to be run with multiple API levels? I thought it was the former.
If it's the latter, I'm skeptical we want this. We don't do this pretty much anywhere else in the repo, and I can't think of any reason we would want to multi-run things for the maps SDK in particular, since the SDK isn't system-API-level based.
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.
My understanding is it is the latter. Here is some documentation that I think supports that conclusion. https://robolectric.org/configuring/#configure-sdk-level. If you wanted to run on a specific set then you would use sdk = .
Looking through ConvertTest I agree but there are other places in packages where it does make sense to run with a minSdk or other broader android versions set. Primarily to make sure branching logic based on android api gets exercised.
Update: a spot check of all of these google maps test entries shows minimal value in multi api level tests because they do not branch.
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'll go ahead and remove them then; the fact that we only do this for these tests, and these tests don't seem like they benefit, makes me think that adding it was a result of a misunderstanding like mine.
|
||
@RunWith(RobolectricTestRunner.class) | ||
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP) |
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.
same here.
|
||
@RunWith(RobolectricTestRunner.class) | ||
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP) |
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.
and here. Not marking any more but if we agree on @config staying then update all instances.
Version override: updating the min version numbers in the app-facing packages is extremely low priority; it can go out in the next normally-shipped version, since delaying it arbitrarily doesn't matter (some are already years out of date). |
flutter/packages@2fcc403...ac21f53 2025-04-20 engine-flutter-autoroll@skia.org Roll Flutter from 3ed38e2 to cfb887c (17 revisions) (flutter/packages#9118) 2025-04-19 stuartmorgan@google.com [various] Scrubs pre-SDK-21 Android code (flutter/packages#9112) 2025-04-18 engine-flutter-autoroll@skia.org Roll Flutter from ecabb1a to 3ed38e2 (23 revisions) (flutter/packages#9114) 2025-04-18 ricardodalarme@outlook.com [flutter_svg] feat: Expose the `colorMapper` property in `SvgPicture` (flutter/packages#9043) 2025-04-18 stuartmorgan@google.com [tool] Add initial file-based command skipping (flutter/packages#8928) 2025-04-18 stuartmorgan@google.com [pigeon] Convert test plugins to SPM (flutter/packages#9105) 2025-04-18 10687576+bparrishMines@users.noreply.github.com [webview_flutter] Adds support to control overscrolling (flutter/packages#8451) 2025-04-17 louisehsu@google.com [in_app_purchase] add Storefront.countryCode() and AppStore.sync() (flutter/packages#8900) 2025-04-17 145144088+camfrandsen@users.noreply.github.com [webview_flutter_wkwebview] Expose the allowsLinkPreview property in WKWebView for iOS (flutter/packages#5029) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations to set over-scroll mode (flutter/packages#9101) 2025-04-17 1063596+reidbaker@users.noreply.github.com [shared_preferences] Update AGP to 8.9.1 (flutter/packages#9106) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [pigeon] Adds Kotlin lint tests to example code and fix lints (flutter/packages#9034) 2025-04-17 30872003+misos1@users.noreply.github.com [video_player_avfoundation] enable more than 30 fps (flutter/packages#7466) 2025-04-17 engine-flutter-autoroll@skia.org Roll Flutter from aef4718 to ecabb1a (25 revisions) (flutter/packages#9104) 2025-04-16 stuartmorgan@google.com [pigeon] Unify iOS and macOS test plugins (flutter/packages#9100) 2025-04-16 engine-flutter-autoroll@skia.org Roll Flutter from db68c95 to aef4718 (7 revisions) (flutter/packages#9098) 2025-04-16 10687576+bparrishMines@users.noreply.github.com [webview_flutter_platform_interface] Adds method to set overscroll mode (flutter/packages#9099) 2025-04-16 matanlurey@users.noreply.github.com Update `CODEOWNERS` (flutter/packages#8984) 2025-04-16 stuartmorgan@google.com [google_sign_is] Update iOS SDK to 8.0 (flutter/packages#9081) 2025-04-16 robert.odrowaz@leancode.pl [camera_avfoundation] Implementation swift migration (flutter/packages#8988) 2025-04-16 32538273+ValentinVignal@users.noreply.github.com [go_router] Adds `caseSensitive` to `GoRoute` (flutter/packages#8992) 2025-04-16 engine-flutter-autoroll@skia.org Manual roll Flutter from 30e53b0 to db68c95 (98 revisions) (flutter/packages#9092) 2025-04-15 stuartmorgan@google.com [tool] Run config-only build for iOS/macOS native-test (flutter/packages#9080) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
flutter/packages@2fcc403...ac21f53 2025-04-20 engine-flutter-autoroll@skia.org Roll Flutter from 409a8ac to cd51fa3 (17 revisions) (flutter/packages#9118) 2025-04-19 stuartmorgan@google.com [various] Scrubs pre-SDK-21 Android code (flutter/packages#9112) 2025-04-18 engine-flutter-autoroll@skia.org Roll Flutter from d0741df to 409a8ac (23 revisions) (flutter/packages#9114) 2025-04-18 ricardodalarme@outlook.com [flutter_svg] feat: Expose the `colorMapper` property in `SvgPicture` (flutter/packages#9043) 2025-04-18 stuartmorgan@google.com [tool] Add initial file-based command skipping (flutter/packages#8928) 2025-04-18 stuartmorgan@google.com [pigeon] Convert test plugins to SPM (flutter/packages#9105) 2025-04-18 10687576+bparrishMines@users.noreply.github.com [webview_flutter] Adds support to control overscrolling (flutter/packages#8451) 2025-04-17 louisehsu@google.com [in_app_purchase] add Storefront.countryCode() and AppStore.sync() (flutter/packages#8900) 2025-04-17 145144088+camfrandsen@users.noreply.github.com [webview_flutter_wkwebview] Expose the allowsLinkPreview property in WKWebView for iOS (flutter/packages#5029) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [webview_flutter_android][webview_flutter_wkwebview] Adds platform implementations to set over-scroll mode (flutter/packages#9101) 2025-04-17 1063596+reidbaker@users.noreply.github.com [shared_preferences] Update AGP to 8.9.1 (flutter/packages#9106) 2025-04-17 10687576+bparrishMines@users.noreply.github.com [pigeon] Adds Kotlin lint tests to example code and fix lints (flutter/packages#9034) 2025-04-17 30872003+misos1@users.noreply.github.com [video_player_avfoundation] enable more than 30 fps (flutter/packages#7466) 2025-04-17 engine-flutter-autoroll@skia.org Roll Flutter from 9616f9c to d0741df (25 revisions) (flutter/packages#9104) 2025-04-16 stuartmorgan@google.com [pigeon] Unify iOS and macOS test plugins (flutter/packages#9100) 2025-04-16 engine-flutter-autoroll@skia.org Roll Flutter from a7ce7ff to 9616f9c (7 revisions) (flutter/packages#9098) 2025-04-16 10687576+bparrishMines@users.noreply.github.com [webview_flutter_platform_interface] Adds method to set overscroll mode (flutter/packages#9099) 2025-04-16 matanlurey@users.noreply.github.com Update `CODEOWNERS` (flutter/packages#8984) 2025-04-16 stuartmorgan@google.com [google_sign_is] Update iOS SDK to 8.0 (flutter/packages#9081) 2025-04-16 robert.odrowaz@leancode.pl [camera_avfoundation] Implementation swift migration (flutter/packages#8988) 2025-04-16 32538273+ValentinVignal@users.noreply.github.com [go_router] Adds `caseSensitive` to `GoRoute` (flutter/packages#8992) 2025-04-16 engine-flutter-autoroll@skia.org Manual roll Flutter from f2d54fd to a7ce7ff (98 revisions) (flutter/packages#9092) 2025-04-15 stuartmorgan@google.com [tool] Run config-only build for iOS/macOS native-test (flutter/packages#9080) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC flutter-ecosystem@google.com on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
- Removes any annotations or runtime checks related to Android API 20 or 21, as 21 is the minimum API version supported by the versions of Flutter these plugins support. - Removes comment/README references to limitations for <21. - Updates READMEs for app-facing packages to reflect the current support levels. Fixes flutter/flutter#157106 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
- Removes any annotations or runtime checks related to Android API 20 or 21, as 21 is the minimum API version supported by the versions of Flutter these plugins support. - Removes comment/README references to limitations for <21. - Updates READMEs for app-facing packages to reflect the current support levels. Fixes flutter/flutter#157106 ## Pre-Review Checklist [^1]: Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling.
Fixes flutter/flutter#157106
Pre-Review Checklist
[shared_preferences]
pubspec.yaml
with an appropriate new version according to the pub versioning philosophy, or I have commented below to indicate which version change exemption this PR falls under1.CHANGELOG.md
to add a description of the change, following repository CHANGELOG style, or I have commented below to indicate which CHANGELOG exemption this PR falls under1.///
).Footnotes
Regular contributors who have demonstrated familiarity with the repository guidelines only need to comment if the PR is not auto-exempted by repo tooling. ↩ ↩2 ↩3