mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 11:07:10 +00:00
fix(android): wait out a route cross-fade before snapshotting
the dump could hold two screens at once, and the runner refuses to act on such a tree, so a quarter of android steps applied no action and the count varied per run: the same seed never walked the same trajectory. the ios companion and the chrome driver already do this.
This commit is contained in:
1 parent
05d65841b7
commit
129bbc7a62
3 files changed
+220
-35
No files matched your search
@@ -63,35 +63,97 @@ internal const val STABILITY_POLL_CAP_MILLIS = 2000L
|
||||
// missed.
|
||||
internal const val MIN_STABLE_STREAK_MILLIS = 750L
|
||||
|
||||
// TRANSITION_* bound the wait a snapshot pays when the tree it read shows two
|
||||
// routes at once, which on Compose means a NavHost cross-fade is in flight.
|
||||
// They are shorter than the constants above because this wait has one job,
|
||||
// outlasting the fade, where the poll above must also catch an async effect
|
||||
// that fires late.
|
||||
//
|
||||
// The streak matches the iOS companion's 300ms for the same reason: what this
|
||||
// wait is for is the cross-fade, and the cross-fade is caught by the
|
||||
// route-screen check rather than by streak length, so the streak only has to
|
||||
// be long enough that the reads spanning it are not all inside one frame.
|
||||
// MIN_STABLE_STREAK's 750ms is sized for a poll that must also catch an async
|
||||
// effect firing late, and at ~160ms per read-and-interval it costs three or
|
||||
// four more reads than this does.
|
||||
internal const val TRANSITION_STABLE_STREAK_MILLIS = 300L
|
||||
|
||||
// The interval is the iOS companion's rather than STABILITY_POLL_INTERVAL's
|
||||
// 250ms: the wide interval exists to stop a per-step poll hammering
|
||||
// UiAutomation, and this one runs only on the frames that show two routes,
|
||||
// about a quarter of steps on folio. The read itself paces the loop.
|
||||
internal const val TRANSITION_POLL_INTERVAL_MILLIS = 100L
|
||||
|
||||
// The cap is the iOS companion's 1500ms, and it has to be at least this: the
|
||||
// NavHost cross-fade is a 700ms tween (Compose navigation's default enter and
|
||||
// exit), it starts when the action lands rather than when the snapshot begins,
|
||||
// and the streak above has to fit after it. Measured on the emulator, a 600ms
|
||||
// cap left about a third of fades unfinished. A layout that holds two routes at
|
||||
// rest costs the full cap once per step and no more.
|
||||
internal const val TRANSITION_POLL_CAP_MILLIS = 1500L
|
||||
|
||||
// awaitSettledTree reads the hierarchy and, while the tree it gets back holds
|
||||
// more than one route, keeps reading until the cross-fade lands or the cap
|
||||
// expires. It returns the last tree read, so the caller gets the settled one
|
||||
// rather than paying for another read.
|
||||
//
|
||||
// Two nodes carrying the SAME route id are one destination, not a transition;
|
||||
// countRouteScreens counts distinct tags, so a screen that nests a repeat of
|
||||
// its own id does not pay this wait at all.
|
||||
internal fun awaitSettledTree(read: () -> String): String {
|
||||
var json = read()
|
||||
if (countRouteScreens(json) <= 1) return json
|
||||
pollUntilStable(
|
||||
TRANSITION_POLL_CAP_MILLIS,
|
||||
TRANSITION_STABLE_STREAK_MILLIS,
|
||||
TRANSITION_POLL_INTERVAL_MILLIS,
|
||||
) {
|
||||
json = read()
|
||||
stabilitySnapshot(json)
|
||||
}
|
||||
return json
|
||||
}
|
||||
|
||||
// pollUntilStable returns when the snapshot has been non-null and equal to
|
||||
// itself for an uninterrupted stretch of at least MIN_STABLE_STREAK_MILLIS,
|
||||
// capped at timeoutMillis. snapshot must omit transient attributes (e.g.
|
||||
// measure-pass bounds) so layout-only flicker doesn't extend the wait, and
|
||||
// must return null when the snapshot looks transitional (e.g. mid NavHost
|
||||
// cross-fade) so the streak resets and the loop keeps polling instead of
|
||||
// declaring a partial state stable.
|
||||
internal fun pollUntilStable(timeoutMillis: Long, snapshot: () -> String?) {
|
||||
// itself for an uninterrupted stretch of at least streakMillis, capped at
|
||||
// timeoutMillis. snapshot must omit transient attributes (e.g. measure-pass
|
||||
// bounds) so layout-only flicker doesn't extend the wait, and must return null
|
||||
// when the snapshot looks transitional (e.g. mid NavHost cross-fade) so the
|
||||
// streak resets and the loop keeps polling instead of declaring a partial
|
||||
// state stable.
|
||||
//
|
||||
// The streak is measured from the start of the read that opened the current
|
||||
// run of identical snapshots, not from when that read returned: a hierarchy
|
||||
// fetch is not instantaneous, and the UI changing mid-fetch would have changed
|
||||
// the snapshot, so the fetch's own duration is evidence of stability. On
|
||||
// Android a fetch costs more than the poll interval, so charging it to the
|
||||
// streak is the difference between two reads and four.
|
||||
internal fun pollUntilStable(
|
||||
timeoutMillis: Long,
|
||||
streakMillis: Long = MIN_STABLE_STREAK_MILLIS,
|
||||
intervalMillis: Long = STABILITY_POLL_INTERVAL_MILLIS,
|
||||
snapshot: () -> String?,
|
||||
) {
|
||||
if (timeoutMillis <= 0) return
|
||||
val deadline = System.currentTimeMillis() + timeoutMillis
|
||||
var runStart = System.currentTimeMillis()
|
||||
var prior = try {
|
||||
snapshot()
|
||||
} catch (_: Exception) {
|
||||
null
|
||||
}
|
||||
var streakStart = 0L
|
||||
while (System.currentTimeMillis() < deadline) {
|
||||
Thread.sleep(STABILITY_POLL_INTERVAL_MILLIS)
|
||||
Thread.sleep(intervalMillis)
|
||||
val currentStart = System.currentTimeMillis()
|
||||
val current = try {
|
||||
snapshot()
|
||||
} catch (_: Exception) {
|
||||
null
|
||||
}
|
||||
val now = System.currentTimeMillis()
|
||||
if (prior != null && current != null && prior == current) {
|
||||
if (streakStart == 0L) streakStart = now
|
||||
if (now - streakStart >= MIN_STABLE_STREAK_MILLIS) return
|
||||
if (System.currentTimeMillis() - runStart >= streakMillis) return
|
||||
} else {
|
||||
streakStart = 0L
|
||||
runStart = currentStart
|
||||
}
|
||||
prior = current
|
||||
}
|
||||
@@ -119,36 +181,43 @@ private val ROUTE_TAG_KEYS = setOf(
|
||||
|
||||
private val jsonMapper = com.fasterxml.jackson.module.kotlin.jacksonObjectMapper()
|
||||
|
||||
// countRouteScreens counts DISTINCT route-level destination tags, not the nodes
|
||||
// carrying them. A screen that nests a node repeating its own route id puts two
|
||||
// tagged nodes in the tree while only one destination is on screen; counting
|
||||
// nodes would read that as a cross-fade that never ends, and the caller would
|
||||
// pay its full poll budget on every step of that screen and never settle.
|
||||
internal fun countRouteScreens(treeJson: String): Int {
|
||||
if (treeJson.isBlank()) return 0
|
||||
return try {
|
||||
val root = jsonMapper.readTree(treeJson)
|
||||
countRouteScreens(root)
|
||||
val tags = mutableSetOf<String>()
|
||||
collectRouteScreens(root, tags)
|
||||
tags.size
|
||||
} catch (_: Exception) {
|
||||
0
|
||||
}
|
||||
}
|
||||
|
||||
private fun countRouteScreens(
|
||||
private fun collectRouteScreens(
|
||||
node: com.fasterxml.jackson.databind.JsonNode,
|
||||
): Int {
|
||||
var count = 0
|
||||
into: MutableSet<String>,
|
||||
) {
|
||||
val attributes = node.get("attributes")
|
||||
if (attributes != null && attributes.isObject) {
|
||||
for (key in ROUTE_TAG_KEYS) {
|
||||
val value = attributes.get(key) ?: continue
|
||||
if (value.isNull) continue
|
||||
if (value.asText().endsWith("Screen")) {
|
||||
count++
|
||||
val text = value.asText()
|
||||
if (text.endsWith("Screen")) {
|
||||
into.add(text)
|
||||
break
|
||||
}
|
||||
}
|
||||
}
|
||||
val children = node.get("children")
|
||||
if (children != null && children.isArray) {
|
||||
for (child in children) count += countRouteScreens(child)
|
||||
for (child in children) collectRouteScreens(child, into)
|
||||
}
|
||||
return count
|
||||
}
|
||||
|
||||
// structuralHash hashes a Maestro TreeNode-shaped JSON string by walking it
|
||||
@@ -873,13 +942,29 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
|
||||
override fun recentLogs(sinceUnixMillis: Long, minLevel: String) =
|
||||
readLogcat(serial, sinceUnixMillis, minLevel)
|
||||
|
||||
// snapshot waits out a NavHost cross-fade before it reads, so the runner is
|
||||
// never handed a tree holding two routes at once. It belongs here rather
|
||||
// than in waitForIdle: the runner gives waitForIdle a one-second deadline
|
||||
// and abandons the RPC when it expires, which is not enough room for a
|
||||
// 700ms fade that began before the settle did, and a wait that outlives the
|
||||
// deadline just races the runner's own fetch on the device-side server.
|
||||
// The snapshot RPC carries the step's deadline instead, so the wait can run
|
||||
// to a bound that actually covers the animation.
|
||||
//
|
||||
// The predicate costs nothing on a settled frame: the read it needs is the
|
||||
// read the snapshot was going to do anyway. That is what makes this
|
||||
// affordable, where the structural poll that used to run in waitForIdle was
|
||||
// not: it fetched the hierarchy ~4 more times on every mutating step.
|
||||
override fun snapshot(): SnapshotSample =
|
||||
SnapshotSample(awaitSettledTree { hierarchy() }, screenshot())
|
||||
|
||||
override fun waitForIdle(durationMillis: Long) {
|
||||
// waitForAppToSettle blocks on the View-system animation and maestro's
|
||||
// own structural settle, which is enough on its own. A follow-up
|
||||
// structural-hash poll used to run here, but each hierarchy fetch is
|
||||
// ~500ms on a physical device, so it cost ~2.8s per mutating step for
|
||||
// marginal benefit; the runner already re-fetches while a frame still
|
||||
// looks transitional.
|
||||
// own structural settle. It cannot see a Compose cross-fade: the fade
|
||||
// keeps both routes alive with the tree byte-identical, so a settle
|
||||
// that watches for change returns in the middle of one. That is what
|
||||
// snapshot above waits out; a structural poll here used to try, cost
|
||||
// ~2.8s per mutating step, and was removed.
|
||||
driver.waitForAppToSettle(null, null, durationMillis.toInt())
|
||||
}
|
||||
|
||||
|
||||
@@ -0,0 +1,95 @@
|
||||
package dev.sanderling.sidecar
|
||||
|
||||
import org.junit.Test
|
||||
import kotlin.test.assertEquals
|
||||
import kotlin.test.assertTrue
|
||||
|
||||
// The Android backend's snapshot waits out a NavHost cross-fade before it
|
||||
// reads. Without that wait the runner is handed a tree holding two routes at
|
||||
// once, refuses to act on it, and the step applies nothing; how many steps land
|
||||
// that way varies run to run, so the same seed walks a different trajectory
|
||||
// each time. These cover the wait (awaitSettledTree) and what it costs.
|
||||
class RouteTransitionTest {
|
||||
private fun screen(id: String, child: String = "") =
|
||||
"""{"attributes":{"resource-id":"$id"},"children":[$child]}"""
|
||||
|
||||
private fun tree(vararg children: String) =
|
||||
"""{"attributes":{"resource-id":"root"},"children":[${children.joinToString(",")}]}"""
|
||||
|
||||
private val crossFade = tree(screen("LedgerScreen"), screen("AddTransactionScreen"))
|
||||
private val landed = tree(screen("AddTransactionScreen"))
|
||||
|
||||
@Test fun waitsForTheCrossFadeToLandAndReturnsTheLandedTree() {
|
||||
// The first read caught both routes alive. The wait must keep reading
|
||||
// until only the destination is left, and hand back that tree: the
|
||||
// caller records what it returns, so returning the fade would put the
|
||||
// frame in the trace whether or not the wait happened.
|
||||
var reads = 0
|
||||
val settled = awaitSettledTree {
|
||||
reads++
|
||||
if (reads <= 3) crossFade else landed
|
||||
}
|
||||
assertTrue(reads > 3, "must keep reading until the fade lands, reads=$reads")
|
||||
assertEquals(1, countRouteScreens(settled), "must return a tree with one route")
|
||||
}
|
||||
|
||||
@Test fun settledFrameCostsExactlyOneRead() {
|
||||
// A hierarchy read is the expensive part of a step, and this one is the
|
||||
// read the snapshot was going to do anyway. A frame that is already on
|
||||
// one route must not pay for a second: that cost, on every step, is why
|
||||
// the unconditional structural poll was removed from waitForIdle.
|
||||
var reads = 0
|
||||
val settled = awaitSettledTree {
|
||||
reads++
|
||||
landed
|
||||
}
|
||||
assertEquals(1, reads, "a one-route frame must read once and return")
|
||||
assertEquals(landed, settled)
|
||||
}
|
||||
|
||||
@Test fun repeatedRouteIdIsOneScreenNotATransition() {
|
||||
// A screen nesting a node that repeats its own route id puts two tagged
|
||||
// nodes in the tree while one destination is on screen. Counting nodes
|
||||
// would read that as a fade that never ends: every step of that screen
|
||||
// would burn the whole poll budget and still hand over a frame the
|
||||
// runner refuses to act on.
|
||||
val nested = tree(screen("HomeScreen", screen("HomeScreen")))
|
||||
assertEquals(1, countRouteScreens(nested), "the same id twice is one route")
|
||||
var reads = 0
|
||||
awaitSettledTree {
|
||||
reads++
|
||||
nested
|
||||
}
|
||||
assertEquals(1, reads, "a repeated route id must not be treated as a transition")
|
||||
}
|
||||
|
||||
@Test fun aLayoutThatKeepsTwoRoutesIsBoundedByTheCap() {
|
||||
// Two routes alive at rest is a real layout, not a fade, and no amount
|
||||
// of waiting will resolve it. The wait is bounded by wall clock, so
|
||||
// such a screen costs the cap once per step and nothing more.
|
||||
val start = System.currentTimeMillis()
|
||||
val settled = awaitSettledTree { crossFade }
|
||||
val elapsed = System.currentTimeMillis() - start
|
||||
assertTrue(
|
||||
elapsed >= TRANSITION_POLL_CAP_MILLIS,
|
||||
"must actually wait out a two-route tree, elapsed=${elapsed}ms",
|
||||
)
|
||||
assertTrue(
|
||||
elapsed < TRANSITION_POLL_CAP_MILLIS + 1000L,
|
||||
"must stop at the cap, elapsed=${elapsed}ms",
|
||||
)
|
||||
assertEquals(crossFade, settled, "the caller still gets a tree to record")
|
||||
}
|
||||
|
||||
@Test fun capCoversTheNavHostFadePlusTheStreak() {
|
||||
// Compose navigation's default enter and exit are a 700ms tween, and
|
||||
// the fade starts when the action lands, not when the snapshot begins.
|
||||
// A cap that does not clear the fade and the streak after it leaves the
|
||||
// frame transitional, which is the whole defect.
|
||||
assertTrue(
|
||||
TRANSITION_POLL_CAP_MILLIS >= 700L + TRANSITION_STABLE_STREAK_MILLIS,
|
||||
"cap ${TRANSITION_POLL_CAP_MILLIS}ms cannot cover a 700ms fade plus a " +
|
||||
"${TRANSITION_STABLE_STREAK_MILLIS}ms streak",
|
||||
)
|
||||
}
|
||||
}
|
||||
@@ -21,23 +21,28 @@ class StabilityPollTest {
|
||||
// begun must reset the clock: the post-transition stable window has
|
||||
// to start over and meet MIN_STABLE_STREAK_MILLIS from scratch.
|
||||
var calls = 0
|
||||
val start = System.currentTimeMillis()
|
||||
// First 4 samples are "calm", then 1 transient change, then "stable"
|
||||
// forever - the calm prefix is meaningless because of the transition.
|
||||
var transientAt = 0L
|
||||
// A short "calm" prefix, one transient change, then "stable" forever.
|
||||
// The prefix is meaningless because of the transition: only the streak
|
||||
// that starts after it can end the wait.
|
||||
pollUntilStable(3000L) {
|
||||
calls++
|
||||
when {
|
||||
calls <= 4 -> "calm"
|
||||
calls == 5 -> "transient"
|
||||
calls <= 2 -> "calm"
|
||||
calls == 3 -> {
|
||||
transientAt = System.currentTimeMillis()
|
||||
"transient"
|
||||
}
|
||||
else -> "stable"
|
||||
}
|
||||
}
|
||||
val elapsed = System.currentTimeMillis() - start
|
||||
val sinceTransition = System.currentTimeMillis() - transientAt
|
||||
assertTrue(calls > 3, "must keep sampling past the transition, got $calls")
|
||||
assertTrue(
|
||||
elapsed >= MIN_STABLE_STREAK_MILLIS,
|
||||
"post-transition streak must reach ${MIN_STABLE_STREAK_MILLIS}ms, elapsed=${elapsed}ms",
|
||||
sinceTransition >= MIN_STABLE_STREAK_MILLIS,
|
||||
"the calm prefix must not count: a full ${MIN_STABLE_STREAK_MILLIS}ms streak has to " +
|
||||
"start over after the transition, returned ${sinceTransition}ms after it",
|
||||
)
|
||||
assertTrue(calls >= 10, "expected enough samples to span calm + transient + stable streak, got $calls")
|
||||
}
|
||||
|
||||
@Test fun transitionalNullsForceLoopToKeepWaiting() {
|
||||
|
||||
Reference in new issue
Block a user