Repository navigation
feat: e2e tests - #1501
feat: e2e tests#1501MrMuzyk wants to merge 7 commits into
Conversation
|
@MrMuzyk is attempting to deploy a commit to the Callstack Team on Vercel. A member of the Team first needs to authorize it. |
|
# Conflicts: # apps/tester-federation-v2/package.json # pnpm-lock.yaml
There was a problem hiding this comment.
Thanks for your work on this. I ran the suite locally at 4feac606 on a dedicated API 35 emulator and an iOS 26.4 simulator: 3/3 on Android and 3/3 on iOS, with --workers 1 --retries 0, in a separate run per platform. So the tests themselves work.
Inline comments cover the details. The main points:
- Trigger model: I'd switch from
pull_request_targettopull_request. See the comment onon:. - The root cause from #1499 is still there.
LogBox.ignoreAllLogs(true)hides the toast, but the MF2 shared versions still resolve tocatalog:testers. WithoutE2E=true, the Gallery test still fails locally. - Determinism: pin the device, set
retries: 0explicitly, and turn telemetry off.
Also:
e2elabel: the repo doesn't have it yet (gh label listreturns nothing). Without it, labelled fork runs are skipped silently. The description already lists this as a step before merge, so I'm noting it here so it doesn't get missed.- No CI run at this head. The green run linked in the description is a
workflow_dispatchonc33c7bb3. The only difference from4feac606is thatsrc/host/index.jsloses the comment explaining the LogBox gating. If the gating stays, please keep that comment. - Running locally with Xcode 27: the iOS build needs
IPHONEOS_DEPLOYMENT_TARGET=15.1passed toxcodebuild, because AsyncStorage's pod targets 13.0. This doesn't affect CI onmacos-15/Xcode 16.4.
| name: e2e | ||
|
|
||
| on: | ||
| pull_request_target: |
There was a problem hiding this comment.
I'd use pull_request here instead of pull_request_target. The job uses no secrets and only needs contents: read, so the target trigger adds risk without enabling anything:
- The label stays on the PR. Once a maintainer adds
e2eto a fork PR, every latersynchronizepush runs fork code with no fresh review. Withpull_request, GitHub's fork-workflow approval does this gating. - The fork picks the pnpm version.
setup-pnpm-nodepasses noversion, sopnpm/action-setupinstalls whateverpackageManagersays in the checked-out, fork-controlledpackage.json, against the fork's lockfile. - Workflow changes can't be tested before merge. With
pull_request_targetthe workflow file always comes frommain, so a PR's edits toe2e.ymlnever run on that PR. That includes this PR. - It tests the PR head, not the merge. The checkout uses
head.sha. The defaultpull_requestcheckout uses the merge ref, which is the right thing for a merge gate.
On cache poisoning, the concern from #1499: GitHub now gives untrusted triggers a read-only cache token for the default-branch scope, so this is mitigated by the platform. The points above still apply.
With pull_request, I'd also:
- drop
labeled/unlabeledfromtypes, since each label change currently starts a new run - add
paths:(e.g.apps/tester-federation-v2/**,packages/**,pnpm-lock.yamland this workflow file) so docs-only PRs don't spend emulator minutes
| # one run per PR; a new push cancels the older one | ||
| concurrency: | ||
| group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} | ||
| cancel-in-progress: true |
There was a problem hiding this comment.
This also applies to push on main, so the next merge cancels the run for the previous one. That run is the signal that main is healthy.
| cancel-in-progress: true | |
| cancel-in-progress: ${{ github.event_name != 'push' }} |
| adb reverse --list | ||
| adb reverse --list | grep -q 'tcp:8081' || { echo '::error::adb reverse missing for tcp:8081'; exit 1; } | ||
| adb reverse --list | grep -q 'tcp:8082' || { echo '::error::adb reverse missing for tcp:8082'; exit 1; } | ||
| adb install -r -g android/app/build/outputs/apk/debug/app-debug.apk |
There was a problem hiding this comment.
android-emulator-runner runs script: with sh -c and no -e. The two adb reverse --list checks are guarded, but adb install isn't. If the install fails (e.g. INSTALL_FAILED_UPDATE_INCOMPATIBLE), the suite still runs and fails about 60 s later with "home-details-button not found".
set -e would break the ec=$? / logcat pattern below, so guarding this line is simpler:
| adb install -r -g android/app/build/outputs/apk/debug/app-debug.apk | |
| adb install -r -g android/app/build/outputs/apk/debug/app-debug.apk || { echo '::error::adb install failed'; exit 1; } |
| adb reverse --list | grep -q 'tcp:8081' || { echo '::error::adb reverse missing for tcp:8081'; exit 1; } | ||
| adb reverse --list | grep -q 'tcp:8082' || { echo '::error::adb reverse missing for tcp:8082'; exit 1; } | ||
| adb install -r -g android/app/build/outputs/apk/debug/app-debug.apk | ||
| pnpm exec e2e run --target android --reporter list,junit; ec=$?; adb logcat -d -v time > .e2e/logcat.txt 2>&1; exit $ec |
There was a problem hiding this comment.
e2e sends anonymous usage telemetry to PostHog by default. Neither job opts out, and the e2e docs recommend setting the opt-out in CI. A workflow-level env: covers both lanes:
env:
E2E_TELEMETRY_DISABLED: '1'| name: e2e-android | ||
| path: apps/tester-federation-v2/.e2e | ||
| include-hidden-files: true | ||
| if-no-files-found: error |
There was a problem hiding this comment.
If the job fails before the e2e step (Gradle, pod install, dev server timeout), .e2e doesn't exist. This step then fails too, so the run shows a second error next to the real one. warn keeps the upload without that extra failure. The same applies to the iOS job.
Related: Show dev server logs runs after the upload, so the host and miniapp server logs never end up in the artifact. iOS artifacts also contain only the report and junit (no device log). Copying /tmp/hostapp.log and /tmp/miniapp.log into .e2e/ before uploading would make failures debuggable from the artifact.
| targets: [ | ||
| { | ||
| name: 'android', | ||
| engine: mobile({ platform: 'android' }), |
There was a problem hiding this comment.
Without device, @e2e-dev/mobile uses every booted device of the platform. Locally, e2e defaults to workers = cores / 2. When I ran pnpm exec e2e run --target android on a machine with several booted emulators and simulators, it would have spread across all of them, including ones other work was using. CI behaves only because each job has exactly one device and CI mode defaults workers to 1.
Pinning it in the config, with an env override for local runs, solved this for me:
engine: mobile({ platform: 'android', device: process.env.E2E_DEVICE }),On iOS the job already names its simulator e2e, so CI can pass that in.
I'd also set retries: 0 and workers: 1 explicitly at the top level. In CI mode e2e defaults to retries: 1, and a test that fails once and then passes is counted as flaky, not failed, so the check stays green. For a 3-test smoke lane that targets cold start and MF remote loading, an intermittent first-attempt failure is exactly the regression we want to see.
| import App from './App'; | ||
|
|
||
| if (__E2E__) { | ||
| LogBox.ignoreAllLogs(true); |
There was a problem hiding this comment.
This hides the symptom from #1499, but the cause is still there. The green run's logcat.txt still logs [ Federation Runtime ] Version catalog:testers from HostApp of shared singleton module react-native-safe-area-context does not satisfy the requirement… (and the same for react-native-screens) on every launch. Locally, at this head, the Gallery test fails without E2E=true because the toast covers the tile.
Cause: the four MF configs pass pkg.dependencies['react-native-safe-area-context'] and ['react-native-screens'] as the shared version/requiredVersion. In package.json those values are the literal string catalog:testers, which MF2's semver check can't parse. Importing the real versions fixes it, the same way the configs already handle react and react-native:
import safeAreaPkg from 'react-native-safe-area-context/package.json' with { type: 'json' };
import screensPkg from 'react-native-screens/package.json' with { type: 'json' };
// ...
version: safeAreaPkg.version,
requiredVersion: safeAreaPkg.version,With that in place, this line can go entirely, or be narrowed to LogBox.ignoreLogs([...]) for anything specific that's left.
Also, ignoreAllLogs(true) hides console.error toasts too, not just warnings (see LogBoxNotificationContainer, which returns nothing when isDisabled). Only fatal errors and red boxes still render. The description says errors are still shown, which only holds for fatal ones. A global mute also means future MF or React Native warnings won't show up in this lane, and catching those is what it's for.
| __WITH_PRELOAD__: | ||
| process.env.WITH_PRELOAD === 'true' || | ||
| process.env.WITH_PRELOAD === '1', | ||
| __E2E__: process.env.E2E === 'true', |
There was a problem hiding this comment.
Minor: __E2E__ (like the existing __WITH_PRELOAD__) isn't declared anywhere for TypeScript, and this app has no typecheck script, so turbo run typecheck skips it. That means e2e.config.ts and e2e/*.e2e.ts are also never type-checked. A small src/globals.d.ts with declare const __E2E__: boolean; plus "typecheck": "tsc --noEmit" (as tester-expo has) would cover both.
| }, | ||
| "devDependencies": { | ||
| "@babel/core": "^7.29.7", | ||
| "@e2e-dev/mobile": "0.11.0", |
There was a problem hiding this comment.
Two things about the new deps:
- Catalog: every other tester dependency is catalog-managed, and
@e2e-dev/mobilehas a peer dependency one2e >=0.15 <1, so the two need to move together. Adding them tocatalog:testers(or adding a "pinned together" comment) would make that explicit. They also bring@vitest/*@5into the lockfile while the catalog pinsvitest ^4. That's harmless, but worth knowing. - Node version:
e2e@0.19.0and@e2e-dev/mobile@0.11.0declareengines.node: ^22.22.3 || >=24.8.0, but the rootenginesallows>=24. There's noengine-strict, so on 24.0–24.7 a contributor only sees a warning and then gets a confusing failure.
| if (this.state.hasError) { | ||
| return ( | ||
| <View style={styles.container}> | ||
| <View testID="miniapp-error" style={styles.container}> |
There was a problem hiding this comment.
Nit: nothing uses miniapp-error yet. When the remote fails to load, the Gallery test keeps polling miniapp-gallery for 45 s and then reports a timeout, so a broken remote looks the same as a slow device. Checking miniapp-error alongside the gallery check would make it fail fast with a clear message. Otherwise this testID can be dropped.
|
@dannyhw Ive addressed the comments and moved away from the approach with 'e2e' label as implied by the comments in the review. It's ready for another round of review ;) |
This branch has not been deployed
2 participants
Summary
Adds an e2e lane that runs the MF2 tester app on a real emulator/simulator on CI, so Re.Pack bundling/runtime regressions are caught before merge instead of in a consumer app.
.github/workflows/e2e.yml— Android on an API 35 emulator, iOS on an iPhone 16 simulator. Each job starts both Re.Pack dev servers (host app :8081, MiniApp :8082), warms both platform compilers, builds the Debug app, then runs the suite against it. Android runs on PRs; iOS runs on merges tomain(and dispatch), since it can't fit inside a PR's deadline yet — Phase 2 moves it onto a beefier runner.apps/tester-federation-v2/e2e/smoke.e2e.ts— 3 deterministic tests (app boots, Home → Detail → back, Home → Gallery loads the MiniApp over the MF remote), viatester-army/e2e+@e2e-dev/mobile. No model credential needed.retries: 0andworkers: 1are set explicitly: a flaked first attempt must read as a failure, notflaky, and the device is pinned (E2E_DEVICE) so a local run never lands on someone else's booted emulator.pull_requestwithpaths:filters (app, packages, lockfile, the workflow itself),contents: read, no secrets, and the default merge-ref checkout — so GitHub's fork-workflow approval is the gate, no label needed, and PR edits toe2e.ymlare self-tested before merge. Apushtomainnever cancels, so the "main is healthy" signal survives a back-to-back merge. Telemetry off at workflow level (E2E_TELEMETRY_DISABLED: '1').catalog:shared-version fix. The four MF configs read sharedversion/requiredVersionfrompackage.json, where those deps are the literal stringcatalog:testers— MF2's semver check can't parse it, warned on every launch, and the LogBox toast absorbed a tap. The configs now import the real versions (react-native-safe-area-context/react-native-screenspackage.jsonwith import attributes), same as they already did forreact/react-native. The__E2E__define andLogBox.ignoreAllLogs(true)are gone entirely — nothing is muted, warnings included, which is what this lane is for. The Gallery test passing with zero suppression is the regression test for this.typecheckscript +src/globals.d.tsfor this app (it was skipped byturbo run typecheck, so the e2e files were never type-checked);e2e/@e2e-dev/mobilemoved tocatalog:testers(peer range>=0.15 <1— they must move together); dev-server logs (/tmp/hostapp.log,/tmp/miniapp.log) collected into the artifacts; guardedadb install; iOS pods via the app's Gemfile (bundle exec pod install).Notes
e2e@0.19.0/@e2e-dev/mobile@0.11.0declareengines.node: ^22.22.3 || >=24.8.0while the repo's rootenginesallows>=24, and there's noengine-strict— on Node 24.0–24.7 a contributor only gets a warning and then a confusing failure. CI pins 24.18.0.tester-federation(v1) carries the samecatalog:bug in its configs (incl.async-storage) — deliberately untouched here; worth a follow-up PR.IPHONEOS_DEPLOYMENT_TARGET=15.1forxcodebuild(AsyncStorage's pod targets 13.0). CI onmacos-15/Xcode 16.4 is unaffected.Test plan
CI is the evidence — with the
pull_requesttrigger, the Android lane runs on this PR's head; iOS can be proven per-PR viaworkflow_dispatchand runs on every merge tomain. On failure, thee2e-android/e2e-iosartifacts carry junit + screenshots + logcat + both dev server logs.Locally (needs Node ≥ 24.8 or 22.22.3; CI pins 24.18.0):
pnpm android/pnpm ioswith the app's own scripts).cd apps/tester-federation-v2, start both servers:pnpm start:hostappandpnpm start:miniapp(no flag needed anymore).E2E_DEVICE=<name or serial> pnpm exec e2e run --target android(or--target ios). With several devices booted, the pin keeps the run off the ones other work is using; unset,@e2e-dev/mobilepicks up every booted device.