From 7716221053f492ac69b30b545b24200379d171c1 Mon Sep 17 00:00:00 2001 From: darken Date: Wed, 5 Aug 2026 15:02:06 +0200 Subject: [PATCH] feat(overview): Cover the review prompt with unit tests Pins the Play review tool's eligibility gate, probe retries, single-flight guard and cancellation handling, the DataStore round trip of the review timestamps, the overview's card priority gate and the card itself. --- .../main/ui/overview/OverviewViewModelTest.kt | 99 +++++++ .../main/ui/overview/cards/ReviewCardTest.kt | 79 ++++++ .../common/review/GplayReviewToolTest.kt | 261 ++++++++++++++++++ .../capod/common/review/ReviewSettingsTest.kt | 72 +++++ 4 files changed, 511 insertions(+) create mode 100644 app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt create mode 100644 app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt create mode 100644 app/src/testGplay/java/eu/darken/capod/common/review/ReviewSettingsTest.kt diff --git a/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt b/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt index bbd45fde..b4bd9c77 100644 --- a/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt +++ b/app/src/test/java/eu/darken/capod/main/ui/overview/OverviewViewModelTest.kt @@ -5,6 +5,7 @@ import eu.darken.capod.common.bluetooth.BluetoothDevice2 import eu.darken.capod.common.bluetooth.BluetoothManager2 import eu.darken.capod.common.debug.Bugs import eu.darken.capod.common.permissions.Permission +import eu.darken.capod.common.review.ReviewTool import eu.darken.capod.common.upgrade.UpgradeRepo import eu.darken.capod.main.core.GeneralSettings import eu.darken.capod.main.core.MonitorMode @@ -60,6 +61,7 @@ class OverviewViewModelTest : BaseTest() { private lateinit var profilesRepo: DeviceProfilesRepo private lateinit var monitorModeResolver: MonitorModeResolver private lateinit var batteryEstimator: BatteryEstimator + private lateinit var reviewTool: ReviewTool private val timeSource: TimeSource = TestTimeSource() private lateinit var missingPermissionsFlow: MutableStateFlow> @@ -70,6 +72,7 @@ class OverviewViewModelTest : BaseTest() { private lateinit var hadLegacyReactionDataFlow: MutableStateFlow private lateinit var upgradeInfoFlow: MutableStateFlow private lateinit var effectiveModeFlow: MutableStateFlow + private lateinit var reviewStateFlow: MutableStateFlow private lateinit var fakeReactionsHintDismissed: FakeDataStoreValue private lateinit var fakeHideUnmatchedDevices: FakeDataStoreValue @@ -88,6 +91,7 @@ class OverviewViewModelTest : BaseTest() { every { it.error } returns null }) effectiveModeFlow = MutableStateFlow(MonitorMode.AUTOMATIC) + reviewStateFlow = MutableStateFlow(ReviewTool.State()) fakeReactionsHintDismissed = FakeDataStoreValue(false) fakeHideUnmatchedDevices = FakeDataStoreValue(false) Bugs.isDebug.value = false @@ -131,6 +135,12 @@ class OverviewViewModelTest : BaseTest() { every { it.profiles } returns profilesFlow every { it.hadLegacyReactionData } returns hadLegacyReactionDataFlow } + + // Explicitly stubbed, never relaxed: a relaxed mock hands back a flow that never emits, + // which would starve the combine backing `state` and hang every test in this class. + reviewTool = mockk().also { + every { it.state } returns reviewStateFlow + } } @AfterEach @@ -152,6 +162,7 @@ class OverviewViewModelTest : BaseTest() { monitorModeResolver = monitorModeResolver, batteryEstimator = batteryEstimator, timeSource = timeSource, + reviewTool = reviewTool, ) @Nested @@ -848,6 +859,94 @@ class OverviewViewModelTest : BaseTest() { } } + @Nested + inner class ReviewCardTests { + + private val connectedAddress = "AA:BB:CC:DD:EE:FF" + + private fun profile(): DeviceProfile = AppleDeviceProfile( + label = "Test", + model = PodModel.AIRPODS_PRO2, + address = connectedAddress, + ) + + /** Quiet overview: a set up profile, no missing permissions, nothing else to act on. */ + private fun quietOverview() { + profilesFlow.value = listOf(profile()) + missingPermissionsFlow.value = emptySet() + effectiveModeFlow.value = MonitorMode.AUTOMATIC + reviewStateFlow.value = ReviewTool.State(shouldAskForReview = true) + } + + @Test + fun `shown on a quiet overview when the tool asks for it`() = runTest(testDispatcher) { + quietOverview() + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe true + } + + @Test + fun `not shown while the tool does not ask for it`() = runTest(testDispatcher) { + quietOverview() + reviewStateFlow.value = ReviewTool.State(shouldAskForReview = false) + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe false + } + + @Test + fun `suppressed by a missing permission`() = runTest(testDispatcher) { + quietOverview() + missingPermissionsFlow.value = setOf(Permission.BLUETOOTH_SCAN) + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe false + } + + @Test + fun `suppressed by the no-profiles setup card`() = runTest(testDispatcher) { + quietOverview() + profilesFlow.value = emptyList() + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe false + } + + @Test + fun `suppressed by the background-monitoring-off card`() = runTest(testDispatcher) { + quietOverview() + effectiveModeFlow.value = MonitorMode.MANUAL + + val vm = createViewModel() + + vm.state.first().showReviewCard shouldBe false + } + + @Test + fun `suppressed by the troubleshooter suggestion`() = runTest(testDispatcher) { + quietOverview() + // Connected via audio but no live data: the hint appears after its debounce window. + connectedDevicesFlow.value = listOf( + mockk(relaxed = true) { every { address } returns connectedAddress } + ) + devicesFlow.value = emptyList() + + val vm = createViewModel() + var latest: OverviewViewModel.State? = null + backgroundScope.launch { vm.state.collect { latest = it } } + + advanceTimeBy(20_000) + + latest!!.showTroubleshootSuggestion shouldBe true + latest!!.showReviewCard shouldBe false + } + } + @Test fun `estimateFor is null when the device has the estimate disabled`() { val device = mockk { diff --git a/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt b/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt new file mode 100644 index 00000000..fb1efee9 --- /dev/null +++ b/app/src/test/java/eu/darken/capod/main/ui/overview/cards/ReviewCardTest.kt @@ -0,0 +1,79 @@ +package eu.darken.capod.main.ui.overview.cards + +import android.content.Context +import androidx.compose.ui.semantics.SemanticsActions +import androidx.compose.ui.test.assertIsEnabled +import androidx.compose.ui.test.assertIsNotEnabled +import androidx.compose.ui.test.hasClickAction +import androidx.compose.ui.test.hasText +import androidx.compose.ui.test.onNodeWithText +import androidx.compose.ui.test.performSemanticsAction +import androidx.test.core.app.ApplicationProvider +import eu.darken.capod.R +import eu.darken.capod.common.compose.PreviewWrapper +import org.junit.Assert.assertTrue +import org.junit.Test +import testhelpers.compose.BaseComposeRobolectricTest + +class ReviewCardTest : BaseComposeRobolectricTest() { + + private val context: Context + get() = ApplicationProvider.getApplicationContext() + + private val bodyText get() = context.getString(R.string.review_app_body) + private val reviewLabel get() = context.getString(R.string.review_app_review_action) + private val dismissLabel get() = context.getString(R.string.review_app_dismiss_action) + + // The card's title reuses the review label, so the action is matched by its click semantics. + private val reviewButton get() = hasText(reviewLabel) and hasClickAction() + + @Test + fun `renders the body and both actions`() { + composeRule.setContent { + PreviewWrapper { + ReviewCard(onReview = {}, onDismiss = {}) + } + } + + composeRule.onNodeWithText(bodyText).assertExists() + composeRule.onNodeWithText(dismissLabel).assertExists() + composeRule.onNode(reviewButton).assertIsEnabled() + } + + @Test + fun `both actions invoke their callback`() { + var reviewed = false + var dismissed = false + + composeRule.setContent { + PreviewWrapper { + ReviewCard( + onReview = { reviewed = true }, + onDismiss = { dismissed = true }, + ) + } + } + + composeRule.onNodeWithText(dismissLabel).performSemanticsAction(SemanticsActions.OnClick) + composeRule.onNode(reviewButton).performSemanticsAction(SemanticsActions.OnClick) + + composeRule.runOnIdle { + assertTrue(dismissed) + assertTrue(reviewed) + } + } + + @Test + fun `the review action is disabled without a hosting activity`() { + composeRule.setContent { + PreviewWrapper { + // Null callback = no Activity to launch Play's review flow with. + ReviewCard(onReview = null, onDismiss = {}) + } + } + + composeRule.onNode(reviewButton).assertIsNotEnabled() + // Dismissing has to stay possible, it doesn't need an Activity. + composeRule.onNodeWithText(dismissLabel).assertIsEnabled() + } +} diff --git a/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt b/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt new file mode 100644 index 00000000..02ba6dc3 --- /dev/null +++ b/app/src/testGplay/java/eu/darken/capod/common/review/GplayReviewToolTest.kt @@ -0,0 +1,261 @@ +package eu.darken.capod.common.review + +import android.app.Activity +import com.google.android.gms.tasks.OnFailureListener +import com.google.android.gms.tasks.OnSuccessListener +import com.google.android.gms.tasks.Task +import com.google.android.gms.tasks.Tasks +import com.google.android.play.core.review.ReviewInfo +import com.google.android.play.core.review.ReviewManager +import eu.darken.capod.common.datastore.DataStoreValue +import eu.darken.capod.common.upgrade.UpgradeRepo +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.shouldBe +import io.mockk.coVerify +import io.mockk.every +import io.mockk.mockk +import io.mockk.slot +import io.mockk.verify +import kotlinx.coroutines.CancellationException +import kotlinx.coroutines.flow.drop +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.launch +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.advanceUntilIdle +import kotlinx.coroutines.withTimeoutOrNull +import org.junit.jupiter.api.Test +import testhelpers.BaseTest +import testhelpers.coroutine.runTest2 +import java.time.Duration +import java.time.Instant + +class GplayReviewToolTest : BaseTest() { + + private val manager = mockk() + private val settings = mockk() + private val upgradeRepo = mockk() + private lateinit var lastDismissedMock: DataStoreValue + private lateinit var reviewedAtMock: DataStoreValue + + // Relaxed so the `.value(new)` writes (which go through `update`) succeed and can be verified. + private fun rwSetting(initial: T): DataStoreValue = mockk>(relaxed = true).apply { + every { flow } returns flowOf(initial) + } + + // The tool's own scope has to run on the test scheduler, otherwise the probe backoff and the + // state throttle would burn real time. + private fun TestScope.tool( + upgradedAt: Instant? = Instant.now().minus(Duration.ofDays(30)), + lastDismissed: Instant? = null, + reviewedAt: Instant? = null, + ): GplayReviewTool { + lastDismissedMock = rwSetting(lastDismissed) + reviewedAtMock = rwSetting(reviewedAt) + every { settings.lastDismissed } returns lastDismissedMock + every { settings.reviewedAt } returns reviewedAtMock + + val upgradeInfo = mockk() + every { upgradeInfo.upgradedAt } returns upgradedAt + every { upgradeRepo.upgradeInfo } returns flowOf(upgradeInfo) + + return GplayReviewTool( + appScope = backgroundScope, + settings = settings, + manager = manager, + upgradeRepo = upgradeRepo, + ).apply { probeRetryDelay = Duration.ofSeconds(1) } + } + + private fun reviewInfo(canShow: Boolean = true): ReviewInfo { + val info = mockk() + // There is no public accessor for the isNoOp flag, the tool sniffs the toString(). + every { info.toString() } returns when { + canShow -> "ReviewInfo{pendingIntent=PendingIntent{1}, isNoOp=false}" + else -> "ReviewInfo{pendingIntent=null, isNoOp=true}" + } + return info + } + + private fun activity(finishing: Boolean = false, destroyed: Boolean = false) = mockk().apply { + every { isFinishing } returns finishing + every { isDestroyed } returns destroyed + } + + private fun launchOk(): Task = Tasks.forResult(null) + + // The `onStart` seed is not a computed state, every assertion has to await the first real one. + private suspend fun GplayReviewTool.computedState() = state.drop(1).first() + + @Test fun `an eligible user is asked for a review`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + + tool().computedState().apply { + shouldAskForReview shouldBe true + hasReviewed shouldBe false + } + } + + @Test fun `a user who has not paid for pro long enough is never probed`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + + tool(upgradedAt = Instant.now().minus(Duration.ofDays(3))) + .computedState().shouldAskForReview shouldBe false + + // Eligibility is decided locally first: Play's request quota is only spent on candidates. + verify(exactly = 0) { manager.requestReviewFlow() } + } + + @Test fun `a recently dismissed card is not shown again`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + + tool(lastDismissed = Instant.now().minus(Duration.ofDays(3))) + .computedState().shouldAskForReview shouldBe false + + verify(exactly = 0) { manager.requestReviewFlow() } + } + + @Test fun `reviewNow launches with a freshly requested ReviewInfo`() = runTest2 { + // ReviewInfo is short lived, the token used for the launch must not be the one the + // availability probe obtained (potentially hours) earlier. + val probeInfo = reviewInfo() + val freshInfo = reviewInfo() + every { manager.requestReviewFlow() } returnsMany listOf( + Tasks.forResult(probeInfo), + Tasks.forResult(freshInfo), + ) + every { manager.launchReviewFlow(any(), any()) } returns launchOk() + val tool = tool() + val activity = activity() + + tool.computedState().shouldAskForReview shouldBe true + tool.reviewNow(activity) + + verify(exactly = 2) { manager.requestReviewFlow() } + verify(exactly = 1) { manager.launchReviewFlow(activity, freshInfo) } + verify(exactly = 0) { manager.launchReviewFlow(activity, probeInfo) } + } + + @Test fun `a failed fresh request keeps the card and persists nothing`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forException(RuntimeException("Play unavailable")) + val tool = tool() + + tool.reviewNow(activity()) + + verify(exactly = 0) { manager.launchReviewFlow(any(), any()) } + // A transient failure is not user intent, the next tap has to be able to retry. + coVerify(exactly = 0) { lastDismissedMock.update(any()) } + coVerify(exactly = 0) { reviewedAtMock.update(any()) } + } + + @Test fun `a fresh isNoOp answer snoozes the card`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo(canShow = false)) + val tool = tool() + + tool.reviewNow(activity()) + + verify(exactly = 0) { manager.launchReviewFlow(any(), any()) } + // isNoOp is Play's quota verdict, asking again right away would be pointless. + coVerify(exactly = 1) { lastDismissedMock.update(any()) } + coVerify(exactly = 0) { reviewedAtMock.update(any()) } + } + + @Test fun `a failed launch persists nothing and does not escape`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + every { manager.launchReviewFlow(any(), any()) } returns Tasks.forException(RuntimeException("launch failed")) + val tool = tool() + + tool.reviewNow(activity()) + + coVerify(exactly = 0) { lastDismissedMock.update(any()) } + coVerify(exactly = 0) { reviewedAtMock.update(any()) } + } + + @Test fun `a dead activity aborts the launch without persisting`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forResult(reviewInfo()) + every { manager.launchReviewFlow(any(), any()) } returns launchOk() + val tool = tool() + + // The fresh request is a Play round-trip, the activity can be gone by the time it returns. + tool.reviewNow(activity(finishing = true)) + + verify(exactly = 0) { manager.launchReviewFlow(any(), any()) } + coVerify(exactly = 0) { lastDismissedMock.update(any()) } + coVerify(exactly = 0) { reviewedAtMock.update(any()) } + } + + @Test fun `overlapping reviewNow calls launch the flow only once`() = runTest2 { + // Park the first call on an unresolved Play request, so the second genuinely overlaps it. + val successListener = slot>() + val pendingRequest = mockk>().apply { + every { isComplete } returns false + every { addOnSuccessListener(capture(successListener)) } returns this + every { addOnFailureListener(any()) } returns this + } + every { manager.requestReviewFlow() } returnsMany listOf( + pendingRequest, + // A second request would resolve instantly, so a broken guard shows up as a launch. + Tasks.forResult(reviewInfo()), + ) + every { manager.launchReviewFlow(any(), any()) } returns launchOk() + val tool = tool() + val activity = activity() + + val first = launch { tool.reviewNow(activity) } + advanceUntilIdle() + + tool.reviewNow(activity) + + successListener.captured.onSuccess(reviewInfo()) + first.join() + + // Without the single-flight guard the second tap would run its own request+launch, and + // Play's flow would pop up again the moment the user returned from the first one. + verify(exactly = 1) { manager.requestReviewFlow() } + verify(exactly = 1) { manager.launchReviewFlow(any(), any()) } + } + + @Test fun `a probe that fails once still resolves within the retry budget`() = runTest2 { + every { manager.requestReviewFlow() } returnsMany listOf( + Tasks.forException(RuntimeException("Play unavailable")), + Tasks.forResult(reviewInfo()), + ) + + tool().computedState().shouldAskForReview shouldBe true + + verify(exactly = 2) { manager.requestReviewFlow() } + } + + @Test fun `a probe that keeps failing hides the card`() = runTest2 { + every { manager.requestReviewFlow() } returns Tasks.forException(RuntimeException("Play unavailable")) + + tool().computedState().shouldAskForReview shouldBe false + + verify(exactly = 3) { manager.requestReviewFlow() } + } + + @Test fun `cancellation during reviewNow is not swallowed`() = runTest2 { + every { manager.requestReviewFlow() } throws CancellationException("scope died") + val tool = tool() + + shouldThrow { tool.reviewNow(activity()) } + + // A cancelled coroutine is not a Play failure: no launch, no bookkeeping. + verify(exactly = 0) { manager.launchReviewFlow(any(), any()) } + coVerify(exactly = 0) { lastDismissedMock.update(any()) } + coVerify(exactly = 0) { reviewedAtMock.update(any()) } + } + + @Test fun `cancellation during the probe is not swallowed into the retry path`() = runTest2 { + every { manager.requestReviewFlow() } throws CancellationException("scope died") + val tool = tool() + + // Cancellation takes the probe down with its scope, no verdict is ever computed (virtual + // time, so the timeout costs nothing). + val computed = withTimeoutOrNull(Duration.ofMinutes(1).toMillis()) { tool.computedState() } + + computed shouldBe null + // The general failure path would have burned all 3 attempts and settled on "unavailable". + verify(exactly = 1) { manager.requestReviewFlow() } + } +} diff --git a/app/src/testGplay/java/eu/darken/capod/common/review/ReviewSettingsTest.kt b/app/src/testGplay/java/eu/darken/capod/common/review/ReviewSettingsTest.kt new file mode 100644 index 00000000..65cdd57e --- /dev/null +++ b/app/src/testGplay/java/eu/darken/capod/common/review/ReviewSettingsTest.kt @@ -0,0 +1,72 @@ +package eu.darken.capod.common.review + +import android.content.Context +import androidx.datastore.preferences.core.edit +import androidx.datastore.preferences.core.stringPreferencesKey +import androidx.test.core.app.ApplicationProvider +import eu.darken.capod.common.datastore.value +import io.kotest.assertions.throwables.shouldThrow +import io.kotest.matchers.shouldBe +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import kotlinx.serialization.SerializationException +import kotlinx.serialization.json.Json +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.annotation.Config +import testhelpers.BaseTest +import testhelpers.TestApplication +import java.time.Instant + +@RunWith(RobolectricTestRunner::class) +@Config(sdk = [33], application = TestApplication::class) +class ReviewSettingsTest : BaseTest() { + + private val json = Json { ignoreUnknownKeys = true } + + // One test method on purpose: ReviewSettings is a @Singleton whose DataStore is bound to the + // Context property delegate, and DataStore forbids two active instances on the same file. + @Test + fun `the review timestamps round-trip through the real DataStore`() = runTest { + // Real time and real I/O: the DataStore does its work off the test scheduler. + withContext(Dispatchers.IO) { + val context = ApplicationProvider.getApplicationContext() + + // Real DataStore, no mocks: this catches a mismatch between what the explicit + // Instant serializer writes and what the reader expects to find. + val settings = ReviewSettings(context, json) + + settings.lastDismissed.value() shouldBe null + settings.reviewedAt.value() shouldBe null + + // Millisecond granularity, that is all InstantEpochMillisSerializer preserves. + val dismissedAt = Instant.ofEpochMilli(1_700_000_000_000L) + val reviewedAt = Instant.ofEpochMilli(1_700_000_123_456L) + + settings.lastDismissed.value(dismissedAt) + settings.reviewedAt.value(reviewedAt) + + settings.lastDismissed.value() shouldBe dismissedAt + settings.reviewedAt.value() shouldBe reviewedAt + + // Writing null clears the key instead of storing a literal "null" that would then be + // decoded on the next read. + settings.lastDismissed.value(null) + settings.lastDismissed.value() shouldBe null + settings.dataStore.data.first().contains(DISMISSED_KEY) shouldBe false + + // onErrorFallbackToDefault is off, so corrupt data surfaces instead of silently + // resetting the snooze/reviewed bookkeeping to "never". + settings.dataStore.edit { it[REVIEWED_KEY] = "not-a-timestamp" } + shouldThrow { settings.reviewedAt.value() } + } + } + + companion object { + private val DISMISSED_KEY = stringPreferencesKey("review.dismissedAt") + private val REVIEWED_KEY = stringPreferencesKey("review.reviewedAt") + } +}