mirror of
https://github.com/priyanshujain/sanderling.git
synced 2026-10-02 19:17:10 +00:00
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.
This commit is contained in:
1 parent
1b01bdbaf1
commit
433f786080
3 files changed
+116
-15
No files matched your search
@@ -859,20 +859,43 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
|
|||||||
|
|
||||||
override fun terminate(bundleId: String) = driver.stopApp(bundleId)
|
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<Int, Int>? = 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))
|
driver.longPress(maestro.Point(x, y))
|
||||||
|
}
|
||||||
|
|
||||||
override fun tapSelector(selector: String) {
|
override fun tapSelector(selector: String) {
|
||||||
val root = driver.contentDescriptor(false)
|
val root = driver.contentDescriptor(false)
|
||||||
val bounds = findBoundsBySelector(root, selector) ?: return
|
val bounds = requireBoundsBySelector(root, selector)
|
||||||
driver.tap(
|
tap((bounds[0] + bounds[2]) / 2, (bounds[1] + bounds[3]) / 2)
|
||||||
maestro.Point(
|
|
||||||
(bounds[0] + bounds[2]) / 2,
|
|
||||||
(bounds[1] + bounds[3]) / 2,
|
|
||||||
),
|
|
||||||
)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun inputText(text: String) {
|
override fun inputText(text: String) {
|
||||||
@@ -926,14 +949,17 @@ class MaestroDriverBackend(private val serial: String?) : DriverBackend {
|
|||||||
toX: Int,
|
toX: Int,
|
||||||
toY: Int,
|
toY: Int,
|
||||||
durationMillis: Long,
|
durationMillis: Long,
|
||||||
) = driver.swipe(
|
) {
|
||||||
|
requireOnScreen(fromX, fromY)
|
||||||
|
driver.swipe(
|
||||||
maestro.Point(fromX, fromY),
|
maestro.Point(fromX, fromY),
|
||||||
maestro.Point(toX, toY),
|
maestro.Point(toX, toY),
|
||||||
maxOf(durationMillis, 250L),
|
maxOf(durationMillis, 250L),
|
||||||
)
|
)
|
||||||
|
}
|
||||||
|
|
||||||
override fun pressKey(key: String) {
|
override fun pressKey(key: String) {
|
||||||
maestroKeyFor(key)?.let { driver.pressKey(it) }
|
driver.pressKey(maestroKeyFor(key))
|
||||||
}
|
}
|
||||||
|
|
||||||
override fun screenshot(): Triple<ByteArray, Int, Int> {
|
override fun screenshot(): Triple<ByteArray, Int, Int> {
|
||||||
@@ -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(
|
internal fun findBoundsBySelector(
|
||||||
root: maestro.TreeNode,
|
root: maestro.TreeNode,
|
||||||
selector: String,
|
selector: String,
|
||||||
@@ -1073,9 +1111,19 @@ internal fun findBoundsInTree(
|
|||||||
return null
|
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? {
|
internal fun parseBounds(s: String): IntArray? {
|
||||||
val pattern = Regex("^\\[(-?\\d+),(-?\\d+),(-?\\d+),(-?\\d+)\\]$")
|
val m = boundsPairs.matchEntire(s)
|
||||||
val m = pattern.matchEntire(s) ?: return null
|
?: boundsFlat.matchEntire(s)
|
||||||
|
?: return null
|
||||||
return IntArray(4) { m.groupValues[it + 1].toInt() }
|
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
|
// maestroKeyFor rejects an unknown key (matching StubDriverBackend) so an
|
||||||
// unmapped or wrong-case key fails loudly instead of being silently dropped.
|
// unmapped or wrong-case key fails loudly instead of being silently dropped. A
|
||||||
internal fun maestroKeyFor(key: String): maestro.KeyCode? {
|
// 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()]
|
val keyCode = StubDriverBackend.KEY_MAP[key.lowercase()]
|
||||||
?: throw IllegalArgumentException("unsupported pressKey value: $key")
|
?: throw IllegalArgumentException("unsupported pressKey value: $key")
|
||||||
return keyCodeToMaestro(keyCode)
|
return keyCodeToMaestro(keyCode)
|
||||||
|
?: throw IllegalArgumentException("no device key for $keyCode")
|
||||||
}
|
}
|
||||||
|
|
||||||
private fun keyCodeToMaestro(adbKeyCode: String): maestro.KeyCode? =
|
private fun keyCodeToMaestro(adbKeyCode: String): maestro.KeyCode? =
|
||||||
|
|||||||
@@ -2,6 +2,7 @@ package dev.sanderling.sidecar
|
|||||||
|
|
||||||
import org.junit.Test
|
import org.junit.Test
|
||||||
import kotlin.test.assertEquals
|
import kotlin.test.assertEquals
|
||||||
|
import kotlin.test.assertFailsWith
|
||||||
import kotlin.test.assertNull
|
import kotlin.test.assertNull
|
||||||
import kotlin.test.assertTrue
|
import kotlin.test.assertTrue
|
||||||
|
|
||||||
@@ -136,6 +137,20 @@ class DeviceOutputParserTest {
|
|||||||
assertNull(findBoundsBySelector(tree, "id:missing"))
|
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<io.grpc.StatusRuntimeException> {
|
||||||
|
requireBoundsBySelector(tree, "id:missing")
|
||||||
|
}
|
||||||
|
assertEquals(io.grpc.Status.Code.NOT_FOUND, refused.status.code)
|
||||||
|
}
|
||||||
|
|
||||||
@Test fun findBoundsBySelectorReturnsNullWhenMatchHasMalformedBounds() {
|
@Test fun findBoundsBySelectorReturnsNullWhenMatchHasMalformedBounds() {
|
||||||
val tree = node("resource-id" to "com.example:id/x", "bounds" to "not-bounds")
|
val tree = node("resource-id" to "com.example:id/x", "bounds" to "not-bounds")
|
||||||
assertNull(findBoundsBySelector(tree, "id:x"))
|
assertNull(findBoundsBySelector(tree, "id:x"))
|
||||||
|
|||||||
@@ -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))
|
||||||
|
}
|
||||||
|
}
|
||||||
Reference in new issue
Block a user