Skip to content

[google_maps_android_flutter] Convert Config.sdk to minSdk in Robolectric tests and lower to LOLLIPOP #7805

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.

Sign up for GitHub

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
yaakovschectman merged 6 commits into from
Oct 18, 2024
Merged

[google_maps_android_flutter] Convert Config.sdk to minSdk in Robolectric tests and lower to LOLLIPOP #7805

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9b238b3
Hold this for me
yaakovschectman Sep 30, 2024
087b113
Found it
yaakovschectman Oct 4, 2024
badb99d
Lower minSdk to LOLLIPOP
yaakovschectman Oct 7, 2024
be087f6
Remove test test
yaakovschectman Oct 7, 2024
d21d81b
Comment build.gradle
yaakovschectman Oct 17, 2024
2942b37
Comment for camera_android
yaakovschectman Oct 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/camera/camera_android/android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,10 @@ buildFeatures {
unitTests.includeAndroidResources = true
unitTests.returnDefaultValues = true
unitTests.all {
// The org.gradle.jvmargs property that may be set in gradle.properties does not impact
// the Java heap size when running the Android unit tests. The following property here
// sets the heap size to a size large enough to run the robolectric tests across
// multiple SDK levels.
jvmArgs "-Xmx1g"
testLogging {
events "passed", "skipped", "failed", "standardOut", "standardError"
Expand Down
5 changes: 5 additions & 0 deletions packages/google_maps_flutter/google_maps_flutter_android/android/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,11 @@ android {
outputs.upToDateWhen {false}
showStandardStreams = true
}
// The org.gradle.jvmargs property that may be set in gradle.properties does not impact
// the Java heap size when running the Android unit tests. The following property here
// sets the heap size to a size large enough to run the robolectric tests across
// multiple SDK levels.
jvmArgs "-Xmx1g"
Copy link
Contributor

@reidbaker reidbaker Oct 7, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

@yaakovschectman yaakovschectman Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor

@reidbaker reidbaker Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Contributor

@reidbaker reidbaker Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@yaakovschectman yaakovschectman Oct 7, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Contributor Author

@yaakovschectman yaakovschectman Oct 17, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added a comment explaining this. Please see if it warrants an LGTM or further requests.

}
}
}
2 changes: 1 addition & 1 deletion ...id/android/src/test/java/io/flutter/plugins/googlemaps/ClusterManagersControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = Build.VERSION_CODES.P)
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP)
public class ClusterManagersControllerTest {
private Context context;
private MapsCallbackApi flutterApi;
Expand Down
2 changes: 1 addition & 1 deletion ...maps_flutter_android/android/src/test/java/io/flutter/plugins/googlemaps/ConvertTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(minSdk = Build.VERSION_CODES.P)
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP)
public class ConvertTest {
@Mock private AssetManager assetManager;

Expand Down
2 changes: 1 addition & 1 deletion ..._android/android/src/test/java/io/flutter/plugins/googlemaps/GoogleMapControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = Build.VERSION_CODES.P)
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP)
public class GoogleMapControllerTest {

private Context context;
Expand Down
2 changes: 1 addition & 1 deletion ...android/android/src/test/java/io/flutter/plugins/googlemaps/GoogleMapInitializerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = Build.VERSION_CODES.P)
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP)
public class GoogleMapInitializerTest {
private GoogleMapInitializer googleMapInitializer;

Expand Down
2 changes: 1 addition & 1 deletion ...r_android/android/src/test/java/io/flutter/plugins/googlemaps/HeatmapsControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = Build.VERSION_CODES.P)
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP)
public class HeatmapsControllerTest {
private HeatmapsController controller;
private GoogleMap googleMap;
Expand Down
2 changes: 1 addition & 1 deletion ...er_android/android/src/test/java/io/flutter/plugins/googlemaps/MarkersControllerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@
import org.robolectric.annotation.Config;

@RunWith(RobolectricTestRunner.class)
@Config(sdk = Build.VERSION_CODES.P)
@Config(minSdk = Build.VERSION_CODES.LOLLIPOP)
public class MarkersControllerTest {
private Context context;
private MapsCallbackApi flutterApi;
Expand Down