Repository navigation
Conversation
This was referenced Sep 29, 2026
bulatgaleev
force-pushed
the
upstream-pr/wda-notvisible-needs-a-miss
branch
from
September 29, 2026 18:31
a866498 to
5a75372
Compare
10 of 16 tasks
bulatgaleev
force-pushed
the
upstream-pr/wda-notvisible-needs-a-miss
branch
from
September 30, 2026 01:41
5a75372 to
3486b7e
Compare
The WDA driver reads an element's name, rect, text and displayed in parallel, and a tap looks an element up four ways at once. The client used Go's default transport, which keeps two idle connections per host, so every such burst closed two connections and opened two new ones. On a real iPhone reached through a forward (SSH, then iproxy over USB) a new connection costs about 300 ms, and new ones opened together fail at once with EOF. In one 44-flow run, 297 of 996 element bursts lost exactly two reads (the two new connections), 2 lost one, and none lost a read on a kept connection: 976 EOFs, each sent again by the dropped-connection retry. WebDriverAgent does not close an idle keep-alive connection (FBHTTPServer exempts them from its reaper), so a stale reuse was not the cause. The client now keeps up to eight idle connections per host. In the new tests, 50 bursts of four reads open 4 connections instead of 102, and through a server that drops every new connection past the first four, no read fails (the old client lost 23 reads there, after 196 dropped connections).
Maestro runs a retry's commands once and then up to maxRetries more times:
`(maxRetries?.toIntOrNull() ?: 1).coerceAtMost(3)`, then
`while (attempt <= maxRetries)` from 0 (Orchestra.kt:934-955, with
MAX_RETRIES_ALLOWED at 1844 and the "1" default at YamlFluentCommand.kt:614).
So `maxRetries: 1` is two attempts, the default is two, and no retry runs
more than four times. The value is evaluated first, so ${...} works
(Commands.kt:986), and anything that is not an integer counts as 1.
The runner ran exactly maxRetries attempts, three when unset, with no cap,
so a flow written with `maxRetries: 1` never retried at all. It also failed
the step when maxRetries was not an integer. The inline and the file form
of retry now count as Maestro does. A negative maxRetries runs no attempt
and passes, since Maestro's loop never starts. A value that is not an
integer is logged and read as 1.
Maestro stops the app before launching it unless the flow says `stopApp: false`. The WDA driver never read stopApp: with a session open and no launch arguments it called WDA's launch on the running app, which only activates it. A relaunch then left the app on the screen it was already on, so a flow checking what survives a restart (a chat's history, or a changed subscription state being read back on launch) was not restarting anything. It now terminates first unless stopApp is false. Launch arguments still force the stop, since they only apply on a real launch. A launchApp that creates the session is unchanged: creating it launches the app fresh.
Maestro's notVisible waits for a lookup that ends in ElementNotFound
(Orchestra.kt:1066-1082); any other error from the lookup fails the step.
assertNotVisible and extendedWaitUntil notVisible treated any failed lookup
(err != nil || info == nil) as the element being gone. A page source that
could not be read, from a WDA error or a dropped connection, passed the
assertion without the screen ever being looked at. The when/while and
assertCondition notVisible checks go through assertNotVisible, so they had
the same hole.
The page-source lookups now return a not-found error when they read the
screen and nothing matches (an anchor that is not there included). Only that
counts as gone. Any other error is tried again until the timeout, and the
step then fails with that error ("Could not check that ... is not visible").
TestAssertNotVisibleWithTimeout served an error for every request and
expected a pass; it now serves an empty screen.
bulatgaleev
force-pushed
the
upstream-pr/wda-notvisible-needs-a-miss
branch
from
September 30, 2026 14:49
3486b7e to
8ccc104
Compare
Contributor
|
Thank you @bulatgaleev. Good catch: a page source that could not be read should not pass It can make WDA flows that were passing on a failed read fail now. That's the correct result, and we'll mention it in the release notes. Merging into |
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.
Summary
Maestro's
notVisiblewaits for a lookup that ends inElementNotFound(Orchestra.kt); any other lookup error fails the step.assertNotVisibleandextendedWaitUntil: notVisibletreated any failed lookup (err != nil || info == nil) as the element being gone, so a page source that could not be read, from a WDA error or a dropped connection, passed the assertion without the screen ever being looked at. Thewhen/whileandassertConditionnotVisible checks go throughassertNotVisibleand had the same hole. Now only a lookup that read the screen and found nothing counts as gone.Type of Change
Changes Made
TestAssertNotVisibleWithTimeoutserved an error for every request and expected a pass; it now serves an empty screen.pkg/driver/wda/notvisible_test.go, and a CHANGELOG.md entry.Related Issues
No existing issue found.
Testing
go test ./pkg/driver/wda/go vet ./...andmake fmt-checkpass (run on the top of the stack, fix(wda): checked selectors work on iOS #185, which contains all five changes)make testas a whole: not run, because its device tests drive whatever device is attached to the machinemake lint: the Makefile has nolinttarget, and the lintersmake checkruns (staticcheck, revive, errcheck, nilaway, gosec) are not installed hereChecklist
Additional Notes
A
notVisiblecheck that used to pass because the screen could not be read now retries, and fails with the lookup's error if the screen still cannot be read at the timeout.Stack 4 of 5: #181 → #182 → #183 → #184 → #185. Merge in that order. This branch is built on #181, #182 and #183, so it also contains their commits. This PR's own change is the top commit,
8ccc104. Once the PRs below it merge, the rest of the diff disappears, and nothing conflicts. All five sit onmainatfe3dd53(the 1.1.28 release).