fix: use localhost (not 127.0.0.1) for XCTest server connection (#1299)#3166
Open
qwertey6 wants to merge 6 commits intomobile-dev-inc:mainfrom
Open
fix: use localhost (not 127.0.0.1) for XCTest server connection (#1299)#3166qwertey6 wants to merge 6 commits intomobile-dev-inc:mainfrom
qwertey6 wants to merge 6 commits intomobile-dev-inc:mainfrom
Conversation
iOS simulators share the host's localhost, causing port collisions when
multiple Maestro processes target different sims simultaneously. Session
tracking was per-platform, so two processes on different devices would
interfere with each other's sessions.
Changes:
- Per-device session tracking: SessionStore keys are now
"{platform}_{deviceId}_{sessionId}" instead of "{platform}_{sessionId}"
- Add --driver-host-port CLI flag for explicit XCTest server port
- Auto-select available ports with isPortAvailable() check
- Refactor SessionStore from singleton to injectable class (DI)
- Add shouldCloseSession(platform, deviceId) for per-device shutdown
instead of global activeSessions().isEmpty()
- Add cross-process file locking to KeyValueStore (~/.maestro/sessions)
- Append PID to debug log directory to prevent parallel race
- Enable useJUnitPlatform() in maestro-cli (was missing)
- Add SessionStoreTest with 8 tests covering isolation and lifecycle
Verified: 3 iOS simulators + Android emulator running simultaneously,
all passing. Both --driver-host-port (explicit) and auto-port-selection
work correctly.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Default --reinstall-driver to false: reuse a healthy running driver instead of killing and reinstalling on every run (~40s saved on iOS) - XCTestDriverClient checks isChannelAlive() before reinstalling — if the user explicitly passes --reinstall-driver, honor it - Cache extracted iOS build products per-device in ~/.maestro/build-products/<deviceId>/ with SHA-256 hash validation: skips extraction when source matches cache, re-extracts on upgrade - Reduce XCTest status check HTTP read timeout from 100s to 3s - Remove Thread.sleep(1000) heartbeat delay hack (no longer needed with per-device session tracking) Single device: ~52s → ~10-12s. Three devices parallel: ~54s → ~18s. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- iOS XCTest runner: add isVersionMatch() to XCTestInstaller interface. LocalXCTestInstaller compares SHA-256 hash of build products against a .running-hash marker written at startup. restartXCTestRunner now checks both isChannelAlive() AND isVersionMatch() — stale runners from a previous Maestro version are replaced automatically. - Android driver: add isDriverVersionCurrent() that hashes the bundled maestro-app.apk and maestro-server.apk, compares against stored hash in ~/.maestro/android-driver-hash. On mismatch, APKs are reinstalled even when reinstallDriver=false. - App binary cache (clearAppState): getCachedAppBinary now compares Info.plist of cached vs installed app. Stale cache from app updates is detected and refreshed before reinstall. Per-device cache dirs (~/.maestro/app-cache/<deviceId>/) prevent parallel races. - Add XCTestDriverClientTest (4 tests) and LocalSimulatorUtilsTest (3 tests) covering version mismatch, reuse, and cache behavior. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On iOS, waitForAppToSettle has two tiers: a server-side screenshot hash check (Tier 1, hardcoded 3000ms) and a client-side hierarchy comparison fallback (Tier 2). The per-command waitToSettleTimeoutMs config only controlled Tier 2, so even waitToSettleTimeoutMs: 100 would still burn up to 3 seconds in Tier 1. Fix: use waitToSettleTimeoutMs as the total settle budget. Tier 1 runs with this timeout, and any remaining time goes to Tier 2: - swipe with waitToSettleTimeoutMs: 500 → capped at 500ms total - default (no config) → unchanged 3000ms behavior Wikipedia e2e flow with tuned timeouts: 25s vs 53s baseline. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
On iOS, when a React Native Pressable has accessibilityLabel set, the
child Text content is collapsed into the parent's accessibility label.
The element's title and value are empty, so Maestro's `text` attribute
was always empty for these elements — making `tapOn: "<text>"` fail to
find buttons that are clearly visible to users.
The element IS reachable via `tapOn { label: ... }` or by regex against
accessibilityText, but those are unintuitive workarounds. Users see text
on screen and expect `tapOn: "that text"` to work — that's the entire
point of the selector.
Fix: in mapViewHierarchy, fall back to element.label (the iOS
accessibility label) when both title and value are empty. accessibilityText
still uses element.label as its canonical source, so the existing
Filters.textMatches accessibilityText fallback continues to work.
This also indirectly fixes the "tap doesn't fire onPress" symptom: when
matching by accessibilityText regex, Maestro might select a parent View
wrapping the Pressable, leading to coordinate taps in the wrong area.
With text populated on the Pressable itself, normal element ranking
picks the correct deepest match.
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The XCTest runner sometimes binds its HTTP server to ::1 (IPv6) only. Maestro CLI was hardcoded to connect via 127.0.0.1 (an IPv4 literal), which cannot reach an IPv6-only socket. Result: every HTTP call fails with "Connection refused" even though the runner is alive and curl can reach it via localhost. Fix: replace 127.0.0.1 with localhost in three places: - MaestroSessionManager.defaultXctestHost - LocalXCTestInstaller constructor default - LocalXCTestInstaller.xcTestDriverStatusCheck (was hardcoded) okhttp's default Dns resolver returns all addresses for localhost (both 127.0.0.1 and ::1) and tries them in order on connection failure, so this works regardless of which address family the runner binds to. This is the same root cause as mobile-dev-inc#1299 (open since July 2023). Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Contributor
|
This dependency chain of PRs is a little unusual... It means they can't be merged out of order. If the first PR is the hardest to get to the finish line, then it blocks everything else. Worse, what if something in the chain is outright rejected? |
Author
|
Fair point — the chain is large. A few clarifications: PRs 1-3 have a real dependency (parallel sessions → driver reuse → version safety for that reuse). PRs 4-7 are independent bug fixes that can be rebased onto That said, the chain exists because each fix exposed the next bug during testing. For example, #3167 (port persistence) only became necessary because #3139 (driver reuse) made it possible for the runner to outlive the CLI process, which then collided with #3166 (this PR — IPv6 binding mismatch). If there's a specific PR you'd like to review first, or if you'd prefer I split the independent ones off the chain, let me know. I can restructure however works best for your review process. For what it's worth, the individual changes are small — this PR is 3 lines changed in 2 files. |
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.
Proposed changes
One-line fix for a 2.5-year-old bug. Maestro CLI was hardcoded to connect to the XCTest runner via
127.0.0.1(an IPv4 literal). When the runner happened to bind to::1(IPv6) only — which can happen with newer Xcode/macOS combinations — every HTTP call failed withConnection refused, even though the runner was alive andcurl localhost:<port>worked.Impact
xcuitest.XCTestDriverClient$XCTestDriverUnreachable#1299 (open since July 2023, "XCUITest Server unreachable")Root cause
iOS XCTest runner sometimes binds its HTTP server to
::1(IPv6) only:But Maestro CLI connects via
127.0.0.1(IPv4 literal):127.0.0.1is IPv4-only — it cannot reach an IPv6-only socket. Connection refused.Why
localhostworks/etc/hostson macOS already has both:InetAddress.getAllByName("localhost")returns both addresses. okhttp's defaultDns.SYSTEMresolver tries each address in order on connection failure, so alocalhostURL works regardless of which address family the server binds to.Changes
Three places hardcoded
127.0.0.1:MaestroSessionManager.defaultXctestHost— was"127.0.0.1", now"localhost". (Note:defaultHostfor the dadb path was already"localhost"— this fix just makes the XCTest constant consistent.)LocalXCTestInstallerconstructor default — was"127.0.0.1", now"localhost"LocalXCTestInstaller.xcTestDriverStatusCheck— had hardcoded"127.0.0.1"even whenhostparameter was passed; now useshostVerification
127.0.0.1:7075tolocalhost/[0:0:0:0:0:0:0:1]:7117, confirming Maestro now uses IPv6 successfully)XCTestDriverClientTestalready used"localhost"for its fake)Issues fixed
Closes #1299
Likely closes #2932