Skip to content

fix(appium): match Android checked state - #154

Merged
omnarayan merged 1 commit into
devicelab-dev:mainfrom
nt-ben-leblond:fix/appium-checked-selector
Aug 29, 2026
Merged

fix(appium): match Android checked state#154
omnarayan merged 1 commit into
devicelab-dev:mainfrom
nt-ben-leblond:fix/appium-checked-selector

Conversation

@nt-ben-leblond

Copy link
Copy Markdown
Contributor

Summary

Fix Android Appium checked: selectors so they read the hierarchy checked attribute instead of the unrelated selected state.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change or feature that changes existing behavior
  • Documentation update
  • Refactoring (no functional changes)

Changes Made

  • Retain Android XML checked state in ParsedElement.
  • Compare Selector.Checked with the parsed checked state.
  • Add a regression fixture where checked and selected deliberately disagree in both directions.
  • Document the fix under Unreleased.

Related Issues

Fixes #153

Testing

  • Full make test passes locally
  • Full linting passes
  • Added tests for new functionality
  • Tested manually with sample flows

Passed locally:

  • go test ./pkg/driver/appium -count=1
  • go test -race ./pkg/driver/appium -count=1
  • go vet ./pkg/driver/appium
  • go build .
  • Original Sauce Labs reproduction with checked: true before the tap and checked: false after it: 80/80 steps passed on Pixel 8 / Android 14 — https://app.saucelabs.com/tests/e95b01337f2145fd8c02d14c5cd0990f

The repository-wide go test ./... run encountered an unrelated environment-sensitive failure in TestListAvailableAVDs_NoEmulatorBinary because this workstation has Android AVDs installed, then was bounded while the browser/CDP package ran its first-time Chromium tests. The complete changed Appium package passes, including under the race detector.

Checklist

  • Code follows project style guidelines
  • Self-reviewed the code
  • Added/updated documentation as needed
  • No breaking changes
  • CHANGELOG.md updated

Additional Notes

The production change is two behaviors: parse checked, then filter against it. The regression exercises the same XML shape observed through Appium on Sauce Labs (checked=true, selected=false).

Parse the Android checked attribute separately from selected and use it for checked selectors. Add a regression where the two states deliberately disagree.\n\nFixes devicelab-dev#153.
@omnarayan
omnarayan merged commit 624166b into devicelab-dev:main Aug 29, 2026
omnarayan added a commit that referenced this pull request Aug 29, 2026
Follow-up to #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 #153 identified the comparison and whose #154
carries the fix this extends.
@omnarayan

Copy link
Copy Markdown
Contributor

Merged — thank you. Clean diagnosis, a fixture that makes checked and selected disagree in both directions, and a real-device run; that's about as complete as a bug fix gets.

One note on scope, in case it comes up: you added the checked case to the Android parse path only, which is right. The second case "selected" in that file is the XCUITest path, and iOS has no checked attribute — it reports switch state in value.

I've followed up in fa63a3b extending the same fix to the UIAutomator2 and DeviceLab drivers, which had the identical comparison and also never parsed the attribute. checked: behaving differently depending on which driver ran the flow seemed worse than
it being uniformly wrong. You're credited there.

And thanks for flagging TestListAvailableAVDs_NoEmulatorBinary — that failure is ours, not yours. It asserts no emulator is on PATH, so it fails on any machine that has the Android SDK installed. It's on the list.

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>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Appium checked selector compares the selected attribute on Android

2 participants