-
Notifications
You must be signed in to change notification settings - Fork 3k
feat: migrate onClick out of the layout files, add testing #2328
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
Conversation
This commit introduces a suite of Espresso tests for several demo activities and refactors numerous activities to use ViewBinding, eliminating `findViewById` and `onClick` XML attributes.
- **Testing Infrastructure:**
- Adds `MapIdlingResource` for both Java and Kotlin test sources to synchronize Espresso tests with map camera movements, preventing flaky tests.
- Introduces custom Truth subjects (`LatLngSubject`, `LatLngBoundsSubject`) for more readable and precise assertions on map-related objects, with built-in tolerance for floating-point comparisons.
- Adds new dependencies for testing: `espresso-idling-resource`, `truth`, and `uiautomator`.
- Increases Gradle daemon heap size to support test execution.
- **New Espresso Tests:**
- Adds comprehensive tests for `GroundOverlayDemoActivity`, `CameraClampingDemoActivity`, `IndoorDemoActivity`, `CameraDemoActivity`, and `VisibleRegionDemoActivity` in both Java and Kotlin modules.
- Tests verify UI interactions, camera behavior, and state changes within the demo activities.
- **Code Refactoring:**
- Enables ViewBinding in the `java-app`, `kotlin-app`, and `common-ui` modules.
- Refactors all major demo activities (e.g., `GroundOverlayDemoActivity`, `MarkerDemoActivity`, `CircleDemoActivity`, `UiSettingsDemoActivity`) to use ViewBinding.
- Replaces `android:onClick` XML attributes with programmatic `setOnClickListener` calls, improving code organization and maintainability.
- Enhances Javadoc and KDoc in `GroundOverlayDemoActivity` to better explain the concepts and implementation details, improving its educational value.
…Provider Refactors the GroundOverlayDemoActivityTest to improve reliability and readability. This commit introduces a `MapProvider` interface and a base `MapDemoActivityTest` class to abstract away common map initialization logic. Key changes include: - Add a `MapProvider` interface to ensure activities provide a `GoogleMap` object. - Create an abstract `MapDemoActivityTest` to handle common test setup, including map initialization and idling resources (WIP). - Update `GroundOverlayDemoActivityTest` to extend `MapDemoActivityTest`, simplifying its setup logic. - Replace `Thread.sleep()` calls with `idlingResource.waitForIdle()`. - Update `GroundOverlayDemoActivity` to use `lateinit` for overlays, preventing nullable types. - Add `maps-utils-ktx` dependency for idiomatic spherical offset calculations.
Enhances the `testLongClickAddsNewCircle` in `CircleDemoActivityTest` by adding detailed comments that explain the test's flow and the critical reason for camera movements: to minimize precision loss during LatLng to screen coordinate conversion for accurate click event simulation. This commit also includes the necessary `LatLngSubject` extension, introducing a `isWith(tolerance).of(target)` assertion. This new assertion allows for more precise and readable verification of spherical distances between `LatLng` objects, directly supporting the improved test logic in `CircleDemoActivityTest`.
Refactors the java version of `GroundOverlayDemoActivityTest` to extend the common `MapDemoActivityTest` base class. This change centralizes test setup, teardown, and map interaction logic, removing redundant code and improving maintainability. - `GroundOverlayDemoActivity` now implements the `MapProvider` interface to correctly expose the map instance and its ready state to the test framework. - Replaces `Thread.sleep()` calls in `CircleDemoActivityTest` and `GroundOverlayDemoActivityTest` with `idlingResource.waitForIdle()`. This creates more reliable tests by waiting for map events to complete rather than using fixed delays. - Adds a `waitForIdle(long)` overload to `MapIdlingResource` to handle cases requiring a brief, controlled pause after the map has settled.
This commit applies several small refactorings to the Kotlin demo activities, focusing on code modernization and resolving lint issues.
- In `MarkerDemoActivity`, replaces `Math.max()` with the more idiomatic Kotlin function `coerceAtLeast()`.
- Simplifies trigonometric function calls by using top-level `sin` and `cos` from the Kotlin standard library instead of `Math`.
- Removes unused imports from `MarkerDemoActivity`.
- In `LayersDemoActivity`, adds `@SuppressLint("MissingPermission")` to the `onMyLocationToggled` method to address a lint warning.
This commit updates the layout files to use the `android:name` attribute instead of the `class` attribute for `FragmentContainerView` to declare the fragment class. This is the modern, recommended practice. It also removes minor whitespace and `onClick` attributes that are no longer necessary. - Replaces the `class` attribute with `android:name` for all instances of `SupportMapFragment` and `SupportStreetViewPanoramaFragment` within `FragmentContainerView` tags across multiple demo layouts. - Removes unnecessary `onClick` attributes from checkboxes in `layers_demo.xml`. - Cleans up minor whitespace in `camera_clamping_demo.xml`, `street_view_panorama_navigation_demo.xml`, `indoor_demo.xml`, and `ground_overlay_demo.xml`.
| @RunWith(AndroidJUnit4.class) | ||
| public class CircleDemoActivityTest extends MapDemoActivityTest<CircleDemoActivity> { | ||
|
|
||
| private static final LatLng HERE_BE_DRAGONS = new LatLng(-34.91459615762562, 138.60572619793246); |
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.
:)
| @Test | ||
| public void testVisibleLevelInfo() { | ||
| try { | ||
| Thread.sleep(200); |
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.
This is a bit of an anti-pattern, but even using the IdlingResource, if the Thread.sleep() gets removed doesn't always work. I don't think there is a better solution.
| private float mMaxZoom; | ||
|
|
||
| private TextView mCameraTextView; | ||
| private com.example.common_ui.databinding.CameraClampingDemoBinding binding; |
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.
We can probably move this into an import.
|
One small comment regarding some classes on the PR: we can probably use imports instead of FQDN when importing a class or attribute. |
|
Looks good. We were discussing whether it could be worth moving some components to a common module, but for now we can merge this PR. |
# [1.19.0](v1.18.0...v1.19.0) (2025-11-06) ### Features * migrate onClick out of the layout files, add testing ([#2328](#2328)) ([dfa18d2](dfa18d2))
|
🎉 This PR is included in version 1.19.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
This pull request represents the initial phase of a broader migration effort to move
onClickhandlers out of XML layout files and into the corresponding Activity/Fragment code. This change aims to improve maintainability, testability, and adherence to modern Android development practices.Crucially, this pull request also introduces testing to the project.
Key changes in this phase include:
GroundOverlayDemoActivityTesthas been refactored to extend the commonMapDemoActivityTestbase class, centralizing test setup, teardown, and map interaction logic.Thread.sleep()calls inCircleDemoActivityTestandGroundOverlayDemoActivityTestwithidlingResource.waitForIdle()for more reliable and robust test execution.GroundOverlayDemoActivitynow implements theMapProviderinterface to correctly expose the map instance and its ready state to the test framework.CircleDemoActivityTesthas been improved with detailed comments explaining test flow and the rationale behind camera movements for accurate click event simulation.LatLngSubjectextension with anisWith(tolerance).of(target)assertion for more precise and readable verification of spherical distances in tests.This is an ongoing effort, and further work will continue in subsequent pull requests to complete the
onClickmigration and expand test coverage across the application.