From b045ab966a7f871108adecd0dedc2bfb64131094 Mon Sep 17 00:00:00 2001 From: Mario Rial <5914559+MarioRial22@users.noreply.github.com> Date: Sat, 18 Jul 2026 18:15:26 +0000 Subject: [PATCH 1/2] fix(devicelab): re-sample the keyboard-blocking check until IME-resize geometry settles MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The keyboard-blocking guard added to the tap paths reads element and keyboard bounds exactly once, right after an input step. Windows with SOFT_INPUT_ADJUST_RESIZE (e.g. a plain AlertDialog whose body is a ScrollView) relayout a few frames after the IME appears or after typing: on that first frame the target still reports covered bounds, then the window shrinks and the target rises above the keyboard. The single-shot check reads the stale first frame and rejects a perfectly tappable element with "Element found but keyboard is covering it", suggesting a hideKeyboard step that is not needed (and on Android falls back to BACK, which apps may intercept). Real-world hit: tapOn android:id/button1 (dialog positive button) right after inputText — reported covered at centerY=1627 vs keyboard top 1560 on the first frame, tappable at ~1400 a few frames later. Worked on v1.1.19, regressed by the new guard. Fix: keep the guard's purpose (fail with the actionable hint when a tap would genuinely land on the IME) but re-sample element vs keyboard geometry every 50ms for up to 2s, returning the error only when the overlap PERSISTS through the whole window. Bail out early (proceed with the tap) as soon as the keyboard is dismissed or the element clears it. The verdict per sample is extracted to keyboardStillCovering and covered by tests with the real-world geometry, including both edges of the 50px suggestion-strip margin. --- pkg/driver/devicelab/keyboard.go | 67 ++++++++++++++++++++------- pkg/driver/devicelab/keyboard_test.go | 48 +++++++++++++++++++ 2 files changed, 98 insertions(+), 17 deletions(-) create mode 100644 pkg/driver/devicelab/keyboard_test.go diff --git a/pkg/driver/devicelab/keyboard.go b/pkg/driver/devicelab/keyboard.go index cf7d07ff..01c8d6fb 100644 --- a/pkg/driver/devicelab/keyboard.go +++ b/pkg/driver/devicelab/keyboard.go @@ -5,6 +5,7 @@ import ( "regexp" "strconv" "strings" + "time" "github.com/devicelab-dev/maestro-runner/pkg/core" "github.com/devicelab-dev/maestro-runner/pkg/flow" @@ -150,35 +151,67 @@ func (d *Driver) consumeInputFlag() bool { var errKeyboardOpen = fmt.Errorf("keyboard is open — add a `- hideKeyboard` step before this step") +// keyboardSettleWindow bounds how long checkKeyboardBlocking re-samples geometry before +// declaring the element covered. Windows with SOFT_INPUT_ADJUST_RESIZE (e.g. a plain +// AlertDialog whose body is a ScrollView) relayout a few frames after the IME appears or +// after typing: on the first frame the target still reports covered bounds, then the +// window shrinks and the target rises above the keyboard. A single-shot check reads that +// stale first frame and rejects a perfectly tappable element. Var (not const) so tests +// can shrink it. +var keyboardSettleWindow = 2 * time.Second + +// keyboardSettlePoll is the re-sample cadence while waiting for the geometry to settle. +const keyboardSettlePoll = 50 * time.Millisecond + +// keyboardStillCovering is the per-sample verdict: true only when the keyboard is +// visible AND a tap on the element's center would land on it. +func keyboardStillCovering(element core.Bounds, keyboard *core.Bounds) bool { + return keyboard != nil && tapWouldHitKeyboard(element, *keyboard) +} + // checkKeyboardBlocking checks if the keyboard overlaps the target element after an input step. // UIA2 finds elements via the accessibility tree even when the keyboard covers them, // but coordinate taps land on the keyboard overlay instead. This detects that case and -// fails fast with a helpful hint instead of silently tapping the keyboard. +// fails with a helpful hint instead of silently tapping the keyboard. +// +// The overlap is re-sampled until keyboardSettleWindow elapses: IME-aware windows +// (SOFT_INPUT_ADJUST_RESIZE) lift the target above the keyboard shortly after an input +// step, and rejecting on first-frame geometry would fail elements that are tappable an +// instant later. Only a PERSISTENT overlap — the true positive this guard exists for — +// returns the error. // Returns nil if this check doesn't apply or element is not blocked — caller should proceed normally. func (d *Driver) checkKeyboardBlocking(wasInput bool, sel flow.Selector) *core.CommandResult { if !wasInput { return nil } - // Find element (UIA2 will find it even behind keyboard) - _, info, err := d.findElementOnce(sel) - if err != nil || info == nil { - // Element genuinely not found — let caller do the full-timeout find - return nil - } + deadline := time.Now().Add(keyboardSettleWindow) + lastKbTop, lastCenterY := -1, -1 + for { + // Find element (UIA2 will find it even behind keyboard) + _, info, err := d.findElementOnce(sel) + if err != nil || info == nil { + // Element genuinely not found — let caller do the full-timeout find + return nil + } - // Element found — check if keyboard overlaps its bounds - kbBounds := d.getKeyboardBounds() - if kbBounds == nil { - return nil - } + kbBounds := d.getKeyboardBounds() + if !keyboardStillCovering(info.Bounds, kbBounds) { + // Keyboard dismissed, or the window resized/panned and the element + // now sits above it — nothing blocks the tap. + return nil + } - if tapWouldHitKeyboard(info.Bounds, *kbBounds) { _, cy := info.Bounds.Center() - return errorResult(errKeyboardOpen, - fmt.Sprintf("Element found but keyboard is covering it (keyboard top: %d, element center Y: %d) — add a `- hideKeyboard` step before this step", - kbBounds.Y, cy)) + lastKbTop, lastCenterY = kbBounds.Y, cy + + if !time.Now().Before(deadline) { + break + } + time.Sleep(keyboardSettlePoll) } - return nil + return errorResult(errKeyboardOpen, + fmt.Sprintf("Element found but keyboard is covering it (keyboard top: %d, element center Y: %d) — add a `- hideKeyboard` step before this step", + lastKbTop, lastCenterY)) } diff --git a/pkg/driver/devicelab/keyboard_test.go b/pkg/driver/devicelab/keyboard_test.go new file mode 100644 index 00000000..d895e33f --- /dev/null +++ b/pkg/driver/devicelab/keyboard_test.go @@ -0,0 +1,48 @@ +package devicelab + +import ( + "testing" + + "github.com/devicelab-dev/maestro-runner/pkg/core" +) + +// Per-sample verdict for the keyboard-blocking settle loop. Pinned with the real-world +// geometry that motivated it: an AlertDialog positive button (android:id/button1) +// reported at centerY=1627 on the first frame after typing (keyboard top 1560), then +// lifted to ~1400 once the window's SOFT_INPUT_ADJUST_RESIZE relayout settled — the +// guard must treat the first sample as "still covering" and the second as clear, and a +// dismissed keyboard (nil bounds) as never covering. + +func elementCenteredAtY(cy int) core.Bounds { + // Height 100 → center = Y + 50. + return core.Bounds{X: 400, Y: cy - 50, Width: 200, Height: 100} +} + +func TestKeyboardStillCovering(t *testing.T) { + keyboard := &core.Bounds{X: 0, Y: 1560, Width: 1080, Height: 840} + + cases := []struct { + name string + element core.Bounds + keyboard *core.Bounds + want bool + }{ + {"covered on first frame after typing", elementCenteredAtY(1627), keyboard, true}, + {"clear after ADJUST_RESIZE relayout", elementCenteredAtY(1400), keyboard, false}, + {"keyboard dismissed mid-settle", elementCenteredAtY(1627), nil, false}, + // The 50px suggestion-strip margin: at keyboard.Y+margin is covered… + {"at margin boundary", elementCenteredAtY(1610), keyboard, true}, + // …one pixel above the margin is still tappable. + {"just above margin", elementCenteredAtY(1609), keyboard, false}, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + got := keyboardStillCovering(tc.element, tc.keyboard) + if got != tc.want { + t.Errorf("keyboardStillCovering(%+v, %+v) = %v, want %v", + tc.element, tc.keyboard, got, tc.want) + } + }) + } +} From cb522014703014850bc8955db3c9e7869e9cd9f8 Mon Sep 17 00:00:00 2001 From: Mario Rial <5914559+MarioRial22@users.noreply.github.com> Date: Tue, 21 Jul 2026 05:31:53 +0000 Subject: [PATCH 2/2] test(devicelab): cover the keyboard-blocking settle loop Extract the re-sampling loop into settleKeyboardBlocking with injected samplers so the settle behavior is testable without a live driver, and cover it end to end: element missing, clear on first frame, relayout lifting the element mid-settle, keyboard dismissed mid-settle, and a persistent overlap failing with the geometry hint, plus a pass through checkKeyboardBlocking with the mock client. No behavior change. Addresses the Codecov patch-coverage warning. --- pkg/driver/devicelab/keyboard.go | 36 ++++--- pkg/driver/devicelab/keyboard_test.go | 132 ++++++++++++++++++++++++++ 2 files changed, 157 insertions(+), 11 deletions(-) diff --git a/pkg/driver/devicelab/keyboard.go b/pkg/driver/devicelab/keyboard.go index 01c8d6fb..152babc7 100644 --- a/pkg/driver/devicelab/keyboard.go +++ b/pkg/driver/devicelab/keyboard.go @@ -173,29 +173,43 @@ func keyboardStillCovering(element core.Bounds, keyboard *core.Bounds) bool { // UIA2 finds elements via the accessibility tree even when the keyboard covers them, // but coordinate taps land on the keyboard overlay instead. This detects that case and // fails with a helpful hint instead of silently tapping the keyboard. -// -// The overlap is re-sampled until keyboardSettleWindow elapses: IME-aware windows -// (SOFT_INPUT_ADJUST_RESIZE) lift the target above the keyboard shortly after an input -// step, and rejecting on first-frame geometry would fail elements that are tappable an -// instant later. Only a PERSISTENT overlap — the true positive this guard exists for — -// returns the error. // Returns nil if this check doesn't apply or element is not blocked — caller should proceed normally. func (d *Driver) checkKeyboardBlocking(wasInput bool, sel flow.Selector) *core.CommandResult { if !wasInput { return nil } + return settleKeyboardBlocking( + func() (*core.ElementInfo, bool) { + // Find element (UIA2 will find it even behind keyboard) + _, info, err := d.findElementOnce(sel) + if err != nil || info == nil { + // Element genuinely not found — let caller do the full-timeout find + return nil, false + } + return info, true + }, + d.getKeyboardBounds, + ) +} + +// settleKeyboardBlocking re-samples element vs keyboard geometry until keyboardSettleWindow +// elapses: IME-aware windows (SOFT_INPUT_ADJUST_RESIZE) lift the target above the keyboard +// shortly after an input step, and rejecting on first-frame geometry would fail elements +// that are tappable an instant later. Only a PERSISTENT overlap — the true positive this +// guard exists for — returns the error. The samplers are injected so the settle behavior +// is testable without a live driver. +func settleKeyboardBlocking(findElement func() (*core.ElementInfo, bool), + keyboardBounds func() *core.Bounds) *core.CommandResult { deadline := time.Now().Add(keyboardSettleWindow) lastKbTop, lastCenterY := -1, -1 for { - // Find element (UIA2 will find it even behind keyboard) - _, info, err := d.findElementOnce(sel) - if err != nil || info == nil { - // Element genuinely not found — let caller do the full-timeout find + info, ok := findElement() + if !ok { return nil } - kbBounds := d.getKeyboardBounds() + kbBounds := keyboardBounds() if !keyboardStillCovering(info.Bounds, kbBounds) { // Keyboard dismissed, or the window resized/panned and the element // now sits above it — nothing blocks the tap. diff --git a/pkg/driver/devicelab/keyboard_test.go b/pkg/driver/devicelab/keyboard_test.go index d895e33f..035fb9f5 100644 --- a/pkg/driver/devicelab/keyboard_test.go +++ b/pkg/driver/devicelab/keyboard_test.go @@ -1,9 +1,12 @@ package devicelab import ( + "strings" "testing" + "time" "github.com/devicelab-dev/maestro-runner/pkg/core" + "github.com/devicelab-dev/maestro-runner/pkg/flow" ) // Per-sample verdict for the keyboard-blocking settle loop. Pinned with the real-world @@ -46,3 +49,132 @@ func TestKeyboardStillCovering(t *testing.T) { }) } } + +// --- Settle loop ----------------------------------------------------------------------- + +// shrinkSettleWindow shortens the settle window so loop tests run in milliseconds, +// restoring the real value afterwards. +func shrinkSettleWindow(t *testing.T, d time.Duration) { + t.Helper() + orig := keyboardSettleWindow + keyboardSettleWindow = d + t.Cleanup(func() { keyboardSettleWindow = orig }) +} + +func TestSettleKeyboardBlocking_ElementNotFound(t *testing.T) { + shrinkSettleWindow(t, 100*time.Millisecond) + res := settleKeyboardBlocking( + func() (*core.ElementInfo, bool) { return nil, false }, + func() *core.Bounds { + t.Fatal("keyboardBounds must not be sampled when the element is missing") + return nil + }, + ) + if res != nil { + t.Errorf("expected nil (caller does the full-timeout find), got %+v", res) + } +} + +func TestSettleKeyboardBlocking_ClearOnFirstSample(t *testing.T) { + shrinkSettleWindow(t, 100*time.Millisecond) + samples := 0 + res := settleKeyboardBlocking( + func() (*core.ElementInfo, bool) { + samples++ + return &core.ElementInfo{Bounds: elementCenteredAtY(1400)}, true + }, + func() *core.Bounds { return &core.Bounds{X: 0, Y: 1560, Width: 1080, Height: 840} }, + ) + if res != nil { + t.Errorf("element above the keyboard must not block, got %+v", res) + } + if samples != 1 { + t.Errorf("clear-on-first-frame must return without re-sampling, sampled %d times", samples) + } +} + +func TestSettleKeyboardBlocking_RelayoutLiftsElementMidSettle(t *testing.T) { + // The real-world repro: first frame still covered (centerY 1627 vs keyboard top + // 1560), then the ADJUST_RESIZE relayout lifts the element clear (~1400). + shrinkSettleWindow(t, 2*time.Second) // generous — the early exit must beat it + samples := 0 + start := time.Now() + res := settleKeyboardBlocking( + func() (*core.ElementInfo, bool) { + samples++ + if samples == 1 { + return &core.ElementInfo{Bounds: elementCenteredAtY(1627)}, true + } + return &core.ElementInfo{Bounds: elementCenteredAtY(1400)}, true + }, + func() *core.Bounds { return &core.Bounds{X: 0, Y: 1560, Width: 1080, Height: 840} }, + ) + if res != nil { + t.Errorf("element lifted mid-settle must not block, got %+v", res) + } + if samples != 2 { + t.Errorf("expected exactly 2 samples (covered, then clear), got %d", samples) + } + if elapsed := time.Since(start); elapsed > time.Second { + t.Errorf("early exit expected well before the settle window, took %v", elapsed) + } +} + +func TestSettleKeyboardBlocking_KeyboardDismissedMidSettle(t *testing.T) { + shrinkSettleWindow(t, 2*time.Second) + samples := 0 + res := settleKeyboardBlocking( + func() (*core.ElementInfo, bool) { + samples++ + return &core.ElementInfo{Bounds: elementCenteredAtY(1627)}, true + }, + func() *core.Bounds { + if samples == 1 { + return &core.Bounds{X: 0, Y: 1560, Width: 1080, Height: 840} + } + return nil // keyboard gone + }, + ) + if res != nil { + t.Errorf("dismissed keyboard must not block, got %+v", res) + } +} + +func TestSettleKeyboardBlocking_PersistentOverlapFails(t *testing.T) { + shrinkSettleWindow(t, 120*time.Millisecond) // ~3 samples at the 50ms cadence + samples := 0 + res := settleKeyboardBlocking( + func() (*core.ElementInfo, bool) { + samples++ + return &core.ElementInfo{Bounds: elementCenteredAtY(1627)}, true + }, + func() *core.Bounds { return &core.Bounds{X: 0, Y: 1560, Width: 1080, Height: 840} }, + ) + if res == nil { + t.Fatal("persistent overlap through the whole settle window must fail") + } + if samples < 2 { + t.Errorf("expected re-sampling before failing, sampled %d times", samples) + } + if !strings.Contains(res.Message, "keyboard top: 1560") || + !strings.Contains(res.Message, "element center Y: 1627") { + t.Errorf("error must keep the actionable geometry hint, got %q", res.Message) + } +} + +func TestCheckKeyboardBlocking_SkipsWhenPreviousStepWasNotInput(t *testing.T) { + d := &Driver{client: &mockDeviceLabClient{}} + if res := d.checkKeyboardBlocking(false, flow.Selector{ID: "android:id/button1"}); res != nil { + t.Errorf("check must not apply after a non-input step, got %+v", res) + } +} + +func TestCheckKeyboardBlocking_ElementNotFoundProceeds(t *testing.T) { + // End-to-end through the driver: the mock client fails FindElement, so the + // settle loop must bail out and let the caller do the full-timeout find. + shrinkSettleWindow(t, 100*time.Millisecond) + d := New(&mockDeviceLabClient{}, &core.PlatformInfo{}, &mockShell{}) + if res := d.checkKeyboardBlocking(true, flow.Selector{ID: "android:id/button1"}); res != nil { + t.Errorf("element not found must proceed to the normal find, got %+v", res) + } +}