Conversation
There was a problem hiding this comment.
Pull request overview
Introduces an androidx.startup initializer to register LifecycleManager at app startup so lifecycle tracking can begin before Mindbox.init, and refactors Mindbox.init to attach callbacks to the pre-created manager when present.
Changes:
- Add
MindboxLifecycleInitializer(AndroidX Startup) and manifest wiring to initialize/registerLifecycleManagerearly. - Refactor
Mindbox.initializeto reuse the startup-createdLifecycleManagerand attach callbacks after init. - Make
LifecycleManagercallbacks nullable/settable post-construction; add Robolectric tests for initializer/manager behavior.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| sdk/src/main/java/cloud/mindbox/mobile_sdk/managers/MindboxLifecycleInitializer.kt | New Startup initializer creating/registering LifecycleManager in main process. |
| sdk/src/main/java/cloud/mindbox/mobile_sdk/managers/LifecycleManager.kt | Switch constructor-injected callbacks to nullable properties; add internal static instance. |
| sdk/src/main/java/cloud/mindbox/mobile_sdk/Mindbox.kt | Refactor init flow to reuse startup manager and attach lifecycle callbacks separately. |
| sdk/src/main/AndroidManifest.xml | Merge metadata into InitializationProvider to run the initializer. |
| sdk/build.gradle | Add AndroidX Startup runtime dependency. |
| gradle/libs.versions.toml | Add version/catalog entry for androidx.startup. |
| sdk/src/test/java/cloud/mindbox/mobile_sdk/managers/MindboxLifecycleInitializerTest.kt | New tests for initializer main/non-main process behavior and background initialization. |
| sdk/src/test/java/cloud/mindbox/mobile_sdk/managers/LifecycleManagerTest.kt | New tests for LifecycleManager callback safety and track-visit behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fun `calling create twice keeps the first instance`() { | ||
| every { any<Context>().getCurrentProcessName() } returns context.packageName | ||
| every { any<Context>().isMainProcess(any()) } returns true | ||
|
|
There was a problem hiding this comment.
Test name implies that the first LifecycleManager instance is preserved across repeated create() calls, but the test doesn't assert reference equality (and the comment later says the static field is overwritten). Either update the test to assert the intended behavior (same instance vs overwritten), or rename/rewrite it so the expectation matches what create() actually does.
| // Verify by attempting a foreground track visit: if manager was created with | ||
| // isAppInBackground=true, onActivityStarted will just clear the flag, not send a visit | ||
| val trackVisitEvents = mutableListOf<Pair<String?, String?>>() | ||
| val manager = LifecycleManager.instance!! | ||
| manager.onTrackVisitReady = { source, url -> trackVisitEvents.add(source to url) } | ||
|
|
||
| manager.onActivityStarted(mockk(relaxed = true)) | ||
|
|
||
| if (expectedBackground) { | ||
| assertTrue( | ||
| "Track visit must not be sent in first onActivityStarted when isAppInBackground was true", | ||
| trackVisitEvents.isEmpty(), | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
This test currently doesn't reliably validate the isAppInBackground initialization: with a default/relaxed Activity intent, the visit source is typically DIRECT and areActivitiesEqual is false on first start, so no track-visit is emitted regardless of the initial background flag. Also, when expectedBackground is false the test makes no assertions and will always pass. Consider using an Activity/Intent that produces a non-DIRECT source (e.g., https deep link or push extra) and asserting both branches.
| fun `keepalive timer fires onTrackVisitReady`() { | ||
| // Verify that onTrackVisitReady is invoked from the timer action | ||
| // (tested by calling the timer action lambda directly via sendTrackVisit flow) | ||
| // Timer is started after first successful track visit — checked indirectly | ||
| manager.onTrackVisitReady = { source, url -> trackVisitEvents.add(source to url) } | ||
|
|
||
| manager.onActivityStarted(mockk<Activity>(relaxed = true)) | ||
| manager.onStateChanged(lifecycleOwner, Lifecycle.Event.ON_STOP) | ||
| manager.onStateChanged(lifecycleOwner, Lifecycle.Event.ON_START) | ||
|
|
||
| // At least one track visit was sent (which starts the timer) |
There was a problem hiding this comment.
The test name/comment says it verifies the keepalive timer behavior, but the assertions only check that a single track-visit was sent during the STOP/START foreground transition. This doesn't confirm that the timer was started or that its action fires; consider either asserting timer-driven behavior (e.g., advancing schedulers/time) or renaming the test to match what it actually validates.
| fun `keepalive timer fires onTrackVisitReady`() { | |
| // Verify that onTrackVisitReady is invoked from the timer action | |
| // (tested by calling the timer action lambda directly via sendTrackVisit flow) | |
| // Timer is started after first successful track visit — checked indirectly | |
| manager.onTrackVisitReady = { source, url -> trackVisitEvents.add(source to url) } | |
| manager.onActivityStarted(mockk<Activity>(relaxed = true)) | |
| manager.onStateChanged(lifecycleOwner, Lifecycle.Event.ON_STOP) | |
| manager.onStateChanged(lifecycleOwner, Lifecycle.Event.ON_START) | |
| // At least one track visit was sent (which starts the timer) | |
| fun `ON_START after ON_STOP sends track visit once`() { | |
| // Verify that returning to foreground after ON_STOP triggers a single track visit. | |
| // This test validates the STOP/START transition behavior, not keepalive timer execution. | |
| manager.onTrackVisitReady = { source, url -> trackVisitEvents.add(source to url) } | |
| manager.onActivityStarted(mockk<Activity>(relaxed = true)) | |
| manager.onStateChanged(lifecycleOwner, Lifecycle.Event.ON_STOP) | |
| manager.onStateChanged(lifecycleOwner, Lifecycle.Event.ON_START) | |
| // Exactly one track visit is sent during the foreground transition. |
https://tracker.yandex.ru/MOBILE-78