Skip to content

chore: make Maestro E2E tests more stable (iOS + Android) - #13789

Open
MounirDhahri wants to merge 11 commits into
mainfrom
chore/make-e2e-tests-more-stable
Open

chore: make Maestro E2E tests more stable (iOS + Android)#13789
MounirDhahri wants to merge 11 commits into
mainfrom
chore/make-e2e-tests-more-stable

Conversation

@MounirDhahri

@MounirDhahri MounirDhahri commented Jul 20, 2026

Copy link
Copy Markdown
Member

This PR resolves []

Description

The iOS Maestro E2E suite has been ~50% flaky on main. Reading the last ~20 runs, the failures fall into three buckets — only one is a real test issue, the other two are infra/timing noise:

  • Bucket A — setup openurl timeout kills the job (code 60): the "Prefetch app associations" step hits the real network via xcrun simctl openurl; when artsy.net is slow it times out and, under set -e, fails the whole job before any test runs.
  • Bucket B — search-button not visible after login: the post-auth HomeView assertions used bare assertVisible (short default timeout), so a slow post-login render intermittently failed.
  • Bucket C — deeplinks flakiness: deeplinks.yml round-trips through Safari and depends on live universal-link resolution + real network data — structurally the flakiest flow.
  • Bucket D — native "rate the app" prompt: the in-app review prompt fires on the 5th app launch and overlays HomeView, blocking assertions (e.g. the post-signup search-button). Reproduced locally: the native "Enjoying Artsy?" review card sits on top of HomeView.

Each fix is a separate commit so they can be reviewed / reverted independently:

Commit Fix Bucket
make iOS app-association prefetch non-fatal prefetch retries and never fails the step A
wait up to 30s for HomeView search-button extendedWaitUntil in login/signup/deeplinks B
retry a failed Maestro flow once shard runner retries each flow once from a clean launch B/C
make config.yml a valid Maestro workspace config fixes local Commands Section Required error
pin iOS CI Maestro to 2.6.1 aligns CI with local dev
make deeplinks flow non-blocking flaky flow warns instead of failing the shard C
suppress rate-the-app prompt during Maestro runs skip in-app review prompt when useMaestroInit is set (+ jest mock) D
pin Android CI Maestro to 2.6.1 aligns Android with iOS/local
upload Maestro screenshots as artifacts only on failure if: failure() artifact upload of ~/.maestro/tests (both platforms)
raise iOS Maestro driver startup timeout to 240s avoids IOSDriverTimeoutException on cold driver installs (Xcode 26) A
document Maestro run commands and flakiness handling AGENTS.md / e2e docs

On failure, each shard now uploads its Maestro screenshots + command logs as a GitHub Actions artifact (maestro-<platform>-shard-N) — findable directly from the run page, and nothing is uploaded on green runs.

Several fixes live in shared files (the flow YAMLs, run_maestro_shard, and the cross-platform rate-prompt suppression), so Android benefits too — its occasional failures were the same short-timeout search-button assertion plus emulator infra noise.

Follow-up work: deeplinks.yml being non-blocking means a real deeplink regression only shows as a ⚠️ in the shard log rather than failing CI. Android's residual flakiness is emulator ColorBuffer GPU errors (infra), mitigated by the per-flow retry.

PR Checklist

  • I have tested my changes on the following platforms:
    • Android.
    • iOS.
  • I hid my changes behind a feature flag, or they don't need one.
  • I have included screenshots or videos at least on Android, or I have not changed the UI.
  • I have added tests, or my changes don't require any.
  • I added an app state migration, or my changes do not require one.
  • I have documented any follow-up work that this PR will require, or it does not require any.
  • I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

  • I would like at least one of the reviewers to run this PR on the simulator or device.
Changelog updates

Changelog updates

Dev changes

  • Stabilize Maestro E2E tests (iOS + Android): non-fatal app-association prefetch, longer HomeView waits, per-flow retry, non-blocking deeplinks, suppressed rate-the-app prompt in E2E, Maestro 2.6.1 alignment across CI/local, and screenshot artifacts uploaded only on failure.

Need help with something? Have a look at our docs, or get in touch with us.

Assisted-by: Claude:Opus-4.8

