Skip to content

feat: modernize Kotlin snippets module with capabilities and visual test suites - #2427

Open
dkhawk wants to merge 3 commits into
feat/snippets-java-appfrom
feat/snippets-kotlin-app
Open

dkhawk wants to merge 3 commits into
feat/snippets-java-appfrom
feat/snippets-kotlin-app

Conversation

@dkhawk

@dkhawk dkhawk commented Sep 15, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Create :snippets:kotlin-app with 14 snippet categories, KTX extensions, and documentation region tags
  • Add Kotlin snippet infrastructure (KotlinSnippetsActivity, MapActivity, SnippetRegistry, TrackedMap)
  • Add Kotlin capabilities test suite (CatalogCapabilitiesTestSuite, CameraControl, Events, MapInit, Marker)
  • Add Kotlin visual test suite (BaseVisualTest, VisualTests)
  • Remove legacy Kotlin snippet modules (snippets/app-ktx, snippets/app-utils-ktx)
  • Update root settings.gradle.kts

Stacked Base

Stacked on #2426 (feat/snippets-java-app).

Reviewers

@kikoso

@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from 3eb7aaa to 8c6b89b Compare September 15, 2026 00:25
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from 8c6b89b to 293149c Compare September 15, 2026 00:35
@dkhawk
dkhawk marked this pull request as ready for review September 15, 2026 00:38
@dkhawk
dkhawk requested a review from kikoso September 15, 2026 00:38
@snippet-bot

snippet-bot Bot commented Sep 15, 2026 •

Copy link
Copy Markdown

Here is the summary of changes.

You are about to add 131 region tags.
You are about to delete 40 region tags.

This comment is generated by snippet-bot.
If you find problems with this result, please file an issue at:
https://github.com/googleapis/repo-automation-bots/issues.
To update this comment, add snippet-bot:force-run label or use the checkbox below:

  • Refresh this comment

@dkhawk
dkhawk added this pull request to stack #2429 September 15, 2026 00:43
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from 293149c to 00469c2 Compare September 15, 2026 18:22
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from 00469c2 to 37ae58f Compare September 15, 2026 22:54

@github-advanced-security github-advanced-security AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Android Lint found more than 20 potential problems in the proposed changes. Check the Files changed tab for more details.

layer.addLayerToMap()
// [END maps_android_utils_kml_add_layer]

// [START maps_android_utils_kml_remove_layer]

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Same class of issue I flagged on #2426, smaller in scope here. This PR drops 3 devsite region tags from the repo entirely:

tag was in
maps_android_utils_kml_remove_layer this file, line 52
maps_android_ktx_install_snippet snippets/app-ktx/build.gradle.kts:74
maps_android_utils_ktx_install_snippet snippets/app-utils-ktx/build.gradle.kts:73

I verified by diffing every [START ...] tag between feat/snippets-java-app and feat/snippets-kotlin-app, then grepping each missing one across the whole tree to rule out relocation.

The two *_install_snippet tags are the Gradle dependency snippets for the KTX libraries, which are exactly the sort of thing an installation page includes by tag. Worth noting that snippets/app-ktx/build.gradle.kts is detected as a rename to snippets/kotlin-app/build.gradle.kts (R059), so the file survives but the tag inside it does not.

Could you carry these three across to the new module, or confirm they are safe to retire?

Separately, nice work on the androidTest suites here. SnippetDiscoveryTest plus the per-capability tests is a good pattern, and it's a real step up from what the legacy modules had.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Addressed in commit bc534e92. All 3 dropped devsite region tags have been restored and verified:

  • maps_android_ktx_install_snippet and maps_android_utils_ktx_install_snippet have been carried over to snippets/kotlin-app/build.gradle.kts.
  • maps_android_utils_kml_remove_layer has been added to UtilsSnippets.kt (removeKmlLayer).
  • Also forwarded missing GoogleMap methods on TrackedMap.kt and eliminated .delegate calls across snippet tags, and sanitized API key logging in MapActivity.kt.

@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from 98c117d to bc534e9 Compare September 29, 2026 23:22
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from bc534e9 to 25476e6 Compare September 30, 2026 03:11
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch 2 times, most recently from 8aace02 to 65c7503 Compare September 30, 2026 22:06
@dkhawk
dkhawk requested review from LoyalAbbas and kikoso September 30, 2026 23:14
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from 65c7503 to d3c82f0 Compare September 30, 2026 23:20
…est suites

- Create :snippets:kotlin-app with 14 snippet categories, KTX extensions, and documentation region tags
- Add Kotlin snippet infrastructure (KotlinSnippetsActivity, MapActivity, SnippetRegistry, TrackedMap)
- Add Kotlin capabilities test suite (CatalogCapabilitiesTestSuite, CameraControl, Events, MapInit, Marker)
- Add Kotlin visual test suite (BaseVisualTest, VisualTests)
- Remove legacy Kotlin snippet modules (snippets/app-ktx, snippets/app-utils-ktx)
- Update root settings.gradle.kts
@dkhawk
dkhawk force-pushed the feat/snippets-kotlin-app branch from d3c82f0 to 5ecabb3 Compare September 30, 2026 23:46

