From 433f78608019ec97bece72f53415a9ac2cc69d03 Mon Sep 17 00:00:00 2001 From: PJ Date: Sun, 16 Aug 2026 17:41:55 +0530 Subject: [PATCH] fix(sidecar): stop dropping gestures, selectors and keys in silence a point outside the screen is refused with OUT_OF_RANGE, a selector that matches nothing with NOT_FOUND, and a key with no device-driver equivalent throws instead of pressing nothing. parseBounds also reads uiautomator's [left,top][right,bottom] form, which is what a device actually reports and which left every by-selector tap on a device resolving to nothing. --- .../dev/sanderling/sidecar/DriverBackend.kt | 95 +++++++++++++++---- .../sidecar/DeviceOutputParserTest.kt | 15 +++ .../sidecar/OffScreenGestureTest.kt | 29 ++++++ 3 files changed, 120 insertions(+), 19 deletions(-) create mode 100644 sidecar/src/test/kotlin/dev/sanderling/sidecar/OffScreenGestureTest.kt diff --git a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt index a496762..bc39fe3 100644 --- a/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt +++ b/sidecar/src/main/kotlin/dev/sanderling/sidecar/DriverBackend.kt @@ -859,20 +859,43 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { override fun terminate(bundleId: String) = driver.stopApp(bundleId) - override fun tap(x: Int, y: Int) = driver.tap(maestro.Point(x, y)) + // requireOnScreen refuses a point the device has no window under. Android + // drops such a MotionEvent in InputDispatcher and the on-device server's + // TapResponse carries no delivery signal, so this is the last layer that + // can tell the runner the gesture reached nothing. + private fun requireOnScreen(x: Int, y: Int) { + val cached = extent + if (cached != null && !offScreen(x, y, cached.first, cached.second)) return + val info = driver.deviceInfo() + val fresh = Pair(info.widthPixels, info.heightPixels) + extent = fresh + if (!offScreen(x, y, fresh.first, fresh.second)) return + throw io.grpc.Status.OUT_OF_RANGE + .withDescription( + "gesture point ($x,$y) is outside the ${fresh.first}x${fresh.second} screen", + ) + .asRuntimeException() + } - override fun longPress(x: Int, y: Int) = + // A cached extent is re-read before any refusal, so a screen that resized + // mid-run cannot make the sidecar invent gesture failures for valid points. + @Volatile + private var extent: Pair? = null + + override fun tap(x: Int, y: Int) { + requireOnScreen(x, y) + driver.tap(maestro.Point(x, y)) + } + + override fun longPress(x: Int, y: Int) { + requireOnScreen(x, y) driver.longPress(maestro.Point(x, y)) + } override fun tapSelector(selector: String) { val root = driver.contentDescriptor(false) - val bounds = findBoundsBySelector(root, selector) ?: return - driver.tap( - maestro.Point( - (bounds[0] + bounds[2]) / 2, - (bounds[1] + bounds[3]) / 2, - ), - ) + val bounds = requireBoundsBySelector(root, selector) + tap((bounds[0] + bounds[2]) / 2, (bounds[1] + bounds[3]) / 2) } override fun inputText(text: String) { @@ -926,14 +949,17 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend { toX: Int, toY: Int, durationMillis: Long, - ) = driver.swipe( - maestro.Point(fromX, fromY), - maestro.Point(toX, toY), - maxOf(durationMillis, 250L), - ) + ) { + requireOnScreen(fromX, fromY) + driver.swipe( + maestro.Point(fromX, fromY), + maestro.Point(toX, toY), + maxOf(durationMillis, 250L), + ) + } override fun pressKey(key: String) { - maestroKeyFor(key)?.let { driver.pressKey(it) } + driver.pressKey(maestroKeyFor(key)) } override fun screenshot(): Triple { @@ -1026,6 +1052,18 @@ private fun buildDadb(serial: String?): dadb.Dadb = ) } +// requireBoundsBySelector refuses a selector that names nothing on the current +// screen. Returning quietly dispatched no tap at all and left the step reading +// as an action that landed. NOT_FOUND keeps it apart from the OUT_OF_RANGE a +// point outside the screen gets: this one never resolved a point to begin with. +internal fun requireBoundsBySelector( + root: maestro.TreeNode, + selector: String, +): IntArray = findBoundsBySelector(root, selector) + ?: throw io.grpc.Status.NOT_FOUND + .withDescription("selector $selector matched no element") + .asRuntimeException() + internal fun findBoundsBySelector( root: maestro.TreeNode, selector: String, @@ -1073,9 +1111,19 @@ internal fun findBoundsInTree( return null } +// Device trees report bounds as uiautomator's [left,top][right,bottom]; the +// flat four-number form comes from the trees the stub backend serves. Reading +// only the flat form left every by-selector tap on a device resolving to +// nothing, which the caller never heard about. +private val boundsPairs = + Regex("^\\[(-?\\d+),(-?\\d+)\\]\\[(-?\\d+),(-?\\d+)\\]$") +private val boundsFlat = + Regex("^\\[(-?\\d+),(-?\\d+),(-?\\d+),(-?\\d+)\\]$") + internal fun parseBounds(s: String): IntArray? { - val pattern = Regex("^\\[(-?\\d+),(-?\\d+),(-?\\d+),(-?\\d+)\\]$") - val m = pattern.matchEntire(s) ?: return null + val m = boundsPairs.matchEntire(s) + ?: boundsFlat.matchEntire(s) + ?: return null return IntArray(4) { m.groupValues[it + 1].toInt() } } @@ -1194,12 +1242,21 @@ internal class WdaRecovery( } } +// offScreen reports a point outside the device's touch surface. The upper +// bounds are exclusive: a tap at x == width is dropped by InputDispatcher just +// as one past it is. +internal fun offScreen(x: Int, y: Int, width: Int, height: Int): Boolean = + x < 0 || y < 0 || x >= width || y >= height + // maestroKeyFor rejects an unknown key (matching StubDriverBackend) so an -// unmapped or wrong-case key fails loudly instead of being silently dropped. -internal fun maestroKeyFor(key: String): maestro.KeyCode? { +// unmapped or wrong-case key fails loudly instead of being silently dropped. A +// mapped key with no device-driver equivalent is rejected the same way: it +// returned null, and the caller then dropped the press with no error at all. +internal fun maestroKeyFor(key: String): maestro.KeyCode { val keyCode = StubDriverBackend.KEY_MAP[key.lowercase()] ?: throw IllegalArgumentException("unsupported pressKey value: $key") return keyCodeToMaestro(keyCode) + ?: throw IllegalArgumentException("no device key for $keyCode") } private fun keyCodeToMaestro(adbKeyCode: String): maestro.KeyCode? = diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt index 38422a7..8cec21e 100644 --- a/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/DeviceOutputParserTest.kt @@ -2,6 +2,7 @@ package dev.sanderling.sidecar import org.junit.Test import kotlin.test.assertEquals +import kotlin.test.assertFailsWith import kotlin.test.assertNull import kotlin.test.assertTrue @@ -136,6 +137,20 @@ class DeviceOutputParserTest { assertNull(findBoundsBySelector(tree, "id:missing")) } + @Test fun findBoundsBySelectorReadsTheBoundsTheDeviceActuallyReports() { + val tree = node("text" to "Sign in", "bounds" to "[129,274][192,292]") + assertEquals(listOf(129, 274, 192, 292), findBoundsBySelector(tree, "text:Sign in")?.toList()) + } + + @Test fun requireBoundsBySelectorRefusesASelectorThatMatchesNothing() { + val tree = node("resource-id" to "com.example:id/x", "bounds" to "[0,0,4,4]") + assertEquals(listOf(0, 0, 4, 4), requireBoundsBySelector(tree, "id:x").toList()) + val refused = assertFailsWith { + requireBoundsBySelector(tree, "id:missing") + } + assertEquals(io.grpc.Status.Code.NOT_FOUND, refused.status.code) + } + @Test fun findBoundsBySelectorReturnsNullWhenMatchHasMalformedBounds() { val tree = node("resource-id" to "com.example:id/x", "bounds" to "not-bounds") assertNull(findBoundsBySelector(tree, "id:x")) diff --git a/sidecar/src/test/kotlin/dev/sanderling/sidecar/OffScreenGestureTest.kt b/sidecar/src/test/kotlin/dev/sanderling/sidecar/OffScreenGestureTest.kt new file mode 100644 index 0000000..2dd2b5e --- /dev/null +++ b/sidecar/src/test/kotlin/dev/sanderling/sidecar/OffScreenGestureTest.kt @@ -0,0 +1,29 @@ +package dev.sanderling.sidecar + +import kotlin.test.Test +import kotlin.test.assertFalse +import kotlin.test.assertTrue + +class OffScreenGestureTest { + + @Test + fun `a point inside the screen is delivered`() { + assertFalse(offScreen(160, 283, 320, 640)) + assertFalse(offScreen(0, 0, 320, 640)) + assertFalse(offScreen(319, 639, 320, 640)) + } + + @Test + fun `a point past an edge is off screen`() { + assertTrue(offScreen(160, 900, 320, 640)) + assertTrue(offScreen(900, 283, 320, 640)) + assertTrue(offScreen(160, -100, 320, 640)) + assertTrue(offScreen(-1, 283, 320, 640)) + } + + @Test + fun `the far edge is exclusive because InputDispatcher drops it`() { + assertTrue(offScreen(320, 283, 320, 640)) + assertTrue(offScreen(160, 640, 320, 640)) + } +}