MounirDhahri and others added 11 commits July 20, 2026 17:14
The 'Prefetch app associations' step opens URLs on the simulator via
xcrun simctl openurl, which hits the real network. When artsy.net is
slow the call times out (NSPOSIXErrorDomain code 60) and, under set -e,
fails the whole job before any test runs — a common source of flaky
iOS Maestro runs. Wrap each prefetch in a retry loop that never fails
the step, preserving the universal-link warm-up without letting it
tank the run.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
The post-auth HomeView assertions used a bare assertVisible with
Maestro's short default timeout, so a slow post-login render (cold
network fetch) intermittently failed with 'Assertion is false:
id: search-button is visible'. Replace with extendedWaitUntil at a
30s timeout in login, signup and the deeplinks HomeView smoke check.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
Maestro's local CLI has no built-in retry-on-failure (only Maestro
Cloud does), so whole-flow retry is done in the shard wrapper. E2E
flows depend on the real network and universal-link resolution, so a
single transient failure shouldn't fail the run. Retry each flow once
from a clean launch before marking the shard failed.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
Running 'maestro test' locally on Maestro 2.x failed with 'Commands
Section Required' because e2e/config.yml carried an 'appId:' + '---'
header, so newer Maestro parsed everything after '---' as a flow's
command list and found 'flows:' instead. A workspace config takes only
the flows block (each flow already declares its own appId). CI is
unaffected — it runs individual flow files, not this config. Also fix
the README run commands to 'maestro test e2e/'.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
CI ran Maestro 1.40.0 while local dev is on 2.6.1, so behavior
diverged and the workspace config.yml that works locally differed
from CI. Align iOS CI to 2.6.1. Android is left on 1.40.0 for now
(out of scope; not validated here).

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
deeplinks.yml is structurally the flakiest flow: it round-trips
through Safari and depends on live universal-link resolution and real
network data. It still runs and retries, but a final failure now warns
instead of failing the shard, so a transient deeplink issue no longer
tanks the whole iOS suite.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
Add an End-to-End Testing section to AGENTS.md covering how to run
flows locally, the CI Maestro version pin, the shard retry / non-
blocking behavior, and guidance for writing stable flows. Link
e2e/README.md from Further Documentation. CLAUDE.md inherits via
@AGENTS.md import.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
The native in-app review prompt fires on the 5th app launch and
overlays the UI, intermittently blocking E2E assertions (e.g. the
post-signup HomeView search-button). Skip it when the useMaestroInit
launch argument is set, matching how push and onboarding prompts are
already suppressed. Add a jest mock for react-native-launch-arguments
so callers (HomeView) don't hit the native linking error.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
Match the iOS bump so both platforms run the same Maestro version as
local dev, avoiding local/CI behavior drift.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
Attach the ~/.maestro/tests output (screenshots + command logs) as a
GitHub Actions artifact when a shard fails, so failures are debuggable
directly from the run page. Uses if: failure() so nothing is uploaded
on green runs, and a per-shard artifact name to stay unique.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>

Assisted-by: Claude:Opus-4.8 [claude-code]
On Maestro 2.6.1 with Xcode 26, the first XCTest driver install on a
freshly-created simulator can be slow and intermittently trips the
120s timeout (IOSDriverTimeoutException). Double the budget to 240s so
a slow-but-healthy cold driver install doesn't fail the run.

Co-Authored-By: Claude Opus 4.8 (1M context) <[email protected]>
@artsyit

artsyit commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

This PR contains the following changes:

  • Dev changes (Stabilize Maestro E2E tests (iOS + Android): non-fatal app-association prefetch, longer HomeView waits, per-flow retry, non-blocking deeplinks, suppressed rate-the-app prompt in E2E, Maestro 2.6.1 alignment across CI/local, and screenshot artifacts uploaded only on failure. - MounirDhahri)

Generated by 🚫 dangerJS against 6b4878d

@claude

claude Bot commented Jul 20, 2026

Copy link
Copy Markdown
Contributor

Code Review

Summary

This PR stabilizes the Maestro E2E suite (iOS + Android) by addressing several distinct flakiness sources: making the iOS app-association prefetch non-fatal, replacing bare assertVisible with extendedWaitUntil on post-auth HomeView checks, adding a once-per-flow retry (with a non-blocking allowlist for deeplinks.yml), pinning Maestro to 2.6.1 across CI, fixing config.yml as a valid workspace config, suppressing the native "rate the app" prompt during E2E, and uploading Maestro screenshots as artifacts on failure. The only app-code change is the review-prompt suppression, which mirrors an existing pattern.

Issues Found

No blocking issues. The changes are well-scoped, each commit is independently revertible, and the app-code change follows an established pattern.

🟢 Suggestion — test coverage for the new suppression path (src/app/utils/useMaybePromptForReview.tsx)
There's no test for useMaybePromptForReview, and the new branch adds behavior worth locking in — specifically that promptForReview is skipped when useMaestroInit is set, and still fires on launchCount === 5 otherwise. The setupJest.tsx mock returns {} (no useMaestroInit), so existing/HomeView tests keep the pre-PR behavior, which is correct. A small unit test overriding LaunchArguments.value to { useMaestroInit: true } would guard against future regressions. Not required for this infra-focused PR.

🟢 Observation — inline type vs. shared interface (src/app/utils/useMaybePromptForReview.tsx:15)
LaunchArguments.value<{ useMaestroInit?: boolean }>() inlines a shape that already exists as MaestroLaunchArguments in src/app/system/devTools/useMaestroInitialization.ts. This matches the existing typing convention (that file also types useMaestroInit?: boolean even though the YAML passes the string "true" — both paths only check truthiness, so it works). Fine as-is; just noting the minor duplication.

Areas Reviewed

  • Architecture & Design: Good. Suppression logic mirrors the existing push/onboarding suppression in useMaestroInitialization.ts. The NON_BLOCKING_FLOWS allowlist and retry loop in run_maestro_shard are clear, and the basename matching is correct.
  • Bugs & Edge Cases: Verified — each flow (login, signup, deeplinks) declares its own appId: \${MAESTRO_APP_ID}, so dropping the appId/--- header from config.yml is safe. The iOS-only prefetch and driver-timeout changes are correctly not mirrored to Android (Android has no such steps). The if: failure() + if-no-files-found: ignore artifact upload won't fail green runs.
  • Testing: The react-native-launch-arguments jest mock in setupJest.tsx is necessary now that HomeView's useMaybePromptForReview calls LaunchArguments.value() under test.
  • Security: No concerns.

Notes

  • LLM usage is disclosed (Assisted-by: Claude:Opus-4.8) per the RFC — 👍
  • No user-facing UI change, so the missing screenshots are appropriately N/A.

Nice, methodical cleanup — the per-bucket commit breakdown in the description makes this easy to reason about.

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.

2 participants