From 2019da8cc3059d912371cba839a29b97a9e1250b Mon Sep 17 00:00:00 2001 From: gmegidish Date: Sun, 4 Oct 2026 23:16:44 +0200 Subject: [PATCH] feat(core)!: strict mode for native locators - an action or single-element query on a locator matching several elements fails at once --- README.md | 2 + docs/src/guides/locators.md | 13 ++++ packages/mobilewright-core/src/index.ts | 4 +- .../mobilewright-core/src/locator.test.ts | 67 +++++++++++++++++ packages/mobilewright-core/src/locator.ts | 49 ++++++++++--- .../mobilewright-core/src/query-engine.ts | 71 +++++++++++++++++++ 6 files changed, 193 insertions(+), 13 deletions(-) diff --git a/README.md b/README.md index 90e0cae4..2ecd306c 100644 --- a/README.md +++ b/README.md @@ -165,6 +165,8 @@ const tree = await screen.viewTree() // raw accessibility tree ( Lazy, chainable element reference. No queries execute until you call an action or assertion. +Locators are **strict**, like Playwright's: an action or single-element query on a locator that matches more than one element fails immediately with `strict mode violation: getByRole('button') resolved to 2 elements`, listing the matches. Narrow the locator, or pick one with `first()`, `last()`, `nth()` or `filter()`. `count()`, `all()` and `toHaveCount()` are not strict. A container and its own text child reporting the same label (an iOS button wrapping its text) count as one element. + **Actions** (all auto-wait for the element to be visible, enabled, and have stable bounds): ```typescript diff --git a/docs/src/guides/locators.md b/docs/src/guides/locators.md index 4a4a5772..de5d8bf7 100644 --- a/docs/src/guides/locators.md +++ b/docs/src/guides/locators.md @@ -108,6 +108,19 @@ of the collapsed framework class above — these are also recognized: | `XCUIElementTypeLink` | `link` | | `XCUIElementTypeNavigationBar` | `header` | +## Strict mode + +Like Playwright, a locator used for an action (`tap`, `fill`, `check`, …) or a single-element query or assertion (`getText`, `isVisible`, `toBeVisible`, …) must resolve to exactly one element. When it matches several, the call fails at once — not after the timeout — with the locator and the matches spelled out: + +```text +strict mode violation: getByRole('button') resolved to 2 elements: + 1) Button "Submit" [submitBtn] + 2) Button "Cancel" [cancelBtn] +Narrow the locator, or use .first(), .last(), .nth(i) or .filter(). +``` + +`count()`, `all()`, `first()`, `last()`, `nth()` and `expect(...).toHaveCount()` are not strict. Native trees often report one control at two levels — an iOS `Button` and the `StaticText` inside it both carry the label — and such an ancestor chain counts as a single element, resolved to the outermost node. + ## Notes and known gaps - **Classes with no role.** Anything not listed above has no role mapping — diff --git a/packages/mobilewright-core/src/index.ts b/packages/mobilewright-core/src/index.ts index c669f5c8..ba39bd9b 100644 --- a/packages/mobilewright-core/src/index.ts +++ b/packages/mobilewright-core/src/index.ts @@ -1,10 +1,10 @@ -export { Locator, LocatorError, type LocatorOptions, type ScrollIntoViewOptions, type StepFn } from './locator.js'; +export { Locator, LocatorError, StrictModeViolationError, type LocatorOptions, type ScrollIntoViewOptions, type StepFn } from './locator.js'; export { WebViewLocator } from './webview-locator.js'; export { Screen, type GetByWebViewOptions } from './screen.js'; export { Device, type DeviceOptions } from './device.js'; export { MobileWebViewPage, MobileWebViewPage as Page } from './page.js'; export { MobileWebViewLocator, MobileWebViewLocator as WebLocator } from './web-locator.js'; export { expect, ExpectError, setSoftFailureHandler, setDefaultStepFn, type ExpectOptions, type SoftFailureHandler } from './expect.js'; -export { queryAll, ROLE_TYPE_MAP, bareTypeName, type LocatorStrategy, type Role } from './query-engine.js'; +export { queryAll, describeStrategy, ROLE_TYPE_MAP, bareTypeName, type LocatorStrategy, type Role } from './query-engine.js'; export { sleep } from './sleep.js'; export type { HardwareButton } from '@mobilewright/protocol'; diff --git a/packages/mobilewright-core/src/locator.test.ts b/packages/mobilewright-core/src/locator.test.ts index f185b9c8..25c53537 100644 --- a/packages/mobilewright-core/src/locator.test.ts +++ b/packages/mobilewright-core/src/locator.test.ts @@ -724,6 +724,73 @@ test.describe('Locator', () => { }); }); + // Playwright strict mode: an action or single-element query on a locator + // that resolves to more than one element fails immediately with a message + // naming the locator, instead of silently acting on the first match. + // Native trees repeat a label on a container and its text child (an iOS + // Button wraps a StaticText with the same label); matches that form one + // ancestor chain are a single element, resolved to the outermost node. + test.describe('strict mode', () => { + // `hierarchy` has two sibling buttons: Submit and Cancel. + const twoButtons = () => new Locator(createMockDriver(hierarchy), { kind: 'role', value: 'button' }); + + test('tap() on a locator matching two sibling elements fails without tapping', async () => { + const driver = createMockDriver(hierarchy); + const locator = new Locator(driver, { kind: 'role', value: 'button' }); + await expect(locator.tap({ timeout: 2000 })).rejects.toThrow(/strict mode violation: getByRole\('button'\) resolved to 2 elements/); + expect(driver._tracker.tapCalls).toEqual([]); + }); + + test('the violation is raised at once, not after the timeout', async () => { + const started = Date.now(); + await expect(twoButtons().tap({ timeout: 5000 })).rejects.toThrow(LocatorError); + expect(Date.now() - started).toBeLessThan(1000); + }); + + test('the message lists the matched elements', async () => { + const error = await twoButtons().tap().then(() => null, (e: Error) => e); + expect(error!.message).toContain('Button "Submit"'); + expect(error!.message).toContain('Button "Cancel"'); + }); + + test('single-element queries and assertions are strict too', async () => { + await expect(twoButtons().getText()).rejects.toThrow(/strict mode violation/); + await expect(twoButtons().isVisible()).rejects.toThrow(/strict mode violation/); + await expect(twoButtons().exists()).rejects.toThrow(/strict mode violation/); + await expect(twoButtons().waitFor({ state: 'visible' })).rejects.toThrow(/strict mode violation/); + }); + + test('count(), all(), first(), last() and nth() are not strict', async () => { + expect(await twoButtons().count()).toBe(2); + expect((await twoButtons().all()).length).toBe(2); + expect(await twoButtons().first().getText()).toBe('Submit'); + expect(await twoButtons().last().getText()).toBe('Cancel'); + expect(await twoButtons().nth(1).getText()).toBe('Cancel'); + }); + + test('a container and its own text child with the same label count as one element', async () => { + const row = node({ + type: 'Button', label: 'Basic UI', bounds: { x: 16, y: 261, width: 370, height: 52 }, + children: [node({ type: 'StaticText', label: 'Basic UI', bounds: { x: 31, y: 276, width: 61, height: 20 } })], + }); + const driver = createMockDriver([node({ type: 'Window', children: [row] })]); + const locator = new Locator(driver, { kind: 'text', value: 'Basic UI' }); + await locator.tap(); + // outermost node: the button's centre, not the text's + expect(driver._tracker.tapCalls).toEqual([[201, 287]]); + }); + + test('two rows that both contain the text are still a violation', async () => { + const rowWith = (label: string, y: number) => node({ + type: 'Cell', label, bounds: { x: 0, y, width: 390, height: 44 }, + children: [node({ type: 'StaticText', label, bounds: { x: 16, y: y + 12, width: 100, height: 20 } })], + }); + const driver = createMockDriver([node({ type: 'Window', children: [rowWith('Delete', 100), rowWith('Delete', 200)] })]); + const locator = new Locator(driver, { kind: 'text', value: 'Delete' }); + await expect(locator.tap()).rejects.toThrow(/resolved to 2 elements/); + }); + }); + test.describe('scrollIntoViewIfNeeded', () => { test('returns immediately without swiping when element is already in the viewport', async () => { const driver = createMockDriver(hierarchy); diff --git a/packages/mobilewright-core/src/locator.ts b/packages/mobilewright-core/src/locator.ts index 7420082c..79428b10 100644 --- a/packages/mobilewright-core/src/locator.ts +++ b/packages/mobilewright-core/src/locator.ts @@ -1,5 +1,5 @@ import type { MobilewrightDriver, ViewNode, Bounds, SwipeDirection, ScreenSize } from '@mobilewright/protocol'; -import { queryAll, matchesRole, type LocatorStrategy, type Role } from './query-engine.js'; +import { queryAll, matchesRole, describeStrategy, describeNode, collapseAncestorChains, type LocatorStrategy, type Role } from './query-engine.js'; import { sleep } from './sleep.js'; import { runStep, type StepLocation } from './stackTrace.js'; @@ -263,7 +263,7 @@ export class Locator { /** Read the element's current value from the latest hierarchy, '' if absent. */ private async _currentValue(): Promise { const roots = await this.driver.getViewHierarchy(); - return queryAll(roots, this.strategy)[0]?.value ?? ''; + return this.resolveStrict(roots)?.value ?? ''; } async screenshot(opts?: { timeout?: number }): Promise { @@ -290,7 +290,7 @@ export class Locator { for (let i = 0; i < maxSwipes; i++) { const roots = await this.driver.getViewHierarchy(); - const node = queryAll(roots, this.strategy)[0] ?? null; + const node = this.resolveStrict(roots); if (node && isWithinViewport(node.bounds, screenSize)) { return; @@ -320,7 +320,7 @@ export class Locator { await this.waitFor({ state: 'visible', timeout: opts?.timeout ?? 0 }); return true; } catch (error) { - if (!(error instanceof LocatorError)) { + if (!(error instanceof LocatorError) || error instanceof StrictModeViolationError) { throw error; } return false; @@ -388,7 +388,7 @@ export class Locator { while (true) { const roots = await this.driver.getViewHierarchy(); - const node = queryAll(roots, this.strategy)[0] ?? null; + const node = this.resolveStrict(roots); if (checkState(node, state)) { return node; @@ -396,7 +396,7 @@ export class Locator { if (Date.now() >= deadline) { throw new LocatorError( - `Locator timed out waiting for state "${state}" after ${effectiveTimeout}ms`, + `Locator timed out waiting for state "${state}" after ${effectiveTimeout}ms (${describeStrategy(this.strategy)})`, this.strategy, ); } @@ -422,7 +422,7 @@ export class Locator { while (true) { const roots = await this.driver.getViewHierarchy(); - const node = queryAll(roots, this.strategy)[0]; + const node = this.resolveStrict(roots); if (!node) { lastReason = 'no matching element found'; @@ -445,7 +445,7 @@ export class Locator { if (Date.now() >= deadline) { throw new LocatorError( - `Locator: ${lastReason} after ${effectiveTimeout}ms`, + `Locator: ${lastReason} after ${effectiveTimeout}ms (${describeStrategy(this.strategy)})`, this.strategy, ); } @@ -453,6 +453,25 @@ export class Locator { } } + /** + * The single element this locator resolves to in `roots`, or null when none. + * Playwright strict mode: more than one match is an error, raised at once — + * unless the matches are one ancestor chain (a container and its own text + * child carrying the same label), which resolves to the outermost node. + */ + private resolveStrict(roots: ViewNode[]): ViewNode | null { + const elements = collapseAncestorChains(queryAll(roots, this.strategy)); + if (elements.length <= 1) { + return elements[0] ?? null; + } + throw new StrictModeViolationError( + `strict mode violation: ${describeStrategy(this.strategy)} resolved to ${elements.length} elements:\n` + + elements.map((n, i) => ` ${i + 1}) ${describeNode(n)}`).join('\n') + + '\nNarrow the locator, or use .first(), .last(), .nth(i) or .filter().', + this.strategy, + ); + } + /** Resolve without waiting — returns null if not found */ private async resolve(timeout: number): Promise { const deadline = Date.now() + timeout; @@ -461,9 +480,9 @@ export class Locator { do { const roots = await this.driver.getViewHierarchy(); - const matches = queryAll(roots, this.strategy); - if (matches.length > 0) { - return matches[0]; + const node = this.resolveStrict(roots); + if (node) { + return node; } if (timeout <= 0) { return null; @@ -559,3 +578,11 @@ export class LocatorError extends Error { this.name = 'LocatorError'; } } + +/** A locator matched more than one element where exactly one was required. Never retried. */ +export class StrictModeViolationError extends LocatorError { + constructor(message: string, strategy: LocatorStrategy) { + super(message, strategy); + this.name = 'StrictModeViolationError'; + } +} diff --git a/packages/mobilewright-core/src/query-engine.ts b/packages/mobilewright-core/src/query-engine.ts index 8a198d62..21c96dd9 100644 --- a/packages/mobilewright-core/src/query-engine.ts +++ b/packages/mobilewright-core/src/query-engine.ts @@ -340,3 +340,74 @@ function isContainedWithin(inner: Bounds, outer: Bounds): boolean { inner.y + inner.height <= outer.y + outer.height ); } + +function describeValue(value: string | RegExp): string { + if (value instanceof RegExp) { + return String(value); + } + const escaped = value.replace(/\\/g, '\\\\').replace(/'/g, '\\\''); + return `'${escaped}'`; +} + +function describeExact(exact: boolean | undefined): string { + return exact ? ', { exact: true }' : ''; +} + +/** Render a strategy the way a test would write it, e.g. getByRole('button', { name: 'Save' }). */ +export function describeStrategy(strategy: LocatorStrategy): string { + switch (strategy.kind) { + case 'root': return 'screen'; + case 'label': return `getByLabel(${describeValue(strategy.value)}${describeExact(strategy.exact)})`; + case 'testId': return `getByTestId(${describeValue(strategy.value)})`; + case 'text': return `getByText(${describeValue(strategy.value)}${describeExact(strategy.exact)})`; + case 'type': return `getByType(${describeValue(strategy.value)})`; + case 'placeholder': return `getByPlaceholder(${describeValue(strategy.value)}${describeExact(strategy.exact)})`; + case 'webview': return strategy.testId === undefined ? 'getByWebView()' : `getByWebView({ testId: ${describeValue(strategy.testId)} })`; + case 'role': { + const name = strategy.name === undefined ? '' : `, { name: ${describeValue(strategy.name)} }`; + return `getByRole(${describeValue(strategy.value)}${name})`; + } + case 'chain': + return strategy.parent.kind === 'root' + ? describeStrategy(strategy.child) + : `${describeStrategy(strategy.parent)}.${describeStrategy(strategy.child)}`; + case 'nth': + if (strategy.index === 0) return `${describeStrategy(strategy.parent)}.first()`; + if (strategy.index === -1) return `${describeStrategy(strategy.parent)}.last()`; + return `${describeStrategy(strategy.parent)}.nth(${strategy.index})`; + case 'filter': { + const parts: string[] = []; + if (strategy.hasText !== undefined) parts.push(`hasText: ${describeValue(strategy.hasText)}`); + if (strategy.hasNotText !== undefined) parts.push(`hasNotText: ${describeValue(strategy.hasNotText)}`); + if (strategy.has !== undefined) parts.push(`has: ${describeStrategy(strategy.has)}`); + if (strategy.hasNot !== undefined) parts.push(`hasNot: ${describeStrategy(strategy.hasNot)}`); + return `${describeStrategy(strategy.parent)}.filter({ ${parts.join(', ')} })`; + } + case 'and': return `${describeStrategy(strategy.left)}.and(${describeStrategy(strategy.right)})`; + case 'or': return `${describeStrategy(strategy.left)}.or(${describeStrategy(strategy.right)})`; + } +} + +/** Short description of a node for error messages: type, visible text and test id. */ +export function describeNode(node: ViewNode): string { + const text = node.label ?? node.text ?? node.value ?? ''; + const id = node.identifier ?? node.resourceId ?? node.key; + return `${node.type} ${JSON.stringify(text)}${id ? ` [${id}]` : ''}`; +} + +/** + * Collapse matches that sit on one ancestor chain into their outermost node: + * an iOS Button and the StaticText inside it carry the same label and are one + * element reported at two levels. `matches` is in document order, so an + * ancestor always precedes its descendants. + */ +export function collapseAncestorChains(matches: ViewNode[]): ViewNode[] { + const kept: ViewNode[] = []; + for (const node of matches) { + const insideKept = kept.some((outer) => flattenNodes(outer.children).includes(node)); + if (!insideKept) { + kept.push(node); + } + } + return kept; +}