From d5f3937338455e5cdb1cca72f4265486319c6c82 Mon Sep 17 00:00:00 2001 From: PJ Date: Sat, 22 Aug 2026 21:17:11 +0530 Subject: [PATCH] fix(sidecar): pollUntilStable stops charging the read to the stability streak The doc has said since b02e86b that the streak is quiet the poll observed and that a 500ms read must not clear a 750ms streak having watched 250ms. now was sampled after snapshot() returns, so it did exactly that: on the doc's own worked example the poll returned after 250ms of observed quiet. Sampling the read's start instead makes the code the sentence. The existing test measured from the end of the first read, which included an interval plus a whole read, so it passed against the bug at any read length. --- .../dev/sanderling/sidecar/DriverBackend.kt | 6 ++--- .../sanderling/sidecar/StabilityPollTest.kt | 22 ++++++++----------- 2 files changed, 12 insertions(+), 16 deletions(-) diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt index 6a6c535..cbf729e 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt @@ -159,15 +159,15 @@ internal fun pollUntilStable( var streakStart = 0L while (System.currentTimeMillis() < deadline) { Thread.sleep(intervalMillis) + val readStart = 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 >= streakMillis) return + if (streakStart == 0L) streakStart = System.currentTimeMillis() + if (readStart - streakStart >= streakMillis) return } else { streakStart = 0L } diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt index 29d8c79..b8008a6 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/StabilityPollTest.kt @@ -20,12 +20,9 @@ class StabilityPollTest { } @Test fun slowSnapshotReadsDoNotEatTheStreak() { - // Every other test here uses an instant lambda and so passes whether or - // not a read is charged to the streak; this one is the difference. - // StubDriverBackend's waitForIdle polls a real `uiautomator dump`, - // which costs hundreds of milliseconds, so the slow read is the case it - // runs in. - val readMillis = 400L + // A hierarchy fetch costs about this on a physical device, which is the + // read pollUntilStable's contract works its numbers out against. + val readMillis = 500L val sampleStarts = mutableListOf() val sampleEnds = mutableListOf() val start = System.currentTimeMillis() @@ -36,19 +33,18 @@ class StabilityPollTest { "stable" } - // What the poll actually watched: the last read began this long after - // the first one returned, and every sample in between matched. - val observedQuiet = sampleStarts.last() - sampleEnds.first() + assertTrue( + sampleStarts.size >= 3, + "a ${readMillis}ms read cannot clear the streak in one pair, starts=$sampleStarts", + ) + val firstMatchingReadReturned = sampleEnds[1] + val observedQuiet = sampleStarts.last() - firstMatchingReadReturned assertTrue( observedQuiet >= MIN_STABLE_STREAK_MILLIS, "the poll returned having observed only ${observedQuiet}ms of " + "quiet, not ${MIN_STABLE_STREAK_MILLIS}ms; " + "starts=$sampleStarts ends=$sampleEnds", ) - assertTrue( - sampleStarts.size >= 3, - "a ${readMillis}ms read cannot clear the streak in one pair, starts=$sampleStarts", - ) } @Test fun streakResetsOnAnyChange() {