Repository navigation
test: refresh text targets and return from connections - #288
Conversation
talosmachina
left a comment
There was a problem hiding this comment.
No findings. expectTextWithin now re-resolves the ancestor on every positive probe, so it follows ActivityShort-N when the id moves to a replacement row. The Lightning close-channel step now taps NavigationBack on iOS as well as Android. Reviewed 0affc996, full tier, static.
What I checked, and 4 candidates I ruled out
Read in full: test/helpers/actions.ts (elementById, expectTextWithin, tap), test/specs/lightning.e2e.ts, test/unit/activity-text.test.ts, and webdriverio's waitUntil timer (node_modules/webdriverio/build/index.js).
Not run here: the PR's code ran nowhere on this side, because the sandboxed runner could not confirm the author's permission and fails closed. This repo reports no CI checks. The evidence for the change is the paired iOS run linked in the body. The author notes there was no new Android device run.
Ruled out
- A stale or missing row throws out of the probe:
isDisplayed()returns false for an element that does not exist. An error fromgetAttributeon a stale element is recorded by webdriverio'sTimer._checkCondition, and the probe is scheduled again until the timeout, so a transient stale element retries instead of failing. - Negative path regressed:
visible: falsestill checks the iOS parent label/value once, then waits for the parent or the descendant to disappear. That is the same branch structure as onmain, only reordered. - Longer worst-case wait: the parent wait plus the probe loop can take up to two timeouts, the same as
main's parent wait plus the descendant wait. - iOS
NavigationBackon an app without the fix: the expected return to Connections comes from bitkit-ios#1054, which is merged onmasteras32ad1977.lightning.e2e.tsis@ios_gate, so it runs only in the app repo's merge gate, against branches that carry or will rebase onto that commit.
Merge confidence: 4/5. No findings, but the helper change runs on all 148 expectTextWithin call sites in the specs, and it was not exercised here or on an Android device.
piotr-iohk
left a comment
There was a problem hiding this comment.
QA review
Scope: full PR review at 0affc996 against 4c0cbef0, including shared-helper callers, platform selectors and CI selection.
No new actionable code findings.
Validation: all 37 unit tests passed. Both replacement-row regressions fail against the base helper and pass at the reviewed head; changed-file ESLint and Prettier checks passed. Existing iOS CI logs confirm this E2E SHA ran with 13 passing cases, including Lightning, and one hardware-transfer skip.
Compatibility checks covered activity selectors and channel-close/Back/drawer navigation in iOS e86c270f and Android e28307b9; these were not full app reviews. The iOS step relies on the companion navigation repair. No local Appium execution or new Android runtime validation was performed.
Reviewer model: gpt-6.1-sol
Reasoning effort: high
expectTextWithincould keep checking an old activity row after a changing list moved its identifier to a replacement element. Resolve the named parent on each positive text probe so the assertion follows the current row on iOS and Android. The initial parent-display wait, timeout, text matching and negative disappearance checks are preserved. The Lightning spec also uses its existing Back step on iOS before opening the wallet drawer, matching the repaired app’s return to the connections list; Android already performed that step.Four helper tests cover iOS merged-label replacement, Android descendant replacement, missing/exact-match failure and the existing disappearance behavior. Both replacement tests fail with the old helper and pass with this change; all 37 existing and new unit tests pass.
Validation: complete ESLint and Prettier checks inspected. The three changed files are clean. ESLint still reports 260 existing errors, and Prettier reports three existing spec files; every reported file is byte-identical to main at
4c0cbef. Current paired iOS validation passed 13 cases on trial 1, 0 failed, 1 explicit hardware-transfer skip, with no test retries. It executed suite0affc996against #1054 app heade86c270f, using the verified original app archive. The older suite0deb9daafailed three trials on the post-close HeaderMenu lookup; those failures remain retained. Complete current logs were audited: 77 stale-element warnings, four native collector restarts and unchanged dependency notices remain disclosed. This is not a new Android device run or manual QA/field-latency result.Companion: bitkit-ios#1054. Its earlier failed run displayed the expected 9,000-sat transfer while the retained activity handle timed out. The separate iOS navigation repair fixes the earlier close-screen/toast failure. The Back step expects #1054’s corrected return to Connections; the app change was externally merged as
32ad1977before this E2E PR was opened. The matching feature branches provide the paired validation.Models used
gpt-6.1-sol(reasoning:xhigh)gpt-6.1-sol(reasoning:high)