Home updating-veil rework: change-triggered, settle-driven, with refresh feedback #114

Merged
bvandeusen merged 4 commits from dev into main 2026-07-31 23:32:06 -04:00
Showing only changes of commit d3b40342b4 - Show all commits
@@ -7,7 +7,6 @@ import kotlinx.coroutines.flow.map
import kotlinx.coroutines.launch import kotlinx.coroutines.launch
import kotlinx.coroutines.test.TestScope import kotlinx.coroutines.test.TestScope
import kotlinx.coroutines.test.advanceTimeBy import kotlinx.coroutines.test.advanceTimeBy
import kotlinx.coroutines.test.advanceUntilIdle
import kotlinx.coroutines.test.runCurrent import kotlinx.coroutines.test.runCurrent
import kotlinx.coroutines.test.runTest import kotlinx.coroutines.test.runTest
import org.junit.jupiter.api.Test 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 SUCCEED_ON_ATTEMPT = 3
private const val QUIET_WINDOWS_TO_OUTLAST = 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 * 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 * 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 * The controller is built on `backgroundScope` throughout: its consumer
* loop runs forever, so hanging it off the test's own scope would stop * loop runs forever, so hanging it off the test's own scope would stop
* `runTest` from ever completing. * `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) @OptIn(ExperimentalCoroutinesApi::class)
class UpdateVeilControllerTest { class UpdateVeilControllerTest {
@@ -101,7 +115,7 @@ class UpdateVeilControllerTest {
assertTrue(controller.visible.value, "veil must hold across staged churn") 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") 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") assertTrue(controller.visible.value, "veil must wait on in-flight art")
screen.artLoading(0) screen.artLoading(0)
advanceUntilIdle() advanceTimeBy(DRAIN_MS)
assertFalse(controller.visible.value, "veil lowers once art has landed") assertFalse(controller.visible.value, "veil lowers once art has landed")
} }
@@ -138,7 +152,7 @@ class UpdateVeilControllerTest {
advanceTimeBy(timings.retryBackoffMs + 1) advanceTimeBy(timings.retryBackoffMs + 1)
assertTrue(controller.visible.value, "veil stays up across the backoff") 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(SUCCEED_ON_ATTEMPT, attempts, "retries until the pull succeeds")
assertEquals(listOf(false, true, false), seen, "raised once, lowered once") assertEquals(listOf(false, true, false), seen, "raised once, lowered once")
} }
@@ -154,7 +168,7 @@ class UpdateVeilControllerTest {
} }
controller.request() controller.request()
advanceUntilIdle() advanceTimeBy(DRAIN_MS)
assertEquals(timings.attempts, attempts, "exhausts its attempts") assertEquals(timings.attempts, attempts, "exhausts its attempts")
assertFalse(controller.visible.value, "gives up silently") assertFalse(controller.visible.value, "gives up silently")
@@ -162,7 +176,7 @@ class UpdateVeilControllerTest {
// — the reconnect-driven recovery still gets to try again later. // — the reconnect-driven recovery still gets to try again later.
succeed = true succeed = true
controller.request() controller.request()
advanceUntilIdle() advanceTimeBy(DRAIN_MS)
assertEquals(timings.attempts + 1, attempts, "a later request still runs") assertEquals(timings.attempts + 1, attempts, "a later request still runs")
} }
@@ -186,7 +200,7 @@ class UpdateVeilControllerTest {
advanceTimeBy(WORK_MS + 1) advanceTimeBy(WORK_MS + 1)
assertTrue(controller.visible.value, "second trigger extends the same veil") 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(2, started, "the mid-session trigger still did its pull")
assertEquals(listOf(false, true, false), seen, "one veil session, not two") assertEquals(listOf(false, true, false), seen, "one veil session, not two")
} }
@@ -215,7 +229,7 @@ class UpdateVeilControllerTest {
} }
controller.request() controller.request()
advanceUntilIdle() advanceTimeBy(DRAIN_MS)
assertTrue(ran, "the refresh still happens") assertTrue(ran, "the refresh still happens")
assertFalse(controller.visible.value, "but no veil over a skeleton") assertFalse(controller.visible.value, "but no veil over a skeleton")
} }
@@ -236,7 +250,7 @@ class UpdateVeilControllerTest {
runCurrent() runCurrent()
assertTrue(controller.visible.value, "raises the moment cached content paints") assertTrue(controller.visible.value, "raises the moment cached content paints")
advanceUntilIdle() advanceTimeBy(SLOW_WORK_MS + DRAIN_MS)
assertFalse(controller.visible.value) assertFalse(controller.visible.value)
} }
} }