Skip to content

Commit bb23268

Browse files
committed
fix(maestro): recognize Android scroll containers when aiming scrollUntilVisible
The divergence note added in the previous commit was wrong, and it was covering for a bug rather than describing a design. Grounding the comparison at the call site instead of in fixture text changes the answer. `node.type` is never normalized on the way in -- it carries the raw platform string -- so the domain of each predicate is exactly what each platform emits: iOS `elementTypeName` returns 31 fixed short names ("Table", "ScrollView", "CollectionView", ...), never "XCUIElementType*". Android `attrs.className`, fully qualified. macOS role-mapped short names; outside Maestro's platform union. Over all 31 iOS names the two predicates agree on every single one. The claimed `XCUIElementTypeTable` / `AXTable` divergence was measured on strings the runner cannot produce; the real emission is "Table", which both predicates accept. The role/subrole arm is macOS-helper-only, so it is inert for Maestro entirely. What remains is Android, one-directional, and a defect: matching a normalized type for EQUALITY recognizes bare `android.widget.ScrollView` and silently misses HorizontalScrollView, NestedScrollView, RecyclerView, ListView and GridView. `scrollUntilVisible` therefore selected no container and fell back to a screen-centred swipe inside essentially every RecyclerView-backed list -- contradicting the function's own documented intent, and contradicting the existing Android test that expects `android.widget.ScrollView` to be selected. So the third walk collapses onto the shared helper too: the substring predicate is also the better fit for Android's open class-name space, where an allow-list would keep missing NestedScrollView and every custom subclass. All three walks now share `@agent-device/contracts/snapshot`, and the explanatory comment is gone because there is nothing left to explain. The test is rewritten to pin the classification over the vocabulary each platform actually emits, with the Android rows as the regression guard.
1 parent 8a6f26d commit bb23268

3 files changed

Lines changed: 178 additions & 215 deletions

File tree

