ReviewOS

stacks/bench-review

mobile: no end-to-end tests for the native app (navigation, deep links, offline cold start)

#81
Open glennmichael123 opened this 9 days ago ·
9 days ago

Part of #79.

Problem

The iOS app has no automated test of its own. The web conformance gate (bun run gate, 18 checks) validates the built dist/, which the app loads — but nothing exercises the native shell: that it launches, that the WebView reaches its content, that a deep link routes, or that the offline fallback actually engages when the server is gone. The offline path is the one most worth testing, because it only runs when something else has already broken.

What to do

Port the wildloop reference suite:

  • .maestro/flows/ — it has 01-navigation.yaml, 02-deep-link.yaml, 02-deep-link-ios.yaml, 03-offline-bundle.yaml
  • scripts/run-mobile-e2e.ts (509 lines) — boots a simulator, builds, installs, runs the flows, saves screenshots and JUnit reports
  • scripts/mobile-target.ts (57 lines) — shared target resolution

Then bun run test:e2e:ios, plus the preview variants that make it usable by hand:

preview:ios            bundled, offline simulator build
preview:ios:local      simulator against ./buddy dev on this Mac
preview:ios:production benchreview.org, for demos with no local server

Flows worth having for bench specifically:

  1. Cold start, bundled — launch with MOBILE_E2E=1 (so webAssets: 'dist-offline'), assert the home page renders and the nav is present. This is the one that catches a broken offline bundle.
  2. Navigation — home → judges index → a judge tab. Note the judge tabs are served by one shared shell (judges/_shell/*) and resolve the id from location.pathname, so this exercises that path natively.
  3. Deep link — benchreview:// scheme and, once #universal-links lands, an applinks: URL.
  4. Offline fallback — start pointed at a server, kill it, assert the app serves bundled pages rather than an error, then assert it returns when the server answers.

Checks

  • bun run test:e2e:ios green locally
  • Wired into CI, with the Maestro + simulator prerequisites documented (wildloop has docs/mobile-e2e.md for this)
  • Screenshots and JUnit output saved as artifacts
  • The suite fails, not passes, when dist-offline/ is missing or empty

Done (1b96a6d4) — three flows green on an iPhone 17 Pro

  ✓ 01-cold-start.yaml
  ✓ 02-navigation.yaml
  ✓ 04-offline.yaml

Adapted rather than ported wholesale: wildloop's runner is 509 lines covering Android, location seeding and signed-in journeys. bench declares deviceFamilies: ['iphone'] and has no Android config, so an Android arm would be dead code pretending to be coverage.

02-navigation goes through the hamburger, because below lg that is the only navigation an iPhone has (#90).

Three things writing it found

1. The offline bundle was a week stale. bun run build refreshes dist/ and does not refresh dist-offline/ — only build:ios chains the two — so the app shipped whatever the bundle last contained. That copy predated the judge directory's load-failure state, so the offline app claimed "No judges yet" during an outage: the exact false claim that state was written to prevent.

The runner now refuses a bundle that is missing, empty, missing a page the flows visit, or older than dist/. 04-offline additionally asserts that string is absent, so the regression cannot come back quietly.

2. The generated Xcode project cannot be built. It declares no PRODUCT_NAME, so Xcode resolves the product to a bare .app and the build dies with "Multiple commands produce …/.app" — the link step and the create-directory step both claiming the same empty name.

Framework, not app: project.yml and project.pbxproj are both written by ./buddy build:ios under storage/framework/, regenerating from that spec with XcodeGen reproduces it exactly, and a sibling project on a newer framework emits PRODUCT_NAME six times. Worked around with an override on the xcodebuild command line, which survives regeneration where editing the generated spec would not.

3. benchreview:// does not route — filed as #94. The scheme is registered and iOS hands the URL to the app; the shell does nothing with it. Same shape as #80: an entitlement with nothing on the other side, failing silently.

That flow is kept as pending-03-deep-link.yaml rather than deleted — it is the evidence — and the runner skips pending-* by default, so a red suite always means a regression rather than a known bug. 04-offline originally reached its second page through that link, which made the offline test fail for a reason unrelated to being offline; it navigates through the menu now.

Also worth knowing

Maestro needs a JDK, and Homebrew's openjdk is keg-only so /usr/libexec/java_home does not find it — it exited before running a single step. The runner resolves JAVA_HOME itself.

A run that executed no tests is treated as a failure. Maestro exits 0 when it matches no flows, and reading that as success is how a suite reports green while testing nothing at all.

Still open

  • Wired into CI. It needs a macOS runner with Xcode and a simulator — a different and much slower job than the current Linux CI, and a cost decision rather than a technical one. The suite is runnable locally today; docs/mobile-e2e.md has the prerequisites.
8 days ago

The suite does not just lack native coverage — it tests an app that no longer exists

Found while closing #94. Measured, not inferred.

Flows 01, 02 and 04 assert web-page text the native app never draws

Assert that "Transparency in the US Chambers of Secrets" is visible... FAILED

That is 01-cold-start.yaml run against the current native build. 02-navigation.yaml taps "Open main menu" (the web header's hamburger) and waits for "Judicial Directory" (a web page heading); 04-offline.yaml is written against a bundled-fallback behaviour the native renderer no longer has. None of those strings exist in the app now — it opens on a native Judges directory with a UIKit tab bar.

MOBILE_E2E=1 is now a no-op, and the runner's docblock says otherwise

scripts/run-mobile-e2e.ts still says:

MOBILE_E2E=1 makes config/mobile.ts choose webAssets: 'dist-offline' with no server at all, so every flow runs against the bundle and none of them depend on the network.

config/mobile.ts has no MOBILE_E2E branch any more — it is always MOBILE_URL ?? 'https://benchreview.org/judges'. The framework refuses webAssets and fallbackWebAssets under renderer: 'native' (no web view to show them in), and the config comments record that. So:

  • the e2e build points at benchreview.org, which is not deployed (#85/#45), and
  • the runner validates dist-offline REQUIRED_PAGES that the app no longer loads.

A run today therefore needs MOBILE_URL pointed at something reachable. That is how 03-deep-link.yaml was verified.

What this means for the work here

03-deep-link.yaml is native-correct and passes (84ab374b). The other three need rewriting against native screens, and the runner needs its offline story and its docblock brought in line with a renderer that has no web view. The offline flow is the one that needs rethinking rather than retargeting: native screens cold-start without a network and show their own offline state, so there is something real to assert — but it is no longer 'the bundle serves the page'.

8 days ago

Rebuilt against the native app — four flows green (9672e933)

  ✓ 01-cold-start.yaml
  ✓ 02-navigation.yaml
  ✓ 03-deep-link.yaml
  ✓ 04-offline.yaml

Following the previous comment: the suite was not merely missing native coverage, it was testing an app that no longer exists. All three old flows asserted web text and MOBILE_E2E=1 had become a no-op pointing the build at an undeployed host.

The run owns a server now

scripts/mobile-e2e-api.ts — fixed data, own port. Two reasons it has to be ours rather than ./buddy dev:

  1. The offline flow has to be able to switch it off. ./buddy dev belongs to whoever is running it; a suite that killed it would take their work away mid-run.
  2. The data has to be the same every run. A flow that taps the first row and asserts what opens needs to know what the first row is.

Fixture names are deliberately unmistakable ("Fixture A. Alder"), so a run accidentally pointed at a real server fails on the first assertion instead of passing by luck.

It runs as a separate PROCESS, not Bun.serve in the runner. Every external command goes through Bun.spawnSync, which blocks the event loop, so an in-process server accepts nothing for the whole of a maestro run. Measured, not guessed — the first version did exactly that and flow 01 sat out its full 90 seconds against a dead port.

Offline means the server is stopped, not airplane mode

The old flow used setAirplaneMode. That cannot work for an app whose screens all fetch: the simulator shares the Mac's network stack and loopback is not the radio, so the app would keep reaching a localhost server with the plane switch on. It passed because the app it tested needed no server at all.

Flows declare the need themselves via tags: offline, which the runner reads — renaming a file cannot quietly turn the offline test into an online one. Verified it can fail: with the tag stripped the fixture stays up and 04 goes red, and a unit test pins that the flow in the repo actually carries the tag.

Two things the flows had to learn

Tabs are tapped by position, not by their word. The bar's labels are not unique on screen — a courthouse's stats row says "Judges" and "Reviews" too, and the back button inherits the previous screen's title. tapOn: "Reviews" matched the stats caption while the flow went on asserting against the screen it was already on. (Assistive tech has the same ambiguity — worth a look during #58.)

A row tapped the instant its text appears can be tapped mid-settle and silently lost — indistinguishable from the missing-id bug, which is the thing this flow exists to catch. It failed that way in a full run and passed on its own before waitForAnimationToEnd went in front of each push.

Guard moved with the app

dist-offline/*.html means nothing now, so the staleness check compares the compiled native screens against the .stx they came from. The footgun is --no-build, reached for while iterating on flows, which is exactly when yesterday's screens get read as today's.

Still open

  • Wired into CI — needs a macOS runner with Xcode, a simulator and Maestro. Unchanged by this.

preview:ios also carried the dead MOBILE_E2E and built an offline bundle nothing loads; it now builds against the fixture, with mobile:fixture-api to serve it.

Sign in to comment on this issue.