From 4d5ae4a3f1901295d71972e68c7eecbc72f26b81 Mon Sep 17 00:00:00 2001 From: Steven Gates Date: Mon, 15 Jun 2026 15:06:05 -0500 Subject: [PATCH 1/2] feat(updates): add Play in-app updates with FLEXIBLE flow + staleness gate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a gentle "update ready — restart to install" nudge on cold start using Google's Play In-App Updates 2.1.0 library. Reuses the existing SnackbarHost in MainActivity for the prompt — no new UI surface, no new OpenLoopUiState. Design (full spec in docs/active/in-app-updates/IMPLEMENTATION.md): - AppUpdateController wraps the Play AppUpdateManager. Activity owns the ActivityResultLauncher (only it can register one before STARTED) and hands it in via attach(); controller owns the listener lifecycle. - Pure shouldPromptForFlexibleUpdate() function encodes the gate: UPDATE_AVAILABLE AND FLEXIBLE allowed AND (staleness >= 3 days OR bypass on). - Debug-only BuildConfig.UPDATE_BYPASS_STALENESS flag (true/debug, false/release) exists so the snackbar can be visually verified via Internal App Sharing, where clientVersionStalenessDays() returns null. Release AABs cannot ship with the bypass on. - Off-Play behavior (sideloaded debug builds) is a silent no-op — Play returns UPDATE_NOT_AVAILABLE, controller logs at debug, no UI is shown. Verification (DoD per docs/DEFINITION_OF_DONE.md): - T1: JVM unit test on the pure gate (8 cases over the availability x staleness x flexible-allowed x bypass matrix). - T2: mockk-based wiring test on AppUpdateController (listener registration, DOWNLOADED routing, completeUpdate delegation, attach idempotency, detach cleanup). Mockk chosen over FakeAppUpdateManager because the latter requires a Context — see PRD §Verification for the trade-off. - T3 (manual, Internal App Sharing) and T4 (internal testing track) documented in the PRD; cannot be run pre-merge. Unit tests 206/206 green. lintDebug 0 errors. assembleDebug + bundleRelease both BUILD SUCCESSFUL. Emulator screenshot deferred — see PR description. Co-Authored-By: Claude Opus 4.7 --- app/build.gradle.kts | 15 + .../github/stozo04/openloop/MainActivity.kt | 70 +++++ .../openloop/update/AppUpdateController.kt | 183 ++++++++++++ app/src/main/res/values/strings.xml | 4 + .../update/AppUpdateControllerTest.kt | 159 +++++++++++ .../openloop/update/AppUpdateGateTest.kt | 136 +++++++++ docs/active/in-app-updates/IMPLEMENTATION.md | 270 ++++++++++++++++++ gradle/libs.versions.toml | 5 + 8 files changed, 842 insertions(+) create mode 100644 app/src/main/java/io/github/stozo04/openloop/update/AppUpdateController.kt create mode 100644 app/src/test/java/io/github/stozo04/openloop/update/AppUpdateControllerTest.kt create mode 100644 app/src/test/java/io/github/stozo04/openloop/update/AppUpdateGateTest.kt create mode 100644 docs/active/in-app-updates/IMPLEMENTATION.md diff --git a/app/build.gradle.kts b/app/build.gradle.kts index b214844..b749265 100644 --- a/app/build.gradle.kts +++ b/app/build.gradle.kts @@ -48,6 +48,13 @@ android { } buildTypes { + debug { + // In-app updates: Play's clientVersionStalenessDays returns null/0 over Internal App + // Sharing, so without a bypass the FLEXIBLE-update snackbar can't be manually verified + // pre-merge. Debug = true so IAS testing surfaces the prompt; release = false so the + // production threshold genuinely gates. See docs/active/in-app-updates/IMPLEMENTATION.md. + buildConfigField("boolean", "UPDATE_BYPASS_STALENESS", "true") + } release { isMinifyEnabled = true isShrinkResources = true @@ -63,6 +70,10 @@ android { // Sign the release (APK/AAB) only when the keystore is present; otherwise leave it // unsigned so non-publishing tasks still build. signingConfig = if (keystorePropertiesFile.exists()) signingConfigs.getByName("release") else null + // Release ALWAYS honors the real staleness threshold — bypass exists only for debug + // IAS verification. Wiring it here (not just relying on the debug branch) makes the + // intent explicit and means a release AAB cannot ship with the bypass on. + buildConfigField("boolean", "UPDATE_BYPASS_STALENESS", "false") } } compileOptions { @@ -171,6 +182,10 @@ dependencies { implementation(libs.androidx.media3.ui) implementation(libs.androidx.media3.effect) + // Play In-App Updates — gentle "update ready" nudge on cold start (FLEXIBLE flow). + // See docs/active/in-app-updates/IMPLEMENTATION.md. -ktx pulls -core transitively. + implementation(libs.play.app.update.ktx) + // Testing testImplementation(libs.junit) testImplementation(libs.mockk) diff --git a/app/src/main/java/io/github/stozo04/openloop/MainActivity.kt b/app/src/main/java/io/github/stozo04/openloop/MainActivity.kt index 97cf567..2ba7e36 100644 --- a/app/src/main/java/io/github/stozo04/openloop/MainActivity.kt +++ b/app/src/main/java/io/github/stozo04/openloop/MainActivity.kt @@ -12,6 +12,7 @@ import androidx.activity.ComponentActivity import androidx.activity.compose.BackHandler import androidx.activity.compose.setContent import androidx.activity.enableEdgeToEdge +import androidx.activity.result.IntentSenderRequest import androidx.activity.result.PickVisualMediaRequest import androidx.activity.result.contract.ActivityResultContracts import androidx.activity.viewModels @@ -47,6 +48,7 @@ import androidx.compose.runtime.LaunchedEffect import androidx.compose.runtime.getValue import androidx.compose.runtime.mutableStateOf import androidx.compose.runtime.remember +import androidx.compose.runtime.rememberCoroutineScope import androidx.compose.runtime.setValue import androidx.compose.ui.Alignment import androidx.compose.ui.Modifier @@ -70,6 +72,7 @@ import androidx.lifecycle.compose.LocalLifecycleOwner import androidx.lifecycle.compose.collectAsStateWithLifecycle import androidx.media3.common.util.UnstableApi import androidx.work.WorkManager +import com.google.android.play.core.appupdate.AppUpdateManagerFactory import io.github.stozo04.openloop.camera.CameraManager import io.github.stozo04.openloop.data.UserPreferencesRepositoryImpl import io.github.stozo04.openloop.data.VideoImporterImpl @@ -90,6 +93,7 @@ import io.github.stozo04.openloop.ui.OpenLoopUiState import io.github.stozo04.openloop.ui.OpenLoopViewModel import io.github.stozo04.openloop.ui.ProcessingScreen import io.github.stozo04.openloop.ui.TrimScreen +import io.github.stozo04.openloop.update.AppUpdateController import io.github.stozo04.openloop.ui.theme.Canvas import io.github.stozo04.openloop.ui.theme.CoralRed import io.github.stozo04.openloop.ui.theme.ElectricLime @@ -100,6 +104,7 @@ import io.github.stozo04.openloop.ui.theme.OutlineVariant import io.github.stozo04.openloop.ui.theme.SurfaceContainer import io.github.stozo04.openloop.ui.theme.SurfaceContainerHigh import io.github.stozo04.openloop.ui.theme.TextPrimary +import kotlinx.coroutines.launch import java.io.File class MainActivity : ComponentActivity() { @@ -131,6 +136,13 @@ class MainActivity : ComponentActivity() { } private lateinit var cameraManager: CameraManager + /** + * Play In-App Updates controller (FLEXIBLE flow). Constructed in [onCreate] once the launcher + * below is registered. See `docs/active/in-app-updates/IMPLEMENTATION.md` for the design and + * the verification plan (T1 unit, T2 fake, T3 Internal App Sharing, T4 internal track). + */ + private lateinit var appUpdateController: AppUpdateController + /** * Set when a boomerang share sheet is launched (slice 06); consumed on the next [onResume]. The * "Saved — view in gallery" snackbar is deferred until then so it shows when the user is actually @@ -156,6 +168,20 @@ class MainActivity : ComponentActivity() { ActivityResultContracts.RequestPermission(), ) { /* granted or denied — export continues either way; notification is best-effort */ } + /** + * Play In-App Updates flow launcher (FLEXIBLE — non-blocking). Registered here as a class + * property so it's wired before `STARTED`, per the Activity Result API contract; the launched + * intent is owned by Play and shows its own confirmation dialog. A non-OK result just means + * the user declined or Play failed — nothing else in the app depends on it. + */ + private val appUpdateLauncher = registerForActivityResult( + ActivityResultContracts.StartIntentSenderForResult(), + ) { result -> + if (result.resultCode != AppUpdateController.RESULT_OK) { + Log.w(TAG, "In-app update flow declined or failed: resultCode=${result.resultCode}") + } + } + private val requestPermissionLauncher = registerForActivityResult( ActivityResultContracts.RequestPermission(), ) { granted -> @@ -203,6 +229,13 @@ class MainActivity : ComponentActivity() { savedInstanceState?.getBoolean(KEY_DEFERRED_SHARE_SHOW_SAVED, true) != false cameraManager = CameraManager(this) + // Wire the Play In-App Updates controller (FLEXIBLE flow). The Activity owns the + // ActivityResultLauncher above; the controller owns the AppUpdateManager + listener. + // onUpdateDownloaded is set inside setContent below, after snackbarHostState exists. + appUpdateController = AppUpdateController( + appUpdateManager = AppUpdateManagerFactory.create(applicationContext), + ).apply { attach(appUpdateLauncher) } + setContent { OpenLoopTheme { val uiState by viewModel.uiState.collectAsStateWithLifecycle() @@ -222,6 +255,10 @@ class MainActivity : ComponentActivity() { val reversePreviewReportAction = stringResource(R.string.snackbar_reverse_preview_report_action) val importFailedMessage = stringResource(R.string.snackbar_import_failed) val undoAction = stringResource(R.string.undo) + // In-app update prompt copy (read in composable scope so the resource lookup is + // invalidated on configuration change). + val updateReadyMessage = stringResource(R.string.update_ready_message) + val updateReadyAction = stringResource(R.string.update_ready_action) // The "N loops deleted" plural is count-dependent, so we capture resources here (in a // composable scope) and resolve the quantity string inside the collect lambda below. // LocalResources (not LocalContext.current.resources) so the read is invalidated on a @@ -230,12 +267,39 @@ class MainActivity : ComponentActivity() { val lifecycleOwner = LocalLifecycleOwner.current + // Coroutine scope for the in-app update snackbar — onUpdateDownloaded fires from a + // Play listener callback (not a coroutine), so we need a scope to drive the + // suspend showSnackbar. rememberCoroutineScope is tied to the composition's + // lifetime; it cancels with the Activity. + val updateSnackbarScope = rememberCoroutineScope() + LaunchedEffect(Unit) { viewModel.requestPostNotifications.collect { maybeRequestPostNotificationsPermission() } } + // Wire the in-app update prompt and fire the cold-start check exactly once. + // onUpdateDownloaded fires from a Play install-state callback (not a coroutine), + // so the closure dispatches via updateSnackbarScope into the host's suspend queue. + // Indefinite + action: restart-required, never auto-dismiss. The styled + // SnackbarHost below renders this with the Electric-Lime accent. + LaunchedEffect(Unit) { + appUpdateController.onUpdateDownloaded = { + updateSnackbarScope.launch { + val result = snackbarHostState.showSnackbar( + message = updateReadyMessage, + actionLabel = updateReadyAction, + duration = SnackbarDuration.Indefinite, + ) + if (result == SnackbarResult.ActionPerformed) { + appUpdateController.completeUpdate() + } + } + } + appUpdateController.checkOnStart() + } + // Collect one-shot boomerang events → share sheet + snackbars (the app's only // SnackbarHost). `when` stays exhaustive with no `else` (Lesson 014) so a new event // must be handled here to compile. @@ -465,6 +529,9 @@ class MainActivity : ComponentActivity() { awaitingShareReturn = false viewModel.onShareSheetClosed() } + // Re-surface the "Update ready" prompt if a Play download completed while the app was + // backgrounded (Google's recommended stalled-update handling). + appUpdateController.checkOnResume() } override fun onSaveInstanceState(outState: Bundle) { @@ -478,6 +545,9 @@ class MainActivity : ComponentActivity() { override fun onDestroy() { super.onDestroy() cameraManager.shutdown() + // Release the Play InstallStateUpdatedListener. The ActivityResultLauncher unregisters + // automatically with the Activity. + if (::appUpdateController.isInitialized) appUpdateController.detach() } } diff --git a/app/src/main/java/io/github/stozo04/openloop/update/AppUpdateController.kt b/app/src/main/java/io/github/stozo04/openloop/update/AppUpdateController.kt new file mode 100644 index 0000000..b8a1233 --- /dev/null +++ b/app/src/main/java/io/github/stozo04/openloop/update/AppUpdateController.kt @@ -0,0 +1,183 @@ +package io.github.stozo04.openloop.update + +import android.app.Activity +import android.util.Log +import androidx.activity.result.ActivityResultLauncher +import androidx.activity.result.IntentSenderRequest +import com.google.android.play.core.appupdate.AppUpdateManager +import com.google.android.play.core.appupdate.AppUpdateOptions +import com.google.android.play.core.install.InstallStateUpdatedListener +import com.google.android.play.core.install.model.AppUpdateType +import com.google.android.play.core.install.model.InstallStatus +import com.google.android.play.core.install.model.UpdateAvailability +import io.github.stozo04.openloop.BuildConfig + +/** + * Default staleness gate for the FLEXIBLE in-app update prompt. Lets a same-week patch reach + * users on day 0–2 without nagging, then nudges anyone still on a 3+ day-old build. Tunable — + * see [shouldPromptForFlexibleUpdate]. + */ +const val UPDATE_STALENESS_DAYS_THRESHOLD: Int = 3 + +/** + * Pure decision: should we prompt the user with a FLEXIBLE in-app update right now? + * + * Extracted as a top-level pure function so it's covered by a plain JVM unit test (T1 in the + * verification plan) without dragging the Play library into a Robolectric harness. Every Play + * field that drives the decision is passed in as a primitive so the test has no need to construct + * a real [com.google.android.play.core.appupdate.AppUpdateInfo]. + * + * Logic: prompt iff Play reports an update is available, FLEXIBLE is allowed, and either the + * staleness bypass is on (debug builds, for IAS verification) or the build has been stale for at + * least [thresholdDays] days. A null [stalenessDays] is treated as "not stale enough" — Play + * returns null when it hasn't yet computed the staleness, which is correct for our gate. + */ +fun shouldPromptForFlexibleUpdate( + availability: Int, + stalenessDays: Int?, + flexibleAllowed: Boolean, + thresholdDays: Int = UPDATE_STALENESS_DAYS_THRESHOLD, + bypassStaleness: Boolean = false, +): Boolean = availability == UpdateAvailability.UPDATE_AVAILABLE + && flexibleAllowed + && (bypassStaleness || (stalenessDays ?: -1) >= thresholdDays) + +/** + * Activity-bound wrapper around Google Play's [AppUpdateManager]. Drives the FLEXIBLE in-app + * update flow: checks Play's update metadata on cold start, kicks off a background download when + * the gate is open, and surfaces the "Update ready — restart to install" prompt via [onUpdateDownloaded] + * once Play has the new APK staged locally. + * + * Designed for testability: + * - [appUpdateManager] is injectable so tests can pass a `FakeAppUpdateManager` (T2). + * - The decision logic lives in [shouldPromptForFlexibleUpdate] (T1). + * - The [ActivityResultLauncher] is **owned by the Activity** (only it can register one before + * `STARTED`) and handed to the controller via [attach]. The controller never reaches for an + * Activity instance itself. + * + * Lifecycle: [attach] in `onCreate` after the launcher is registered; [detach] in `onDestroy` to + * release the [InstallStateUpdatedListener]. [checkOnStart] runs once per cold start; [checkOnResume] + * runs on every `onResume` to re-surface a download that completed while the app was backgrounded + * (Google's recommended stalled-update handling). + * + * Off-Play behavior: when the app is sideloaded (or running on a device without Play Services), + * [AppUpdateManager.getAppUpdateInfo] returns `UPDATE_NOT_AVAILABLE` and the controller silently + * no-ops. No exception is thrown and no UI is shown — the correct behavior for debug installs. + */ +class AppUpdateController( + private val appUpdateManager: AppUpdateManager, + private val bypassStaleness: Boolean = BuildConfig.UPDATE_BYPASS_STALENESS, + private val stalenessThresholdDays: Int = UPDATE_STALENESS_DAYS_THRESHOLD, +) { + /** + * Invoked when Play has finished downloading a newer APK and it's ready to install. The + * Activity sets this to fire the "Update ready — restart to install" snackbar. Stays nullable + * so the controller can be constructed before the SnackbarHostState exists (it's created + * inside `setContent`). + */ + var onUpdateDownloaded: (() -> Unit)? = null + + private var updateLauncher: ActivityResultLauncher? = null + private var installStateListener: InstallStateUpdatedListener? = null + + /** + * Idempotent. Wires the [ActivityResultLauncher] (owned by the host Activity) and registers + * the [InstallStateUpdatedListener] that routes `DOWNLOADED` transitions to [onUpdateDownloaded]. + * Safe to call multiple times — subsequent calls are no-ops. + */ + fun attach(launcher: ActivityResultLauncher) { + if (updateLauncher != null) return + updateLauncher = launcher + installStateListener = InstallStateUpdatedListener { state -> + if (state.installStatus() == InstallStatus.DOWNLOADED) { + Log.i(TAG, "Update downloaded — prompting to restart") + onUpdateDownloaded?.invoke() + } + }.also { appUpdateManager.registerListener(it) } + } + + /** + * One-shot check on cold start. If Play reports a newer build and the gate is open, kicks off + * a FLEXIBLE update flow (Play's confirmation dialog → background download). If the gate is + * closed for any reason, silently no-ops with a debug log. + */ + fun checkOnStart() { + val launcher = updateLauncher ?: run { + Log.w(TAG, "checkOnStart called before attach(); skipping") + return + } + appUpdateManager.appUpdateInfo + .addOnSuccessListener { info -> + val shouldPrompt = shouldPromptForFlexibleUpdate( + availability = info.updateAvailability(), + stalenessDays = info.clientVersionStalenessDays(), + flexibleAllowed = info.isUpdateTypeAllowed(AppUpdateType.FLEXIBLE), + thresholdDays = stalenessThresholdDays, + bypassStaleness = bypassStaleness, + ) + if (shouldPrompt) { + Log.i( + TAG, + "UPDATE_AVAILABLE, starting FLEXIBLE flow " + + "(staleness=${info.clientVersionStalenessDays()}, bypass=$bypassStaleness)", + ) + appUpdateManager.startUpdateFlowForResult( + info, + launcher, + AppUpdateOptions.newBuilder(AppUpdateType.FLEXIBLE).build(), + ) + } else { + Log.d( + TAG, + "No flexible update prompted: availability=${info.updateAvailability()} " + + "staleness=${info.clientVersionStalenessDays()} " + + "flexibleAllowed=${info.isUpdateTypeAllowed(AppUpdateType.FLEXIBLE)}", + ) + } + } + .addOnFailureListener { e -> + // Routine off-Play / no-network failures — log at WARN, no Crashlytics noise. + Log.w(TAG, "appUpdateInfo check failed", e) + } + } + + /** + * Re-surface the "Update ready" prompt when the user returns to the app after a download + * completed in the background (Google's recommended stalled-update handling). Cheap — just a + * single [AppUpdateManager.getAppUpdateInfo] poll. + */ + fun checkOnResume() { + appUpdateManager.appUpdateInfo.addOnSuccessListener { info -> + if (info.installStatus() == InstallStatus.DOWNLOADED) { + Log.i(TAG, "checkOnResume: update already DOWNLOADED — prompting") + onUpdateDownloaded?.invoke() + } + } + } + + /** Trigger Play to install the staged APK and restart the app. Called from the snackbar action. */ + fun completeUpdate() { + Log.i(TAG, "completeUpdate: handing off to Play install") + appUpdateManager.completeUpdate() + } + + /** + * Unregister the [InstallStateUpdatedListener]. Call from `onDestroy`. The + * [ActivityResultLauncher] is owned by the Activity and unregisters with it automatically. + */ + fun detach() { + installStateListener?.let { appUpdateManager.unregisterListener(it) } + installStateListener = null + } + + companion object { + const val TAG: String = "AppUpdate" + + /** + * Result-code constant matching `Activity.RESULT_OK`. Exposed so the launcher callback in + * the Activity can be a one-liner that doesn't pull in the full `android.app.Activity` + * surface just for this comparison. + */ + const val RESULT_OK: Int = Activity.RESULT_OK + } +} diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index 113b744..f374b46 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -47,4 +47,8 @@ That clip\'s a bit long OpenLoop makes loops from videos up to 30 seconds. Pick a shorter clip and we\'ll loop it. Got it + + + Update ready — restart to install. + Restart diff --git a/app/src/test/java/io/github/stozo04/openloop/update/AppUpdateControllerTest.kt b/app/src/test/java/io/github/stozo04/openloop/update/AppUpdateControllerTest.kt new file mode 100644 index 0000000..80ce99b --- /dev/null +++ b/app/src/test/java/io/github/stozo04/openloop/update/AppUpdateControllerTest.kt @@ -0,0 +1,159 @@ +package io.github.stozo04.openloop.update + +import android.util.Log +import androidx.activity.result.ActivityResultLauncher +import androidx.activity.result.IntentSenderRequest +import com.google.android.play.core.install.InstallState +import com.google.android.play.core.install.InstallStateUpdatedListener +import com.google.android.play.core.install.model.InstallStatus +import com.google.android.play.core.appupdate.AppUpdateManager +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkStatic +import io.mockk.slot +import io.mockk.unmockkStatic +import io.mockk.verify +import org.junit.After +import org.junit.Assert.assertEquals +import org.junit.Before +import org.junit.Test + +/** + * T2 verification — `AppUpdateController` wiring against a mocked [AppUpdateManager]. Documented + * in `docs/active/in-app-updates/IMPLEMENTATION.md` §Verification. + * + * **Why mockk, not `FakeAppUpdateManager`:** Google's `FakeAppUpdateManager` requires a [Context] + * in its constructor, which a plain JVM unit test cannot provide. Either we add Robolectric (a + * sizeable test-classpath dep just for this) or we move T2 to instrumented (`androidTest`, needs + * an emulator). Mockk catches the same wiring contracts — listener registered on attach, listener + * routes DOWNLOADED to the host callback, listener unregistered on detach, `completeUpdate()` is + * reachable — without any of that cost. The end-to-end UI verification lives in T3 (Internal App + * Sharing) where it genuinely belongs. + */ +class AppUpdateControllerTest { + + /** + * `AppUpdateController` logs progress via [android.util.Log]; the JVM stub throws unless mocked. + * Stubbed once per test rather than globally via `testOptions.unitTests.isReturnDefaultValues` + * so the change is local to this file. + */ + @Before + fun stubAndroidLog() { + mockkStatic(Log::class) + every { Log.i(any(), any()) } returns 0 + every { Log.w(any(), any()) } returns 0 + every { Log.w(any(), any(), any()) } returns 0 + every { Log.d(any(), any()) } returns 0 + } + + @After + fun releaseAndroidLog() { + unmockkStatic(Log::class) + } + + private fun newController( + manager: AppUpdateManager, + bypassStaleness: Boolean = false, + ) = AppUpdateController( + appUpdateManager = manager, + bypassStaleness = bypassStaleness, + stalenessThresholdDays = 3, + ) + + @Test + fun `attach registers a listener and detach unregisters the SAME instance`() { + val manager = mockk(relaxed = true) + val launcher = mockk>(relaxed = true) + + val controller = newController(manager) + controller.attach(launcher) + + val registeredSlot = slot() + verify(exactly = 1) { manager.registerListener(capture(registeredSlot)) } + + controller.detach() + verify(exactly = 1) { manager.unregisterListener(registeredSlot.captured) } + } + + @Test + fun `attach is idempotent — second call does not re-register the listener`() { + val manager = mockk(relaxed = true) + val launcher = mockk>(relaxed = true) + + val controller = newController(manager) + controller.attach(launcher) + controller.attach(launcher) + + verify(exactly = 1) { manager.registerListener(any()) } + } + + @Test + fun `listener routes DOWNLOADED to onUpdateDownloaded`() { + val manager = mockk(relaxed = true) + val launcher = mockk>(relaxed = true) + val controller = newController(manager) + + var downloadedCallbackInvocations = 0 + controller.onUpdateDownloaded = { downloadedCallbackInvocations++ } + + val listenerSlot = slot() + every { manager.registerListener(capture(listenerSlot)) } returns Unit + controller.attach(launcher) + + val downloadedState = mockk() + every { downloadedState.installStatus() } returns InstallStatus.DOWNLOADED + listenerSlot.captured.onStateUpdate(downloadedState) + + assertEquals(1, downloadedCallbackInvocations) + } + + @Test + fun `listener does NOT invoke onUpdateDownloaded for non-DOWNLOADED states`() { + val manager = mockk(relaxed = true) + val launcher = mockk>(relaxed = true) + val controller = newController(manager) + + var downloadedCallbackInvocations = 0 + controller.onUpdateDownloaded = { downloadedCallbackInvocations++ } + + val listenerSlot = slot() + every { manager.registerListener(capture(listenerSlot)) } returns Unit + controller.attach(launcher) + + // Drive the listener through every non-DOWNLOADED status — none should fire the snackbar. + val nonDownloadedStatuses = listOf( + InstallStatus.PENDING, + InstallStatus.DOWNLOADING, + InstallStatus.INSTALLING, + InstallStatus.INSTALLED, + InstallStatus.FAILED, + InstallStatus.CANCELED, + ) + nonDownloadedStatuses.forEach { status -> + val state = mockk() + every { state.installStatus() } returns status + listenerSlot.captured.onStateUpdate(state) + } + + assertEquals(0, downloadedCallbackInvocations) + } + + @Test + fun `completeUpdate delegates to the AppUpdateManager`() { + val manager = mockk(relaxed = true) + val controller = newController(manager) + + controller.completeUpdate() + + verify(exactly = 1) { manager.completeUpdate() } + } + + @Test + fun `detach before attach is a safe no-op`() { + val manager = mockk(relaxed = true) + val controller = newController(manager) + + controller.detach() // should not throw, should not call unregisterListener + verify(exactly = 0) { manager.unregisterListener(any()) } + } +} diff --git a/app/src/test/java/io/github/stozo04/openloop/update/AppUpdateGateTest.kt b/app/src/test/java/io/github/stozo04/openloop/update/AppUpdateGateTest.kt new file mode 100644 index 0000000..e0fb90d --- /dev/null +++ b/app/src/test/java/io/github/stozo04/openloop/update/AppUpdateGateTest.kt @@ -0,0 +1,136 @@ +package io.github.stozo04.openloop.update + +import com.google.android.play.core.install.model.UpdateAvailability +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test + +/** + * T1 verification — pure-function coverage of the FLEXIBLE update gate. Documented in + * `docs/active/in-app-updates/IMPLEMENTATION.md` §Verification. + * + * Every input is a primitive; no Play `AppUpdateInfo` is constructed. The matrix exercises + * availability × staleness × FLEXIBLE-allowed × bypass. + */ +class AppUpdateGateTest { + + @Test + fun `prompts when update available, stale enough, flexible allowed, no bypass`() { + assertTrue( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_AVAILABLE, + stalenessDays = 3, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + } + + @Test + fun `prompts when staleness exceeds threshold`() { + assertTrue( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_AVAILABLE, + stalenessDays = 10, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + } + + @Test + fun `does NOT prompt when staleness is one day short of threshold`() { + assertFalse( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_AVAILABLE, + stalenessDays = 2, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + } + + @Test + fun `does NOT prompt when staleness is null and bypass is off`() { + // Play returns null when it hasn't computed staleness yet (typical for fresh releases + // and for Internal App Sharing). Without the bypass, that must hold the prompt back. + assertFalse( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_AVAILABLE, + stalenessDays = null, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + } + + @Test + fun `bypass forces a prompt even with null staleness`() { + // This is the IAS-verification case: bypass on (debug build) overrides the staleness gate + // so the snackbar UI can be manually confirmed end-to-end before merging. + assertTrue( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_AVAILABLE, + stalenessDays = null, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = true, + ), + ) + } + + @Test + fun `bypass does NOT override availability`() { + // Even with bypass on, if Play says no update is available we must not prompt — the + // bypass only relaxes the staleness check, never invents an update. + assertFalse( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_NOT_AVAILABLE, + stalenessDays = null, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = true, + ), + ) + } + + @Test + fun `does NOT prompt when FLEXIBLE update type is not allowed`() { + assertFalse( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UPDATE_AVAILABLE, + stalenessDays = 30, + flexibleAllowed = false, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + } + + @Test + fun `does NOT prompt for unknown availability state`() { + // UNKNOWN, DEVELOPER_TRIGGERED_UPDATE_IN_PROGRESS, etc. — silent no-op. + assertFalse( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.UNKNOWN, + stalenessDays = 30, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + assertFalse( + shouldPromptForFlexibleUpdate( + availability = UpdateAvailability.DEVELOPER_TRIGGERED_UPDATE_IN_PROGRESS, + stalenessDays = 30, + flexibleAllowed = true, + thresholdDays = 3, + bypassStaleness = false, + ), + ) + } +} diff --git a/docs/active/in-app-updates/IMPLEMENTATION.md b/docs/active/in-app-updates/IMPLEMENTATION.md new file mode 100644 index 0000000..d8f1a43 --- /dev/null +++ b/docs/active/in-app-updates/IMPLEMENTATION.md @@ -0,0 +1,270 @@ +# In-App Updates — Implementation Plan + +**Branch (suggested):** `feature/in-app-updates` +**Status:** PRD draft — awaiting sign-off + +## Problem statement + +OpenLoop has no way to surface "you're on an old build" to users in-product. Today, getting a user +onto a newer `versionCode` depends entirely on them opening Play Store and noticing the update — +which most users won't do. As the app moves from concept spike toward shipping the core loop +feature, the cost of leaving users on stale builds rises: an old build can be missing a Crashlytics +fix, a codec hardening, or (eventually) the boomerang feature itself. + +The user-facing ask: **on app startup, check whether the user is on the latest version; if not, +gently nudge them to update.** + +## Scope + +### In + +- Detect "newer build available on Play" from the app process at cold start. +- Background-download the new build via Play (no user wait). +- Surface a dismissible "Update ready — restart to install" prompt in the app's existing + `SnackbarHost` once the download completes. +- Re-prompt on `onResume` if a download from a prior session is sitting in `DOWNLOADED`. + +### Out (explicit non-goals — defer) + +- Immediate / blocking update flow (full-screen "must update"). Reserve for a future PR if/when a + truly broken build ships; gated on Play Console's per-release update-priority field. +- Dedicated banner UI on Camera/Gallery. Existing snackbar carries the signal. +- Periodic WorkManager checks. Cold-start + onResume covers typical sessions. +- F-Droid / sideloaded distribution. Play-only by current product decision; the API silently + no-ops off-Play, which is the correct behavior. +- Custom "what's new in this version" copy. Play handles release notes; we don't duplicate. + +## Architecture + +Reuses three existing surfaces — no new screens, no new state machine entries: + +| Concern | Where it lives | Notes | +|---|---|---| +| Update check | `MainActivity.onCreate` (one-shot) + `MainActivity.onResume` (stalled-download check) | Mirrors Google's sample placement. Activity-bound because `startUpdateFlowForResult` needs an `ActivityResultLauncher`. | +| Snackbar surface | Existing `SnackbarHost` in `MainActivity.onCreate`'s `setContent` (the same one that shows Saved / Undo / SaveFailed) | Single host already wired; just one more `showSnackbar(...)` call site. | +| Strings | `app/src/main/res/values/strings.xml` | New: `update_ready_message`, `update_ready_action` (i.e. "Restart"). | + +State stays out of `OpenLoopUiState` / `OpenLoopViewModel`. The update flow is a peripheral concern +of the Activity, not a routed app state — it must not block the camera, the editor, or any in-flight +render. Wrapping it in the sealed `when` would force every state to think about it. + +### Dependency + +```kotlin +// libs.versions.toml +play-app-update = "2.1.0" +play-app-update-ktx = { group = "com.google.android.play", name = "app-update-ktx", version.ref = "play-app-update" } + +// app/build.gradle.kts +implementation(libs.play.app.update.ktx) +``` + +`app-update-ktx` pulls `app-update` transitively. Verified current via +`developer.android.com/guide/playcore/in-app-updates/kotlin-java` (2026-06-15). API levels 21+ — +well under our `minSdk 26`. + +### Debug-only staleness bypass + +`clientVersionStalenessDays()` only populates after Play has known about the new build for some +days, and **returns null/0 over Internal App Sharing** — so without a bypass, the snackbar UI is +impossible to verify manually before merging. Add a `BuildConfig` flag: + +```kotlin +// app/build.gradle.kts +buildTypes { + debug { + buildConfigField("boolean", "UPDATE_BYPASS_STALENESS", "true") + } + release { + buildConfigField("boolean", "UPDATE_BYPASS_STALENESS", "false") + // … existing release config + } +} +``` + +The gate function honors it: + +```kotlin +fun shouldPromptForFlexibleUpdate( + availability: Int, + stalenessDays: Int?, + flexibleAllowed: Boolean, + thresholdDays: Int = UPDATE_STALENESS_DAYS_THRESHOLD, + bypassStaleness: Boolean = BuildConfig.UPDATE_BYPASS_STALENESS, +): Boolean = availability == UpdateAvailability.UPDATE_AVAILABLE + && flexibleAllowed + && (bypassStaleness || (stalenessDays ?: -1) >= thresholdDays) +``` + +Release builds always honor the real threshold — `UPDATE_BYPASS_STALENESS` is wired only into the +debug build type, so a release AAB literally cannot ship with the bypass on. + +### Decision: FLEXIBLE only, with staleness gate + +Per sign-off above. Concretely: + +```kotlin +if (info.updateAvailability() == UpdateAvailability.UPDATE_AVAILABLE + && (info.clientVersionStalenessDays() ?: -1) >= UPDATE_STALENESS_DAYS_THRESHOLD + && info.isUpdateTypeAllowed(AppUpdateType.FLEXIBLE) +) { … } +``` + +`UPDATE_STALENESS_DAYS_THRESHOLD = 3`. Rationale: lets a same-week patch reach users on day 0–2 +without nagging; nudges anyone still on a week-old build. Easy to tune; not user-facing copy. + +Any other `updateAvailability` value (e.g. `UPDATE_NOT_AVAILABLE`, `DEVELOPER_TRIGGERED_UPDATE_IN_PROGRESS`) +is a silent no-op. Sideloaded debug builds return `UPDATE_NOT_AVAILABLE`, which is correct. + +## Implementation steps + +Ordered. Each numbered step is independently committable. + +1. **Add the dependency.** `libs.versions.toml` + `app/build.gradle.kts`. Sync; verify no resolution + conflicts (Play Core libs are well-isolated, no overlap with Firebase / Media3 / CameraX). + +2. **Add strings.** `update_ready_message = "Update ready — restart to install."`, + `update_ready_action = "Restart"`. English only (the rest of the app is English-only today). + +3. **Create `AppUpdateController` (thin wrapper).** `io.github.stozo04.openloop.update`. Hides the + Play API behind a small interface so MainActivity stays readable and unit-testable parts can be + covered without the Play library on the JVM classpath. Surface: + + ```kotlin + class AppUpdateController(private val activity: ComponentActivity) { + fun register(): Unit // registers the ActivityResultLauncher + InstallStateUpdatedListener; idempotent + fun checkOnStart(): Unit // called from onCreate after setContent + fun checkOnResume(): Unit // called from onResume — only fires the "downloaded" prompt + fun onDownloadComplete: (() -> Unit)? // set by MainActivity to trigger the snackbar + fun completeUpdate(): Unit // called when the user taps Restart + } + ``` + + Listener lifecycle: `register()` in `onCreate`, `unregister` in `onDestroy`. The + `InstallStateUpdatedListener` is the only long-lived thing; the `appUpdateInfoTask` + listeners are one-shot. + +4. **Wire into `MainActivity`.** + - Instantiate `AppUpdateController` in `onCreate` before `setContent`. + - Set `onDownloadComplete = { snackbarHostState.showSnackbar(...) }` inside the `setContent` + scope (it captures the existing `SnackbarHostState`). + - Call `controller.checkOnStart()` after `setContent`. + - Add `controller.checkOnResume()` to the existing `onResume` override. + - Add `controller.unregister()` in `onDestroy` next to `cameraManager.shutdown()`. + +5. **Snackbar contract.** Use `SnackbarDuration.Indefinite` with an action — matches the + restart-required semantics (the user must actively choose to restart). Reuses the existing + styled snackbar; no new UI. + +6. **Logcat tag.** Single `TAG = "AppUpdate"` on the controller, all failure paths logged at + `Log.w` (e.g. `startUpdateFlowForResult` returning a non-OK result). No Crashlytics non-fatals — + update failures are routine (no network, sideloaded build) and would only generate noise. + +## Verification plan + +Testing in-app updates is the awkward part of the API — the real Play flow only fires when Play +sees a newer build than the one running. Four tiers, cheapest first. + +### T1 — JVM unit test on the gate function + +**Proves:** decision logic — only prompts when availability=UPDATE_AVAILABLE **and** staleness ≥ +threshold (or bypass is on) **and** FLEXIBLE allowed. +**How:** `app/src/test/java/.../update/AppUpdateGateTest.kt`. The gate is a pure function over +`(Int, Int?, Boolean, Int, Boolean)` — no Play types — so JUnit covers it directly. ~6 cases: +matrix of {AVAILABLE, NOT_AVAILABLE} × {null, 0, threshold-1, threshold} × {flexible allowed, +not allowed} × {bypass on, off}. +**Falls short of:** Activity wiring, snackbar plumbing. + +### T2 — mockk-based wiring test + +**Proves:** `AppUpdateController` correctly drives the listener — `attach()` registers a single +`InstallStateUpdatedListener`, that listener routes `DOWNLOADED` (and *only* `DOWNLOADED`) to +`onUpdateDownloaded`, `detach()` unregisters the same listener instance, `completeUpdate()` +delegates, and `attach()` is idempotent. +**How:** `app/src/test/java/.../update/AppUpdateControllerTest.kt`. Inject a mockk +`AppUpdateManager`, capture the registered listener with `slot()`, +drive it manually through every `InstallStatus` and verify the host callback fires exactly when +`DOWNLOADED` is observed. +**Why not Google's `FakeAppUpdateManager`:** its constructor requires a `Context`, which a plain +JVM unit test cannot provide. Adopting it would force either Robolectric (a big test-classpath dep +for one feature) or moving T2 to `androidTest` (needs an emulator on every run). Mockk catches the +same wiring contracts at a fraction of the cost. The end-to-end UI verification belongs in T3 +anyway, where it's a real device against real Play servers. +**Falls short of:** real UI rendering and Play's confirmation dialog. By design — that's T3. + +### T3 — Internal App Sharing (IAS) + +**Proves:** real Play update flow on a real device against real Play servers — the only way to +visually confirm the snackbar appears and Restart actually relaunches on the new build before +publishing to a track. +**How:** with the debug-only staleness bypass on (release builds always honor the threshold): +1. Build release-signed AAB at current versionCode (22), upload to Play Console IAS, install on + phone via the IAS link, open from launcher. Logcat: `AppUpdate: UPDATE_NOT_AVAILABLE`. +2. Bump versionCode to 23, upload that AAB to IAS, click the **new** IAS link on the phone — + *do not install from the Play Store page it shows*. Open the *installed v22* from the launcher. +3. Expect: logcat `AppUpdate: UPDATE_AVAILABLE, starting FLEXIBLE flow` → Play's confirmation + dialog → background download → snackbar **"Update ready — restart to install"** with + **Restart** → tap Restart → app relaunches as v23. +4. Repeat step 2 with a release build (bypass off) to confirm the threshold genuinely gates the + snackbar. + +**Requirements:** Same applicationId + signing key; the device's Google account has previously +downloaded OpenLoop from Play. `inAppUpdatePriority` is not honored over IAS (irrelevant — we're +FLEXIBLE-only). `clientVersionStalenessDays()` is null over IAS (hence the bypass). + +### T4 — Internal testing track + +**Proves:** true production semantics, real staleness counter, real Play propagation. +**How:** publish v22 to internal track, install, publish v23, wait 15 min–several hours, cold-start. +**Falls short of:** fast iteration — Play propagation is slow and bumps versionCode for real. +Reserved for the post-merge "confirm `clientVersionStalenessDays` actually populates after 3 days" +observation noted under [Known limitations](#known-limitations). + +### Off-Play (sideload) check + +Build debug, install via `adb install`, cold-start. Logcat: `AppUpdate: UPDATE_NOT_AVAILABLE +(sideloaded)`. No snackbar, no exceptions. Confirms the silent no-op. + +## Go/no-go checklist before opening the PR + +In order: + +1. `./gradlew :app:testDebugUnitTest` — T1 + T2 both green. +2. `./gradlew :app:lintDebug` — zero new errors over the baseline. +3. `./gradlew :app:assembleDebug` and `:app:bundleRelease` — both `BUILD SUCCESSFUL` with exit 0 + and zero `e:` errors. +4. Off-Play sideload check above. +5. T3 step 1 (current versionCode on IAS, expect UPDATE_NOT_AVAILABLE). +6. T3 steps 2–3 (new versionCode on IAS, expect snackbar → Restart → relaunch on v23). +7. T3 step 4 (release build, bypass off — confirm threshold genuinely gates). +8. Launch app on emulator, capture screenshot per DoD, attach to PR alongside an IAS screenshot + of the snackbar in action. + +## Known limitations + +- **`clientVersionStalenessDays()` in production cannot be verified pre-merge.** IAS doesn't + populate it. The only honest confirmation is a post-merge observation: ship to the internal + testing track, wait 3+ days, install v_current on a fresh device, cold-start, and confirm the + snackbar fires once the threshold is met. Noted on the PR as a known limitation, not as a + shipped guarantee. + +## Acceptance criteria + +A change is done when: + +1. Building a release AAB with a higher `versionCode`, uploading to internal testing, and + cold-starting the previous build (after ≥ 3 days of staleness, or with the threshold + temporarily set to 0) surfaces the styled snackbar with **"Update ready — restart to install"** + and a **Restart** action. +2. Tapping **Restart** triggers the Play install flow and the app relaunches on the new build. +3. A debug install via `adb install` shows no snackbar at any point, with no exceptions in logcat. +4. `OpenLoopUiState` is unchanged. No new routed state, no `when` branch added to `OpenLoopNavHost`. +5. The `DEFINITION_OF_DONE.md` gate is clean: debug + release build green, lint clean (or + baselined), instrumented + unit tests pass, app launched on emulator with screenshot attached + to the PR. + +## Open questions + +- **Threshold value.** 3 days is a guess. We can ship at 3, watch a release cycle, and tune. Worth + a `BuildConfig` field? Probably not — change-and-recompile is fine for a single integer that + doesn't differ per build type. (The bypass *is* a `BuildConfig` field — see above.) diff --git a/gradle/libs.versions.toml b/gradle/libs.versions.toml index 8f5b7f9..7f2eeea 100644 --- a/gradle/libs.versions.toml +++ b/gradle/libs.versions.toml @@ -22,6 +22,7 @@ googleServices = "4.4.4" firebaseCrashlyticsGradle = "3.0.7" baselineprofile = "1.5.0-alpha06" benchmark = "1.5.0-alpha06" +playAppUpdate = "2.1.0" [libraries] @@ -79,6 +80,10 @@ firebase-analytics = { group = "com.google.firebase", name = "firebase-analytics androidx-benchmark-macro-junit4 = { group = "androidx.benchmark", name = "benchmark-macro-junit4", version.ref = "benchmark" } androidx-test-uiautomator = { group = "androidx.test.uiautomator", name = "uiautomator", version = "2.3.0" } +# Play In-App Updates (docs/active/in-app-updates/IMPLEMENTATION.md) — official Google library +# for checking and prompting Play-Store update availability. -ktx adds the suspend/flow helpers. +play-app-update-ktx = { group = "com.google.android.play", name = "app-update-ktx", version.ref = "playAppUpdate" } + [plugins] android-application = { id = "com.android.application", version.ref = "agp" } From 98ffc4ab19bb27614de572f148eb764f4a5af88e Mon Sep 17 00:00:00 2001 From: Steven Gates Date: Mon, 15 Jun 2026 15:06:35 -0500 Subject: [PATCH 2/2] fix(tests): unblock pre-existing test failures so the DoD gate passes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four test failures on main were blocking unit test compilation/runs. None were caused by this branch — all were left behind when prior feature commits changed semantics. Folded inline so this PR can clear the DoD test gate. 1. DeviceMediaHintsTest + SamsungReversePreviewRegressionTest referenced SAMSUNG_POST_TRANSFORM_CODEC_SETTLE_MS / PRE_REVERSE_CODEC_SETTLE_MS, which commit 4eed9b6 renamed to Duration-typed constants without the _MS suffix. Mechanical update: import kotlin.time.Duration.Companion.milliseconds, compare Duration values directly. Without this the whole test source set failed to compile, blocking every other test. 2. OpenLoopViewModelTest `reverse generation failure flags reverseFailed` polled with virtual-time delay(10) inside withTimeout(5_000), but ensureReversedSegment runs on real Dispatchers.IO. Virtual time burned through the 5s budget in microseconds before the real IO thread could dispatch its continuation. Switched to awaitReversePreviewFailedFallback() — the codebase's existing real-time spin helper, used by 13 other tests in the same file. 3. OpenLoopViewModelTest `saveBoomerang ... emitting Share` asserted both RAW and BOOMERANG entries remained in storage after save. Commit 7d202bf ("Delete source video") changed the render flow to delete the source raw after rendering the boomerang. Updated assertions to match: exactly one BOOMERANG, zero RAW, boomerang's sourceRawId still references its source. Full suite: 206/206 passing. Co-Authored-By: Claude Opus 4.7 --- .../openloop/media/DeviceMediaHintsTest.kt | 5 +++-- .../SamsungReversePreviewRegressionTest.kt | 3 ++- .../openloop/ui/OpenLoopViewModelTest.kt | 22 +++++++++++-------- 3 files changed, 18 insertions(+), 12 deletions(-) diff --git a/app/src/test/java/io/github/stozo04/openloop/media/DeviceMediaHintsTest.kt b/app/src/test/java/io/github/stozo04/openloop/media/DeviceMediaHintsTest.kt index 5d9dc68..ea707a3 100644 --- a/app/src/test/java/io/github/stozo04/openloop/media/DeviceMediaHintsTest.kt +++ b/app/src/test/java/io/github/stozo04/openloop/media/DeviceMediaHintsTest.kt @@ -3,14 +3,15 @@ package io.github.stozo04.openloop.media import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test +import kotlin.time.Duration.Companion.milliseconds class DeviceMediaHintsTest { @Test fun samsungPreviewReverseCap_isBelowExportCap() { assertEquals(480, SAMSUNG_PREVIEW_REVERSE_MAX_SHORT_SIDE) - assertEquals(500L, SAMSUNG_POST_TRANSFORM_CODEC_SETTLE_MS) - assertTrue(PRE_REVERSE_CODEC_SETTLE_MS >= 200L) + assertEquals(500.milliseconds, SAMSUNG_POST_TRANSFORM_CODEC_SETTLE) + assertTrue(PRE_REVERSE_CODEC_SETTLE >= 200.milliseconds) assert(SAMSUNG_PREVIEW_REVERSE_MAX_SHORT_SIDE < MAX_OUTPUT_SHORT_SIDE) } } diff --git a/app/src/test/java/io/github/stozo04/openloop/media/SamsungReversePreviewRegressionTest.kt b/app/src/test/java/io/github/stozo04/openloop/media/SamsungReversePreviewRegressionTest.kt index 0cf69e5..070a19f 100644 --- a/app/src/test/java/io/github/stozo04/openloop/media/SamsungReversePreviewRegressionTest.kt +++ b/app/src/test/java/io/github/stozo04/openloop/media/SamsungReversePreviewRegressionTest.kt @@ -4,6 +4,7 @@ import android.os.Build import org.junit.Assert.assertEquals import org.junit.Assert.assertTrue import org.junit.Test +import kotlin.time.Duration.Companion.milliseconds /** RTL device API (SM-S921B); JVM unit tests default to SDK 0 without this. */ private const val RTL_SDK_INT = Build.VERSION_CODES.BAKLAVA @@ -33,7 +34,7 @@ class SamsungReversePreviewRegressionTest { /** Log: Transformer Release 28.303 → reverse start 28.324 — settle must be non-zero. */ @Test fun postTransformSettleMs_isPositive() { - assertTrue(SAMSUNG_POST_TRANSFORM_CODEC_SETTLE_MS >= 200L) + assertTrue(SAMSUNG_POST_TRANSFORM_CODEC_SETTLE >= 200.milliseconds) } /** diff --git a/app/src/test/java/io/github/stozo04/openloop/ui/OpenLoopViewModelTest.kt b/app/src/test/java/io/github/stozo04/openloop/ui/OpenLoopViewModelTest.kt index 117c5f5..6be72ae 100644 --- a/app/src/test/java/io/github/stozo04/openloop/ui/OpenLoopViewModelTest.kt +++ b/app/src/test/java/io/github/stozo04/openloop/ui/OpenLoopViewModelTest.kt @@ -889,12 +889,10 @@ class OpenLoopViewModelTest { fakeVideoProcessor.failReverse = true viewModel.onNextFromTrim() // default FORWARD_THEN_REVERSE → reverse generation runs + fails - // Reverse runs on Dispatchers.IO (real pool in JVM tests) — poll until the UI updates. - withTimeout(5_000) { - while (viewModel.editorTabState.value.previewLoading != null) { - delay(10) - } - } + // Reverse runs on real Dispatchers.IO (see ensureReversedSegment) — virtual-time delay() + // burns through the polling budget before the IO thread can dispatch. Use the same + // real-time spin helper the rest of the suite uses for reverse-failure fallback. + awaitReversePreviewFailedFallback() val tab = viewModel.editorTabState.value assertFalse("user can continue with forward-only preview", tab.reverseFailed) @@ -1063,10 +1061,16 @@ class OpenLoopViewModelTest { assertEquals(1, fakeRenderScheduler.enqueueCount) assertEquals(1, fakeVideoProcessor.renderCount) - // One RAW (promoted) + one BOOMERANG (registered), boomerang points at the raw. - val raw = fakeVideoStorage.saved.single { it.kind == VideoKind.RAW } + // The boomerang is registered and carries sourceRawId pointing at the raw it came from; + // the raw itself is deleted after rendering (commit 7d202bf "Delete source video" — + // the boomerang is the final artifact, the raw is no longer kept). So we should see + // exactly one BOOMERANG and zero RAW entries after a successful save. val boomerang = fakeVideoStorage.saved.single { it.kind == VideoKind.BOOMERANG } - assertEquals(raw.id, boomerang.sourceRawId) + assertTrue("boomerang must reference its source raw", (boomerang.sourceRawId ?: 0L) > 0L) + assertTrue( + "raw must be deleted after the boomerang is rendered", + fakeVideoStorage.saved.none { it.kind == VideoKind.RAW }, + ) assertEquals(1, fakeVideoStorage.discardedScratches.size) // scratch cleaned up after save assertNull(viewModel.editorState.value)