fix(android): require id and text on the same element in both drivers - #158
Merged
Merged
Conversation
A selector naming both an id and a text was matched as an OR. The builder emitted resourceId-only strategies and then text-only ones as independent candidates, and the finder returns on the first that hits anything — so the id matched, the text was never read, and a wrong text: passed green against the right element. Same defect as the WDA one fixed in devicelab-dev#130, in the driver that fix did not cover. UiSelector chains, so the constraints are now built as fragments and combined into one query when both are given. Ordering is preserved: exact id before substring, textContains before the case-insensitive fallback, and clickable-first for taps. Single-attribute selectors are unchanged.
The DeviceLab Android builder had the same OR as uiautomator2: resourceId strategies and text strategies were emitted as independent candidates, and the finder returns on the first that hits anything. Combined the same way, preserving the hint* extensions and the extra regex tier. Note this is necessary but not sufficient: with the chained selector `resourceId(X).textContains(Y)` the on-device agent still resolves by resourceId alone and ignores the text predicate, so an id+text assertion still passes against an element with the right id and the wrong text. That needs an agent-side fix; the Go side is correct now either way.
RieBi
changed the title
omnarayan
added a commit
that referenced
this pull request
Aug 29, 2026
The third driver with #157's bug, after the two #158 fixed. findElementDirect tries the id first and returns on the first match, so a selector naming both an id and a text answered on the id alone and never read the text — an assertVisible with the right id and a wrong text passed green. Routed to page source when both are given, which is where a selector's fields are all checked together. That is the same route state filters already take a few lines above, for the same reason: the direct queries each speak for one attribute, and no combination of them can express "the element with both". It costs a page-source fetch on those selectors. The alternative was a chained UiSelector for Android and a combined predicate for iOS — a third dialect of selector composition alongside the fragments #158 introduced, to be kept in agreement with them forever. Correctness through the matcher everything else already agrees with is worth the round trip. Follows @RieBi's #157 and #158, which found and fixed the same fault in UIAutomator2 and DeviceLab.
pr4bh4sh
added a commit
to pr4bh4sh/maestro-runner
that referenced
this pull request
Aug 30, 2026
* fix(report): a step's label wins over its selector in the HTML report `label:` is the author saying, in their own words, what a step is for. In the HTML report it was consulted last — after the selector, the text and the direction — so it only ever appeared on steps that had none of those to show instead. That produced the reported symptom exactly: in one flow, a labelled extendedWaitUntil displayed its label while a labelled tapOn showed its selector and a labelled assertVisible showed its matched text. Same syntax, three different outcomes, and no way to tell from the YAML which you would get (devicelab-dev#150). The label now comes first. JUnit and Allure already preferred it, so this also ends the HTML report disagreeing with the other two about what a step is called. Verified on a generated report by running the report's own extractKeyValue over real flow data: before, three of four labelled steps showed a derived value; after, all four show the label, including non-ASCII text. Reported by @SmileYang966. * fix(selectors): anchors mean something, and regex queries stop 404ing Two faults, one root: the WDA query path and the page-source path did not agree on what a text selector means, and neither agreed with Maestro (devicelab-dev#151). A regex selector was interpolated verbatim into WDA's NSPredicate, so `^SIGN OUT$` asked for a literal caret via `CONTAINS[c]` and every query 404'd — six failed round trips per step before the page-source path resolved it anyway. Regex selectors now go straight to page source. NSPredicate does have MATCHES, but its ICU dialect is not Go's, and a third opinion about what a pattern means is the last thing this needed. Measured on a simulator, an anchored selector goes from 2.6s to 44ms. The costly half was quieter. The page-source matcher compiled every regex with (?i), so an anchored pattern could not distinguish what it was written to distinguish: `^SIGN OUT$` matched a "Sign out" row exactly as readily as the "SIGN OUT" button, and whichever appeared first in the page source won. The reporter had a flow tap the row behind an open dialog, report success, and fail several steps later somewhere unrelated. Regex selectors are now matched case-sensitively on all four drivers, as Maestro does. Scope of the behaviour change is narrower than it sounds: `looksLikeRegex` only fires on actual metacharacters, so plain selectors like `tapOn: "sign out"` are untouched and still match case-insensitively. Only patterns written deliberately as regexes change, which is exactly where the old behaviour was wrong. A pattern that wants insensitivity can still ask: `(?i)^sign out$`. Verified on a simulator both ways: `^welcome back$` passed against text reading "Welcome Back" on 1.1.25, and now fails. Reported by @Ahlisen, with a reproducer and a diagnosis that were both exactly right. * perf(flutter): stop re-serialising the widget tree on every poll The Flutter fallback re-fetched the app's entire widget tree continuously for the whole of every element search — 2.9MB twice a second on the reporting user's app, for as long as the search ran. Steps that took under 700ms with the fallback disabled took 4-9s with it on (devicelab-dev#152). Two causes, both fixed. The widget dump was fetched up front on every poll, though it is consulted only when the semantics search finds nothing. Searches the semantics tree could answer — the common case — paid megabytes for a result they never used. It is now fetched after that search misses. The poll loop had no delay at all. It spun as fast as the VM Service would answer, so a search running to timeout hammered the connection for the entire find window while contending with the app it was inspecting. It now paces at 300ms, chosen to stay under the inner driver's 1s poll so Flutter still answers first when it can. A tree that has not repainted cannot start matching, so polling faster than the app can change buys nothing. Measured on a real Flutter app over one 6-second search: 362 widget-tree fetches before, 46 after. Reported by @Ahlisen, with measurements. The second half of that report — a tapOn reporting success without issuing a tap — is not addressed here and remains open. * fix(appium): match Android checked state (devicelab-dev#154) The page-source parser dropped the Android checked attribute, so state filtering compared checked: against the unrelated selected state. A switch whose checked and selected disagree — the normal case for a Switch that is on but not focused — could not be matched at all. Scoped to the Android parse path; XCUITest has no checked attribute and reports switch state in value. * fix(android): checked: reads the checked state on the native drivers too Follow-up to devicelab-dev#154, which fixed this on the Appium driver and left UIAutomator2 and DeviceLab with the same bug — `checked:` behaving differently depending on which driver ran the flow is worse than it being wrong everywhere. All three compared `checked:` against the unrelated `selected` state, and none of them parsed the Android `checked` attribute at all. A Switch that is on but not focused reports checked="true" selected="false", which is the ordinary case, so the selector could not match it — and would happily match a different, unchecked element that happened to be selected. Test fixture makes the two disagree in both directions, on both drivers. Credit to @nt-ben-leblond, whose devicelab-dev#153 identified the comparison and whose devicelab-dev#154 carries the fix this extends. * build: -trimpath in the Makefile too The commit that introduced this covered the release script and said it covered the Makefile as well; the Makefile change was left unstaged. Without it a `make build` binary still carries the build machine's absolute paths, which is the disclosure devicelab-dev#149 arrived with. * fix(android): require id and text on the same element in both drivers (devicelab-dev#158) The finder returns on the first strategy that matches, and id strategies were emitted independently of text strategies — so whenever the id matched, the text was never read, and assertVisible with a right id and a wrong text passed green. Composing the constraints as fragments means no query can match on one attribute alone. Android counterpart of the iOS fix in devicelab-dev#130. * build: compile for Windows `GOOS=windows go build ./...` failed on one line, in the iOS driver — the last place a Windows user would look: pkg/driver/devicelab_ios/setup.go:275: unknown field Setpgid in struct literal of type syscall.SysProcAttr `Setpgid` is Unix-only; the Windows SysProcAttr has no such field. It was the sole occurrence in the repository, so a single build-tagged helper is the whole fix: the Unix build keeps putting xcodebuild in its own process group, and the Windows build gets a no-op. Nothing is lost by that no-op. Windows has no process groups in the POSIX sense, and this package drives xcodebuild against iOS simulators, so it never runs there. The point is only that the package compiles — which is what the rest of maestro-runner needs in order to build for a platform where the Android and web drivers work perfectly well. The Makefile has had a build-windows target throughout, so this was a target nobody could run. All six targets cross-compile now: darwin, linux and windows on arm64 and amd64. Reported by @RieBi, with the fix. * fix(appium): require id and text on the same element The third driver with devicelab-dev#157's bug, after the two devicelab-dev#158 fixed. findElementDirect tries the id first and returns on the first match, so a selector naming both an id and a text answered on the id alone and never read the text — an assertVisible with the right id and a wrong text passed green. Routed to page source when both are given, which is where a selector's fields are all checked together. That is the same route state filters already take a few lines above, for the same reason: the direct queries each speak for one attribute, and no combination of them can express "the element with both". It costs a page-source fetch on those selectors. The alternative was a chained UiSelector for Android and a combined predicate for iOS — a third dialect of selector composition alongside the fragments devicelab-dev#158 introduced, to be kept in agreement with them forever. Correctness through the matcher everything else already agrees with is worth the round trip. Follows @RieBi's devicelab-dev#157 and devicelab-dev#158, which found and fixed the same fault in UIAutomator2 and DeviceLab. * fix(flow): parse flow files with Windows line endings A flow file with CRLF failed on its first line: invalid steps: yaml: unmarshal errors: line 1: cannot unmarshal !!map into []yaml.Node The document splitter divided on "\n", leaving a trailing "\r" on every line. Its separator test compares the untrimmed line against "---" — deliberately, to tell a real document break from an indented "---" inside a block scalar — and "---\r" fails that. So the break was never found, the file parsed as a single document, and the header's map was handed to the step list. Windows checks out with core.autocrlf=true by default, which made this every flow on Windows rather than an edge case. Normalising CRLF before the split fixes it, and a test pins that a "---" inside a block scalar is still content rather than a separator. Also builds the expected path in TestScriptEngine_ResolvePath with filepath.Join instead of a literal "/". ResolvePath joins with the host separator, so the hard-coded form failed on Windows against the identical answer spelled with backslashes. Both found by @RieBi while verifying the Windows build in devicelab-dev#159, along with the end-to-end confirmation that Android flows do run there. --------- Co-authored-by: Om Narayan <omnarayan@robustest.com> Co-authored-by: Ben LeBlond <ben.leblond@ninjatrader.com> Co-authored-by: Sviatoslav Zubar <riebisv@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #157
The bug
On Android, a selector naming both an
id:and atext:did not require them on the same element.buildSelectorsWithOptionsappendedresourceIdstrategies and then independenttext/descriptionstrategies, and the finder returns on the first strategy that finds anything — so whenever the id matched, the text was never read.The consequence is the dangerous direction:
assertVisiblewith the right id and a wrongtext:passed green, masking a wrong displayed value. The mirror case fails instead: with several elements sharing an id,assertNotVisiblereported a match for an element that was not present.Same defect fixed for iOS/WDA in v1.1.25 (#130) — "the WDA finder returned as soon as it found an element with the given id (never checking text)" — in the drivers that change did not cover. Both Android builders had it:
uiautomator2anddevicelab.The change
In both drivers the id and text constraints are built as fragments, and when both are given only their combination is emitted, so no query can match on one alone.
UiSelectorchains natively, so this is one query rather than a post-filter.Ordering is preserved:
resourceIdbefore theresourceIdMatchessubstring fallbacktextContainsbefore the case-insensitivetextMatchesfallbackhintContains/hintMatchesextensions and its extra regex tierSelectors naming only an id, or only a text, emit exactly what they did before.
One caveat, on DeviceLab
The Go side is correct for both drivers now, but for
--driver devicelabthis is necessary and not sufficient. Given the chained selectornew UiSelector().resourceId(X).textContains(Y), the on-device agent still resolves byresourceIdalone and ignores the text predicate.Measured on an Android emulator (API 36), with an id and a text that exist on different elements — this must fail:
uiautomator2devicelabWith both id and text absent, DeviceLab correctly fails, so it is specifically the chained predicate being dropped. That looks like an agent-side change I can't make from here — happy to file it separately if you'd prefer it tracked on its own.
Tests
pkg/driver/uiautomator2/idtext_and_test.goandpkg/driver/devicelab/idtext_and_test.go, mirroringpkg/driver/wda/idtext_and_test.go:TestBuildSelectors_IDAndTextAreANDed— every emitted strategy constrains both. Fails against the previous code, printing the independent OR queries it produced.TestBuildSelectors_SingleAttributeUnchanged— id-only still emits the exact and substring queries, text-only the text queries. Passes before and after, which is what makes this a fix rather than a behaviour change.TestBuildSelectorsForTap_CombinedKeepsClickableFirst— the clickable preference survives the combination.go test ./pkg/driver/uiautomator2/... ./pkg/driver/devicelab/...passes. On a device,--driver uiautomator2now behaves the same as the JVM CLI for all three cases.