diff --git a/android/app/src/test/java/com/fabledsword/minstrel/shared/UpdateVeilControllerTest.kt b/android/app/src/test/java/com/fabledsword/minstrel/shared/UpdateVeilControllerTest.kt index e509b53d..e57027f9 100644 --- a/android/app/src/test/java/com/fabledsword/minstrel/shared/UpdateVeilControllerTest.kt +++ b/android/app/src/test/java/com/fabledsword/minstrel/shared/UpdateVeilControllerTest.kt @@ -7,7 +7,6 @@ import kotlinx.coroutines.flow.map import kotlinx.coroutines.launch import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.advanceTimeBy -import kotlinx.coroutines.test.advanceUntilIdle import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runTest import org.junit.jupiter.api.Test @@ -22,6 +21,13 @@ private const val ART_IN_FLIGHT = 3 private const val SUCCEED_ON_ATTEMPT = 3 private const val QUIET_WINDOWS_TO_OUTLAST = 3 +// Time to let a session finish once the screen has stopped changing: the +// retry backoffs, the quiet window and the minimum hold all fit inside it, +// while staying well under maxHoldMs. That gap matters — if a drain ran +// past the ceiling, "the veil lowered" would no longer distinguish +// "it settled" from "it gave up", which is the whole point of these tests. +private const val DRAIN_MS = 3_000L + /** * The veil's job is to stay up until the screen has stopped moving. Each * test pins one of the ways the previous fixed-delay implementation @@ -30,6 +36,14 @@ private const val QUIET_WINDOWS_TO_OUTLAST = 3 * The controller is built on `backgroundScope` throughout: its consumer * loop runs forever, so hanging it off the test's own scope would stop * `runTest` from ever completing. + * + * Consequence, and the reason every wait below is an explicit + * `advanceTimeBy`: **`advanceUntilIdle()` is useless here.** It advances + * only while *foreground* work remains, and everything this controller + * does lives in `backgroundScope` — so it returns having run nothing, and + * assertions land on a session that never started (CI run 3163 failed all + * seven of these with "expected 3, actual 0" and friends). Drive the clock + * deliberately instead; don't "simplify" these back to advanceUntilIdle. */ @OptIn(ExperimentalCoroutinesApi::class) class UpdateVeilControllerTest { @@ -101,7 +115,7 @@ class UpdateVeilControllerTest { assertTrue(controller.visible.value, "veil must hold across staged churn") } - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertFalse(controller.visible.value, "veil lowers once the screen goes quiet") } @@ -118,7 +132,7 @@ class UpdateVeilControllerTest { assertTrue(controller.visible.value, "veil must wait on in-flight art") screen.artLoading(0) - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertFalse(controller.visible.value, "veil lowers once art has landed") } @@ -138,7 +152,7 @@ class UpdateVeilControllerTest { advanceTimeBy(timings.retryBackoffMs + 1) assertTrue(controller.visible.value, "veil stays up across the backoff") - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertEquals(SUCCEED_ON_ATTEMPT, attempts, "retries until the pull succeeds") assertEquals(listOf(false, true, false), seen, "raised once, lowered once") } @@ -154,7 +168,7 @@ class UpdateVeilControllerTest { } controller.request() - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertEquals(timings.attempts, attempts, "exhausts its attempts") assertFalse(controller.visible.value, "gives up silently") @@ -162,7 +176,7 @@ class UpdateVeilControllerTest { // — the reconnect-driven recovery still gets to try again later. succeed = true controller.request() - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertEquals(timings.attempts + 1, attempts, "a later request still runs") } @@ -186,7 +200,7 @@ class UpdateVeilControllerTest { advanceTimeBy(WORK_MS + 1) assertTrue(controller.visible.value, "second trigger extends the same veil") - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertEquals(2, started, "the mid-session trigger still did its pull") assertEquals(listOf(false, true, false), seen, "one veil session, not two") } @@ -215,7 +229,7 @@ class UpdateVeilControllerTest { } controller.request() - advanceUntilIdle() + advanceTimeBy(DRAIN_MS) assertTrue(ran, "the refresh still happens") assertFalse(controller.visible.value, "but no veil over a skeleton") } @@ -236,7 +250,7 @@ class UpdateVeilControllerTest { runCurrent() assertTrue(controller.visible.value, "raises the moment cached content paints") - advanceUntilIdle() + advanceTimeBy(SLOW_WORK_MS + DRAIN_MS) assertFalse(controller.visible.value) } }