Lines changed: 163 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,163 @@
1+
import { describe, expect, test } from 'vitest';
2+
import type { SnapshotNode } from '@agent-device/kernel/snapshot';
3+
import { resolveMaestroScrollableGesture } from '../runtime-port-geometry.ts';
4+
// The snapshot-facing platform union ('android' | 'ios'), not program-ir's,
5+
// which also carries 'web'.
6+
import type { MaestroPlatform } from '../runtime-target-policy.ts';
7+
import { isMaestroNodeVisible } from '../snapshot-policy.ts';
8+
9+
// Maestro asks two questions about a scrollable node, and they must agree:
10+
//
11+
// clips - which ancestor bounds a node, deciding whether it counts as visible
12+
// swipes - which container scrollUntilVisible starts its swipe inside
13+
//
14+
// Both now go through contracts' isScrollableNodeLike. This table pins the
15+
// classification against the vocabulary each platform actually emits:
16+
// `elementTypeName` in RunnerTests+Snapshot.swift for iOS (short PascalCase
17+
// names, NOT XCUIElementType*), and the fully-qualified `className` from the
18+
// Android view hierarchy. The Android rows are the regression guard: the swipe
19+
// side used to equality-match a normalized type, so it recognized bare
20+
// ScrollView and silently missed every other Android scroll container.
21+
22+
const APPLICATION: SnapshotNode = {
23+
index: 0,
24+
ref: '@e1',
25+
type: 'Application',
26+
visibleToUser: true,
27+
rect: { x: 0, y: 0, width: 402, height: 874 },
28+
};
29+
30+
// Tall and narrow so the vertical-axis check in selectMaestroScrollableViewport
31+
// accepts it, and short enough that the target below sits outside it.
32+
const CONTAINER_RECT = { x: 0, y: 100, width: 200, height: 600 };
33+
const TARGET_RECT = { x: 20, y: 750, width: 100, height: 40 };
34+
35+
function snapshotWithContainer(containerType: string) {
36+
const nodes: SnapshotNode[] = [
37+
APPLICATION,
38+
{
39+
index: 1,
40+
ref: '@e2',
41+
parentIndex: 0,
42+
type: containerType,
43+
visibleToUser: true,
44+
rect: CONTAINER_RECT,
45+
},
46+
{
47+
index: 2,
48+
ref: '@e3',
49+
parentIndex: 1,
50+
type: 'Button',
51+
identifier: 'target',
52+
visibleToUser: true,
53+
rect: TARGET_RECT,
54+
},
55+
];
56+
return { createdAt: 0, nodes };
57+
}
58+
59+
/** True when the container clips the target, i.e. it is the effective viewport. */
60+
function clips(containerType: string, platform: MaestroPlatform): boolean {
61+
const { nodes } = snapshotWithContainer(containerType);
62+
// The target sits outside the container rect but inside the Application rect,
63+
// so it reads as hidden exactly when the container is the viewport.
64+
return !isMaestroNodeVisible(nodes[2]!, nodes, platform);
65+
}
66+
67+
/** True when scrollUntilVisible swipes inside the container instead of the screen. */
68+
function swipes(containerType: string, platform: MaestroPlatform): boolean {
69+
const gesture = resolveMaestroScrollableGesture(
70+
snapshotWithContainer(containerType),
71+
{ id: 'target' },
72+
'down',
73+
600,
74+
platform,
75+
);
76+
// No container selected => daemon-runtime-port.ts falls back to a plain
77+
// screen scroll.
78+
if (!gesture) return false;
79+
expect(gesture.viewport).toEqual(CONTAINER_RECT);
80+
return true;
81+
}
82+
83+
// Every name `elementTypeName` can return. Scroll containers per the runner's
84+
// own `scrollContainerTypes` set: collectionView, scrollView, table.
85+
const IOS_VOCABULARY: ReadonlyArray<[string, boolean]> = [
86+
['Application', false],
87+
['Window', false],
88+
['Button', false],
89+
['Cell', false],
90+
['StaticText', false],
91+
['TextField', false],
92+
['TextView', false],
93+
['SecureTextField', false],
94+
['Switch', false],
95+
['Slider', false],
96+
['Link', false],
97+
['Image', false],
98+
['NavigationBar', false],
99+
['TabBar', false],
100+
['CollectionView', true],
101+
['Table', true],
102+
['ScrollView', true],
103+
['Toolbar', false],
104+
['SearchField', false],
105+
['SegmentedControl', false],
106+
['Stepper', false],
107+
['Picker', false],
108+
['ActivityIndicator', false],
109+
['ProgressIndicator', false],
110+
['CheckBox', false],
111+
['MenuItem', false],
112+
['WebView', false],
113+
['Other', false],
114+
['Keyboard', false],
115+
['Key', false],
116+
['Element(42)', false],
117+
];
118+
119+
describe('iOS: both walks agree across the runner vocabulary', () => {
120+
test.each(IOS_VOCABULARY)('%s is a scroll container: %s', (type, scrollable) => {
121+
expect(clips(type, 'ios')).toBe(scrollable);
122+
expect(swipes(type, 'ios')).toBe(scrollable);
123+
});
124+
});
125+
126+
// Fully-qualified class names, as `attrs.className` delivers them.
127+
const ANDROID_VOCABULARY: ReadonlyArray<[string, boolean]> = [
128+
['android.widget.ScrollView', true],
129+
// These five were classified as clipping but NOT as swipe containers before
130+
// the predicates were unified, so scrollUntilVisible fell back to a
131+
// screen-centred swipe inside every RecyclerView-backed list.
132+
['android.widget.HorizontalScrollView', true],
133+
['androidx.core.widget.NestedScrollView', true],
134+
['androidx.recyclerview.widget.RecyclerView', true],
135+
['android.widget.ListView', true],
136+
['android.widget.GridView', true],
137+
['android.widget.LinearLayout', false],
138+
['android.widget.FrameLayout', false],
139+
['android.view.View', false],
140+
['android.widget.Button', false],
141+
['androidx.compose.ui.platform.ComposeView', false],
142+
];
143+
144+
describe('Android: both walks agree across common view classes', () => {
145+
test.each(ANDROID_VOCABULARY)('%s is a scroll container: %s', (type, scrollable) => {
146+
expect(clips(type, 'android')).toBe(scrollable);
147+
expect(swipes(type, 'android')).toBe(scrollable);
148+
});
149+
});
150+
151+
test('a RecyclerView is selected as the swipe viewport, not the whole screen', () => {
152+
const gesture = resolveMaestroScrollableGesture(
153+
snapshotWithContainer('androidx.recyclerview.widget.RecyclerView'),
154+
{ id: 'target' },
155+
'down',
156+
600,
157+
'android',
158+
);
159+
// Regression: this was `undefined` (screen-centred swipe) before the fix.
160+
expect(gesture?.viewport).toEqual(CONTAINER_RECT);
161+
// The swipe starts inside the list, at the container's centre.
162+
expect(gesture?.gesture.from).toEqual({ x: 100, y: 400 });
163+
});

packages/maestro/src/internal/__tests__/scrollable-predicate-divergence.test.ts

Lines changed: 0 additions & 160 deletions
This file was deleted.

0 commit comments

Comments
 (0)