[google_maps_android_flutter] Convert Config.sdk to minSdk in Robolectric tests and lower to LOLLIPOP#7805
Conversation
|
https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version states |
yaakovschectman
added
the
override: no versioning needed
stuartmorgan-g
added
the
override: no changelog needed
| outputs.upToDateWhen {false} | ||
| showStandardStreams = true | ||
| } | ||
| jvmArgs "-Xmx1g" |
There was a problem hiding this comment.
Good find but instead of setting the jvm args in build.gradle set them in gradle.properties like we do in the rest of the places we bump memory.
For this pr I think you need to create a gradle.properties and put this value there.
Here is an example from where daco bumped the memory used in the templates. https://github.com/flutter/flutter/pull/156201/files
There was a problem hiding this comment.
For whatever it's worth, this approach is what the other packages, or at least those I looked at, do, and is how I found this fix initially.
There was a problem hiding this comment.
https://github.com/search?q=repo%3Aflutter%2Fpackages%20jvmArgs%20%22-Xmx1g%22&type=code
I see one example. Can you modify camera and this pr to use gradle.properties?
There was a problem hiding this comment.
The reasoning is that keeping track of memory allocation is hard enough as a class of problem and having 2 different ways we do it makes it even harder. Especially because properties clober each other so any jvm args set in one place will be overridden in the second place.
There was a problem hiding this comment.
I may be misunderstanding the actual use of this. Creating gradle.properties in the same path as this build.gradle file and populating it with org.gradle.jvmargs=-Xmx1G (and removing the added line from build.gradle) reintroduces the OOM that was the issue in the first place. And trying instead to set the property android.testOptions.unitTests.all.jvmArgs in gradle.properties also does not resolve the OOM error, either. It at least appears that whatever is set in gradle.properties is not impacting the android unit tests the way build.gradle does.
There was a problem hiding this comment.
I've added a comment explaining this. Please see if it warrants an LGTM or further requests.
reidbaker
left a comment
reidbaker
left a comment
There was a problem hiding this comment.
Thanks for finding this! Optionally can you add the same comment to camera where you found the first example.
…` in Robolectric tests and lower to `LOLLIPOP` (flutter/packages#7805)
…` in Robolectric tests and lower to `LOLLIPOP` (flutter/packages#7805)
flutter/packages@2a1c477...b6f7e47 2024-10-21 49699333+dependabot[bot]@users.noreply.github.com [path_provider]: Bump androidx.annotation:annotation from 1.8.2 to 1.9.0 in /packages/path_provider/path_provider_android/android (flutter/packages#7895) 2024-10-18 stuartmorgan@google.com [in_app_purchase] Update iOS Pigeon for non-nullable generics (flutter/packages#7820) 2024-10-18 10687576+bparrishMines@users.noreply.github.com [interactive_media_ads] Adds internal wrapper for Android native `Ad` (flutter/packages#7880) 2024-10-18 stuartmorgan@google.com [video_player] Remove Android API 19 SSL handling (flutter/packages#7876) 2024-10-18 109111084+yaakovschectman@users.noreply.github.com [google_maps_android_flutter] Convert `Config.sdk` to `minSdk` in Robolectric tests and lower to `LOLLIPOP` (flutter/packages#7805) 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
3 participants
Tests in the title package that are run using the Robolectric test runner will now run on all SDKs level >= LOLLIPOP (i.e. 21). Raises max heap size to 1G.
Affects only tests, should be version-bump exempt.
Fixes flutter/flutter#152931
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.