diff --git a/android/app/src/main/java/com/noop/ui/TodayScoring.kt b/android/app/src/main/java/com/noop/ui/TodayScoring.kt index afd0a85da6..b638e07bf5 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScoring.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScoring.kt @@ -6,7 +6,6 @@ import com.noop.analytics.ChargeDriver import com.noop.analytics.RecoveryDrivers import com.noop.analytics.RestScorer import com.noop.analytics.ScoreConfidence -import com.noop.R import com.noop.data.DailyMetric import java.time.LocalDate import java.time.Instant @@ -388,15 +387,29 @@ sealed class SyncChipState { companion object { /** Pure + unit-tested. Mirrors Swift `SyncChipState.resolve` exactly: backfilling wins over a - * known last-sync, which wins over the 5/MG experimental fallback. */ + * known last-sync, which wins over the 5/MG experimental fallback. + * + * [nowSec] (unix seconds) and [nowLabel] (the already-translated "now" word) are PARAMETERS + * rather than things this function reaches for, which is what keeps "pure" true. Resolving the + * word in here instead cost the twin its own test: it goes through `NoopApplication`, which + * throws `IllegalStateException: NoopApplication is not attached` under a plain JVM unit test — + * these run without Robolectric, so no Application is ever attached. Only the `< 60s` branch of + * [shortSyncAgo] wants a word, so the failure was invisible until a case landed in it. Same + * injected-clock style as [recordingStateFor] just above. + * + * Swift's twin keeps both inside its own `shortAgo` and is fine there — XCTest runs against a + * real bundle, so `String(localized:)` resolves. The two SIGNATURES therefore differ on purpose; + * the decision they encode does not. Don't "restore parity" by moving the lookup back in here. */ fun resolve( backfilling: Boolean, chunks: Int, lastSyncAtSec: Long?, historySyncExperimental: Boolean, + nowSec: Long, + nowLabel: String, ): SyncChipState = when { backfilling -> Syncing(chunks) - lastSyncAtSec != null -> Synced(shortSyncAgo(lastSyncAtSec)) + lastSyncAtSec != null -> Synced(shortSyncAgo(lastSyncAtSec, nowSec, nowLabel)) historySyncExperimental -> ExperimentalLive else -> Hidden } @@ -405,11 +418,13 @@ sealed class SyncChipState { /** Compact relative age for the header chip ("now" / "Nm" / "Nh" / "Nd") from a unix-SECONDS timestamp — * deliberately terse. "now" is the only word in here (the rest is digits + a unit letter), so it's the - * only piece that needs a catalog entry to translate. Mirrors the iOS `SyncChipState.shortAgo`. */ -internal fun shortSyncAgo(unixSec: Long): String { - val secs = (System.currentTimeMillis() / 1000L - unixSec).coerceAtLeast(0) + * only piece that needs a catalog entry to translate; it arrives as [nowLabel], resolved by the + * composable that owns the chip, so the bucketing stays framework-free. [nowSec] is unix seconds, + * injected for the same reason. Mirrors the iOS `SyncChipState.shortAgo`. */ +internal fun shortSyncAgo(unixSec: Long, nowSec: Long, nowLabel: String): String { + val secs = (nowSec - unixSec).coerceAtLeast(0) return when { - secs < 60 -> uiString(R.string.l10n_today_screen_sync_chip_now_c9bc849a) + secs < 60 -> nowLabel secs < 3600 -> "${secs / 60}m" secs < 86_400 -> "${secs / 3600}h" else -> "${secs / 86_400}d" diff --git a/android/app/src/main/java/com/noop/ui/TodayScreen.kt b/android/app/src/main/java/com/noop/ui/TodayScreen.kt index 9e14e2c345..deb0490d58 100644 --- a/android/app/src/main/java/com/noop/ui/TodayScreen.kt +++ b/android/app/src/main/java/com/noop/ui/TodayScreen.kt @@ -2132,7 +2132,19 @@ private fun SyncStatusChip( lastSyncAt: Long?, historySyncExperimental: Boolean, ) { - when (val state = SyncChipState.resolve(backfilling, chunks, lastSyncAt, historySyncExperimental)) { + // The clock and the translated "now" word are resolved HERE, in the composable that already depends + // on both, and handed down — so `SyncChipState.resolve` stays a genuinely pure decision that a plain + // JVM unit test can call with no attached Application. Reading the clock at composition time (rather + // than snapshotting it) is unchanged behaviour: `shortSyncAgo` did exactly this on every recomposition. + val state = SyncChipState.resolve( + backfilling = backfilling, + chunks = chunks, + lastSyncAtSec = lastSyncAt, + historySyncExperimental = historySyncExperimental, + nowSec = System.currentTimeMillis() / 1000L, + nowLabel = uiString(R.string.l10n_today_screen_sync_chip_now_c9bc849a), + ) + when (state) { is SyncChipState.Syncing -> ChipCapsule( Icons.Filled.Autorenew, "${state.chunks}", Palette.accent, uiString(R.string.l10n_today_screen_sync_chip_syncing_desc_bfc290e7, state.chunks)) diff --git a/android/app/src/test/java/com/noop/ui/SyncChipStateTest.kt b/android/app/src/test/java/com/noop/ui/SyncChipStateTest.kt index aef8fd9622..b623951bfc 100644 --- a/android/app/src/test/java/com/noop/ui/SyncChipStateTest.kt +++ b/android/app/src/test/java/com/noop/ui/SyncChipStateTest.kt @@ -8,22 +8,37 @@ import org.junit.Test * #245: `SyncChipState.resolve` is the one place the Today top bar's `SyncStatusChip` decides which of * the four sync states to show. Mirrors the iOS `SyncChipStateTests` 1:1 — same priority order (backfilling * wins over a known last-sync, which wins over the 5/MG experimental fallback), same cold-start `Hidden` case. + * + * Every case pins [NOW] instead of reading the system clock, and passes the "now" word in as [NOW_LABEL] + * instead of letting `resolve` resolve it: these are plain JVM tests with no Robolectric, so there is no + * attached `NoopApplication` to read the string catalog from. That is what made the `< 60s` cases below + * throw `IllegalStateException: NoopApplication is not attached` while every other case passed — only the + * sub-minute branch of `shortSyncAgo` needs a translated word. */ class SyncChipStateTest { + private companion object { + /** Any fixed instant works now that `resolve` takes its clock as an argument. */ + const val NOW = 1_700_000_000L + + /** Stand-in for the `l10n_today_screen_sync_chip_now_*` catalog entry the composable passes in. */ + const val NOW_LABEL = "now" + } + @Test fun backfilling_isSyncingWithChunkCount() { val state = SyncChipState.resolve( backfilling = true, chunks = 7, lastSyncAtSec = null, historySyncExperimental = false, + nowSec = NOW, nowLabel = NOW_LABEL, ) assertEquals(SyncChipState.Syncing(7), state) } @Test fun lastSyncedAt_isSyncedWithAgeText() { - val now = System.currentTimeMillis() / 1000L val state = SyncChipState.resolve( - backfilling = false, chunks = 0, lastSyncAtSec = now - 65, historySyncExperimental = false, + backfilling = false, chunks = 0, lastSyncAtSec = NOW - 65, historySyncExperimental = false, + nowSec = NOW, nowLabel = NOW_LABEL, ) assertEquals(SyncChipState.Synced("1m"), state) } @@ -32,6 +47,7 @@ class SyncChipStateTest { fun historySyncExperimental_withNoLastSync_isExperimentalLive() { val state = SyncChipState.resolve( backfilling = false, chunks = 0, lastSyncAtSec = null, historySyncExperimental = true, + nowSec = NOW, nowLabel = NOW_LABEL, ) assertEquals(SyncChipState.ExperimentalLive, state) } @@ -40,25 +56,42 @@ class SyncChipStateTest { fun coldStart_noBackfillNoSyncNoExperimental_isHidden() { val state = SyncChipState.resolve( backfilling = false, chunks = 0, lastSyncAtSec = null, historySyncExperimental = false, + nowSec = NOW, nowLabel = NOW_LABEL, ) assertEquals(SyncChipState.Hidden, state) } @Test fun backfilling_takesPriorityOverLastSyncedAt() { - val now = System.currentTimeMillis() / 1000L val state = SyncChipState.resolve( - backfilling = true, chunks = 2, lastSyncAtSec = now - 5, historySyncExperimental = false, + backfilling = true, chunks = 2, lastSyncAtSec = NOW - 5, historySyncExperimental = false, + nowSec = NOW, nowLabel = NOW_LABEL, ) assertEquals(SyncChipState.Syncing(2), state) } @Test fun lastSyncedAt_takesPriorityOverHistorySyncExperimental() { - val now = System.currentTimeMillis() / 1000L val state = SyncChipState.resolve( - backfilling = false, chunks = 0, lastSyncAtSec = now - 5, historySyncExperimental = true, + backfilling = false, chunks = 0, lastSyncAtSec = NOW - 5, historySyncExperimental = true, + nowSec = NOW, nowLabel = NOW_LABEL, ) assertTrue("A known last-sync should win over the experimental fallback", state is SyncChipState.Synced) } + + /** + * Regression guard for the branch that used to be unreachable from a unit test: a sub-minute sync must + * render the caller-supplied word verbatim. This fails to even compile — let alone pass — if the string + * lookup ever moves back inside `shortSyncAgo`, which is the whole point of keeping it out. No iOS twin: + * Swift's `shortAgo` resolves its own clock and `String(localized:)`, and can afford to because XCTest + * runs against a real bundle. + */ + @Test + fun lastSyncedUnderAMinute_isSyncedWithTheInjectedNowLabel() { + val state = SyncChipState.resolve( + backfilling = false, chunks = 0, lastSyncAtSec = NOW - 5, historySyncExperimental = false, + nowSec = NOW, nowLabel = NOW_LABEL, + ) + assertEquals(SyncChipState.Synced("now"), state) + } }