// Initialize the manager with the context and the map.
// (Activity extends context, so we can pass 'this' in the constructor.)
val manager = ClusterManager<MyItem>(context, map.delegate)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

TrackedMap.delegate & test-harness casts are still inside active DevSite region tags Because ClusterManager, GeoJsonLayer, KmlLayer, and the *Manager classes require a real GoogleMap instance, 12 lines inside 7 active [START ...] blocks in UtilsSnippets.kt (L96, L233, L243, L341, L354, L512–530) still pass map.delegate—and EventsSnippets.kt:L42–47 / DatasetLayerSnippets.kt:L125–146 include internal MapActivity casts and R.id.custom_controls_container wiring inside published tags. Root cause: map is typed as TrackedMap rather than GoogleMap, so anything inside [START ...] that needs GoogleMap leaks .delegate verbatim onto developers.google.com, which will not compile when developers copy-paste it. Fix: Either alias val map: GoogleMap = this.map.delegate outside the [START ...] tag (like KtxSnippets.kt:L71 does) or wrap internal harness setup in // [START_EXCLUDE silent] ... // [END_EXCLUDE].

groupTitle = groupAnnotation.title,
action = { context, map, scope ->
try {
val trackedMap = TrackedMap(map, addedElements)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

this section silently swallows all snippet crashes and SnippetDiscoveryTest closes before onMapReady runs Unlike origin/feat/snippets-java-app (where SnippetRegistry.java:L116–124 was updated to unwrap InvocationTargetException and rethrow RuntimeException), SnippetRegistry.kt:L125–127 still does catch (e: Exception) { e.printStackTrace() }, making MapActivity.kt:L297 dead code. Root cause: Reflection exceptions are swallowed inside SnippetRegistry, and SnippetDiscoveryTest.verifyAllSnippetsLaunchWithoutCrash() (L102–112) only checks activity.mapView != null in onCreate before closing ActivityScenario.use { ... }—destroying the Activity before getMapAsync / onMapReady ever executes snippet.action. Fix: Unwrap InvocationTargetException and rethrow in SnippetRegistry.kt (matching SnippetRegistry.java), and await onMapReady / snippet execution in SnippetDiscoveryTest.kt before closing the scenario.

@LoyalAbbas

Copy link
Copy Markdown

Missing MapsObject.kt drops the Kotlin counterparts for 4 getting-started DevSite region tags In origin/feat/snippets-java-app, MapsObject.java was added to :snippets:java-app to preserve maps_android_on_create_set_content_view, maps_android_on_map_ready_callback, maps_android_on_map_ready_add_marker, and maps_android_get_map_async, but its Kotlin counterpart (MapsObject.kt from snippets/app/.../kotlin/MapsObject.kt on main) was never ported to :snippets:kotlin-app. Root cause: snippet-bot only checks whether a tag exists somewhere in the repo (which MapsObject.java satisfied), masking that the Kotlin definitions were deleted with snippets/app—and leaving snippets/kotlin-app/src/main/res/layout/main.xml orphaned (flagged as UnusedResources by Android Lint). Fix: Port MapsObject.kt into snippets/kotlin-app/src/main/java/com/example/snippets/kotlin/MapsObject.kt to maintain Java/Kotlin DevSite tab parity.

title = "3c. Remove Single Cluster Item",
description = "What it does: Removes a specified single item from the active ClusterManager collection.\nHow to see the effect: The target marker pin is removed and surrounding cluster count numbers decrement.",
)
fun removeSingleClusterItem() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

removeSingleClusterItem & infoWindow: MyItem does not override equals/hashCode and addItems() uses "Title 0", so clusterManager?.removeItem(MyItem(..., "Title to remove", ...)) fails reference-equality lookup and removes nothing; meanwhile infoWindow() calls it.addItem(infoWindowItem) after setUpClusterer() without calling it.cluster(), so the added marker never renders.

title = "2. Map Fragment Transaction",
description = "What it does: Dynamically adds a SupportMapFragment into the Activity view hierarchy programmatically.\nHow to see the effect: A new map fragment view is instantiated and rendered into the container layout frame.",
)
fun mapFragment() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Commits a SupportMapFragment into R.id.map_container (the root RelativeLayout), whereas MapActivity.recreateMapView() only clears R.id.map_view_holder—permanently covering the screen and breaking all subsequent Next / Previous snippet navigations.

title = "2. Set Panorama Location",
description = "What it does: Sets the panorama view geographic coordinates, search radius, and outdoor source filter.\nHow to see the effect: The Street View camera jumps directly to target coordinates.",
)
fun setLocation() {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

setLocation(), zoomPanorama(), and animatePanorama() allocate unused local LatLng/StreetViewPanoramaCamera variables and launch StreetViewActivity with no Intent extras (so all 4 items do the exact same thing), while ktxAddMarker() and polylinePolygonDsl() add shapes in Sydney / Mountain View without moving the camera from (0, 0).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants