Skip to content

Fix childOf selectors not refreshing hierarchy#3242

Draft
Fishbowler wants to merge 1 commit intomainfrom
childOf-selector-fix
Draft

Fix childOf selectors not refreshing hierarchy#3242
Fishbowler wants to merge 1 commit intomainfrom
childOf-selector-fix

Conversation

@Fishbowler
Copy link
Copy Markdown
Contributor

Proposed changes

A user found a timing issue with chidOf selectors.

This failed:

- tapOn: Close
- copyTextFrom:
          traits: text
          childOf:
            id: parent

This succeeded:

- tapOn: Close
- assertVisible:
         id: parent
- copyTextFrom:
          traits: text
          childOf:
            id: parent

Investigation of the code found that the codepath for discovering the parent in childOf took a single hierarchy snapshot and evaluated it repeatedly in a pointless loop. Using the assertVisible against the parent was sensible waiting for the parent to become visible (you know, with actual checking), before the flow then progressed.

This change fixes this oversight by having the childOf path more closely match what the regular path does.

I asked my friend Claude about the performance difference in this change:

  Old code (single-level childOf, parent found but child not found):
  1. findElementViewHierarchy(childOf, timeout) — 1 driver call to get a snapshot, then findElementWithTimeout(timeout, parentFilter, snapshot) which, since initialHierarchy != null, immediately finds the parent in the static snapshot (or spins fruitlessly if it's not there, for up to timeout ms)
  2. If parent found quickly: findElementWithTimeout(timeout, childFilter, parentSubtree) — spins for up to timeout ms against the static subtree
  3. Total worst case: 1× timeout

  New code (same scenario):
  - Single withTimeoutSuspend(timeout) loop. Each iteration fetches one fresh hierarchy and resolves both parent and child.
  - Total worst case: 1× timeout

  So no double-wait regression for the common single-level case.

  Where the old code was actually worse: For deeply nested childOf chains (childOf: { childOf: { childOf: ... } }), findElementViewHierarchy was recursive and each level called findElementWithTimeout with the same full timeout. That could stack up to N × timeout on failure. The new code resolves the entire
   chain synchronously within a single timeout loop, so it's strictly better there.

  The one real trade-off: On failure, the old code made 1 driver call (the initial snapshot) then CPU-spun uselessly for timeout ms. The new code makes many driver calls (one per loop iteration, each ~100–500ms on device) during the same timeout window. More I/O, but it's the correct I/O — those are
  genuine retries against live data. The failure window duration is the same.

  So: same failure duration for the simple case, faster failure for nested childOf, more driver traffic during failure windows. Net positive.

That matches my thinking.

More I/O, but it's the correct I/O

👍

Testing

Added unit tests.

Issues fixed

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.

1 participant