diff --git a/.github/workflows/maestro.yml b/.github/workflows/maestro.yml index 6c8fd6003..937df148d 100644 --- a/.github/workflows/maestro.yml +++ b/.github/workflows/maestro.yml @@ -13,6 +13,11 @@ on: exclude_tags: description: "Maestro exclude-tags (side-effecting flows excluded by default)" default: "spends-funds,creates-account" + minify: + description: "Run against an R8-minified build, an unminified one, or both in turn" + type: choice + default: "true" + options: ["true", "false", "both"] schedule: - cron: "37 7 * * *" # nightly, off the top of the hour @@ -21,10 +26,34 @@ concurrency: cancel-in-progress: true jobs: + # `minify: both` has to become a matrix before the job starts, and a matrix cannot be + # built from an input inline. Scheduled runs get no input, so they fall through to the + # minified build — that is the one that catches a broken keep rule. + variants: + runs-on: ubuntu-latest + outputs: + minify: ${{ steps.pick.outputs.minify }} + steps: + - id: pick + run: | + case "${{ github.event.inputs.minify || 'true' }}" in + both) echo 'minify=["true","false"]' >> "$GITHUB_OUTPUT" ;; + false) echo 'minify=["false"]' >> "$GITHUB_OUTPUT" ;; + *) echo 'minify=["true"]' >> "$GITHUB_OUTPUT" ;; + esac + maestro: - name: Maestro E2E (${{ github.event.inputs.tags || 'smoke' }}) + name: Maestro E2E (${{ github.event.inputs.tags || 'smoke' }}, minify=${{ matrix.minify }}) + needs: variants runs-on: ubuntu-latest timeout-minutes: 60 + strategy: + # One at a time: every flow signs into the same shared test account, so two runners + # would fight over its balance and session. + max-parallel: 1 + fail-fast: false + matrix: + minify: ${{ fromJSON(needs.variants.outputs.minify) }} steps: - uses: actions/checkout@v4 with: @@ -85,10 +114,16 @@ jobs: env: MAESTRO_TAGS: ${{ github.event.inputs.tags || 'smoke' }} MAESTRO_EXCLUDE_TAGS: ${{ github.event.inputs.exclude_tags || 'spends-funds,creates-account' }} + # Read by tryReadProperty in apps/flipcash/app/build.gradle.kts. local.properties + # wins over the environment there, and the step above never writes this key, so the + # value set here is the one Gradle sees. UI_TESTABLE stays true either way, since it + # is off only for the shipping `release` variant. + DEBUG_MINIFY: ${{ matrix.minify }} # Test-account credentials (map GitHub secrets -> the env vars run.sh reads). SEED_PHRASE: ${{ secrets.MAESTRO_SEED_PHRASE }} LOGIN_DEEPLINK: ${{ secrets.MAESTRO_LOGIN_DEEPLINK }} TIPCARD_DEEPLINK: ${{ secrets.MAESTRO_TIPCARD_DEEPLINK }} + LOGIN_USERNAME: ${{ secrets.MAESTRO_LOGIN_USERNAME }} USDF_ONLY_DEEPLINK: ${{ secrets.MAESTRO_USDF_ONLY_DEEPLINK }} CONTACT_NAME: ${{ secrets.MAESTRO_CONTACT_NAME }} CONTACT_PHONE: ${{ secrets.MAESTRO_CONTACT_PHONE }} @@ -97,7 +132,7 @@ jobs: if: always() uses: actions/upload-artifact@v4 with: - name: maestro-report + name: maestro-report-minify-${{ matrix.minify }} path: | maestro-report.xml ~/.maestro/tests/** diff --git a/apps/flipcash/app/proguard-rules.pro b/apps/flipcash/app/proguard-rules.pro index a97a0d11a..2e908f9d9 100644 --- a/apps/flipcash/app/proguard-rules.pro +++ b/apps/flipcash/app/proguard-rules.pro @@ -1,11 +1,20 @@ # Preserve source file names and line numbers for stack traces (call site tracking, Bugsnag) -keepattributes SourceFile,LineNumberTable -# Keep AppRoute class names. `annotatedEntry` derives each screen's root test tag -# from the route's simple name (NavMetadata.screenRootTag), so obfuscating these -# renames every screen-root resource-id the UI tests address. --keepnames class com.flipcash.app.core.AppRoute --keepnames class com.flipcash.app.core.AppRoute$** +# Keep every navigation route class. `annotatedEntry` derives each screen's root test +# tag from `T::class.simpleName` (NavMetadata.screenRootTag), and `getSimpleName()` on a +# nested class reads the `InnerClasses` attribute, which R8 emits only for classes matched +# by a full -keep. Under -keepnames the binary name survives but the attribute does not, so +# `AppRoute.Main.Scanner` reports `AppRoute$Main$Scanner` and the tag becomes +# `app_route$main$scanner_screen`. The global -keepattributes InnerClasses does not change +# that, and neither does reading the binary name in Kotlin: routes outside AppRoute, such as +# `OnboardingStep`, need their names kept regardless. Matching on the NavKey supertype covers +# both hierarchies and any future one. Class-only, with no member wildcard, so members stay +# shrinkable and renameable; the whole rule costs 137 classes and 16 KB. +# +# Only builds with BuildConfig.UI_TESTABLE expose these tags as resource-ids, so this buys +# nothing for the shipping release — it lets Maestro run against a minified build. +-keep class * implements androidx.navigation3.runtime.NavKey # Protobuf keep rules ship with the contract packages themselves, from 0.3.0 on: # com.flipcash:{ocp,flipcash2}-client-protocol carry them in META-INF/proguard/, which diff --git a/maestro/README.md b/maestro/README.md index 205cb1815..6219513c9 100644 --- a/maestro/README.md +++ b/maestro/README.md @@ -25,7 +25,11 @@ iOS's `FlipcashUITests`. Flows are plain YAML under `maestro/`; reusable pieces CONTACT_NAME=Brandon McAnsh # an on-Flipcash contact for send-to-contact CONTACT_PHONE=+15869802333 # seed this contact into the emulator ``` - The runner (`run.sh`) forwards all of these to Maestro. + The runner (`run.sh`) forwards all of these to Maestro. It checks them first: a missing + `SEED_PHRASE` or `LOGIN_DEEPLINK` stops the run with the variable named, and the + flow-specific ones warn with the flows that read them. Without that check an unset + credential reaches Maestro as an empty value and surfaces much later as + `wallet_screen is visible` failing inside a login subflow. ## Running @@ -196,9 +200,14 @@ flow deliberately, clear the exclude, e.g. `MAESTRO_TAGS=tipping MAESTRO_EXCLUDE - The workflow boots a KVM `x86_64` emulator (`reactivecircus/android-emulator-runner`), sets up the same build secrets as the unit-test job, installs the Maestro CLI, runs the lane, and uploads the report. -- Triggers: **`workflow_dispatch`** (choose `tags`/`exclude_tags`) and a **nightly schedule** - (smoke). It's real-backend E2E against the shared account, so it's deliberately not on every PR; - add a `pull_request:` trigger to gate PRs (won't run on fork PRs, which lack secrets). +- The run sets `DEBUG_MINIFY`, so the suite exercises an R8-minified debug build by default. + That's the build that catches a broken keep rule: screen-root test tags come from + `T::class.simpleName`, which needs the route classes kept (see `apps/flipcash/app/proguard-rules.pro`). + The `minify` dispatch input takes `true`, `false`, or `both`; `both` runs the two builds in turn + (one at a time — every flow signs into the same shared account) and uploads a report per variant. +- Triggers: **`workflow_dispatch`** (choose `tags`/`exclude_tags`/`minify`) and a **nightly schedule** + (smoke, minified). It's real-backend E2E against the shared account, so it's deliberately not on + every PR; add a `pull_request:` trigger to gate PRs (won't run on fork PRs, which lack secrets). Run locally the same way CI does: ```bash @@ -206,7 +215,7 @@ MAESTRO_TAGS=smoke maestro/run.sh --tags smoke ``` **Required GitHub secrets** (test-account creds — the workflow maps them to the env vars -`run.sh` reads): `MAESTRO_SEED_PHRASE`, `MAESTRO_LOGIN_DEEPLINK`, `MAESTRO_TIPCARD_DEEPLINK`, +`run.sh` reads): `MAESTRO_SEED_PHRASE`, `MAESTRO_LOGIN_DEEPLINK`, `MAESTRO_LOGIN_USERNAME`, `MAESTRO_TIPCARD_DEEPLINK`, `MAESTRO_USDF_ONLY_DEEPLINK`, `MAESTRO_CONTACT_NAME`, `MAESTRO_CONTACT_PHONE` — plus the existing build secrets (`FLIPCASH2_GOOGLE_SERVICES`, `FLIPCASH_BUGSNAG_API_KEY`, `FLIPCASH_MIXPANEL_API_KEY`, `COINBASE_ONRAMP_API_KEY`, `GOOGLE_CLOUD_PROJECT_NUMBER`). diff --git a/maestro/run.sh b/maestro/run.sh index 3a0c3588c..87940cbe6 100755 --- a/maestro/run.sh +++ b/maestro/run.sh @@ -37,6 +37,38 @@ USDF_ONLY_DEEPLINK="$(cred USDF_ONLY_DEEPLINK)" CONTACT_NAME="$(cred CONTACT_NAME)" CONTACT_PHONE="$(cred CONTACT_PHONE)" +# Nothing downstream treats an unset credential as an error. It reaches Maestro as an empty +# `-e` value, `inputText: ${SEED_PHRASE}` types nothing, the disabled Log In button does not +# respond, and the run fails twenty minutes later on `wallet_screen is visible` — an assertion +# inside a subflow that names none of the actual cause. Report the variable instead. +# Names only: never echo a credential's value. +missing_login=() +missing_flow=() +if [[ -z "$SEED_PHRASE" ]]; then missing_login+=("SEED_PHRASE subflows/login.yaml, subflows/login_with_flags.yaml"); fi +if [[ -z "$LOGIN_DEEPLINK" ]]; then missing_login+=("LOGIN_DEEPLINK subflows/login_with_deeplink.yaml"); fi +if [[ -z "$USDF_ONLY_DEEPLINK" ]]; then missing_flow+=("USDF_ONLY_DEEPLINK usdf_only_gate.yaml"); fi +if [[ -z "$TIPCARD_DEEPLINK" ]]; then missing_flow+=("TIPCARD_DEEPLINK tip_deeplink.yaml"); fi +if [[ -z "$LOGIN_USERNAME" ]]; then missing_flow+=("LOGIN_USERNAME vanity_deeplink_self.yaml, vanity_deeplink_tip.yaml"); fi +if [[ -z "$CONTACT_NAME" || -z "$CONTACT_PHONE" ]]; then + missing_flow+=("CONTACT_NAME/PHONE blocking.yaml, tip_chat.yaml") +fi + +# Flow-specific credentials are only a problem for the flows that read them, and a local run +# of one flow has no reason to set the rest — warn, don't block. +if (( ${#missing_flow[@]} > 0 )); then + printf 'warning: unset — the flows that read them will fail:\n' >&2 + printf ' %s\n' "${missing_flow[@]}" >&2 +fi + +# Both login paths gone means no flow can get past its first step, so stop here. +if (( ${#missing_login[@]} > 0 )); then + printf 'error: login credentials unset:\n' >&2 + printf ' %s\n' "${missing_login[@]}" >&2 + printf 'Set them in %s, or in the environment. CI maps them from the MAESTRO_* repo secrets\n' "$ENV_FILE" >&2 + printf 'in .github/workflows/maestro.yml; a secret that does not exist expands to an empty string.\n' >&2 + exit 1 +fi + # App Links verification does not survive a fresh install; approve so # https://app.flipcash.com/... deeplinks open the app, not Chrome. adb -s "$DEVICE" shell pm set-app-links --package "$APP_ID" 2 all >/dev/null 2>&1 || true @@ -55,8 +87,10 @@ seed_contact() { adb -s "$DEVICE" shell content insert --uri "$raw_uri" \ --bind account_name:s: --bind account_type:s: >/dev/null 2>&1 local rid + # `|| true` because pipefail turns a no-match grep into a failed assignment, which under + # `set -e` kills the script silently — before the empty-rid guard below can report it. rid=$(adb -s "$DEVICE" shell content query --uri "$raw_uri" --projection _id 2>/dev/null \ - | grep -oE '_id=[0-9]+' | cut -d= -f2 | sort -n | tail -1) + | grep -oE '_id=[0-9]+' | cut -d= -f2 | sort -n | tail -1 || true) [[ -z "$rid" ]] && { echo "warning: could not seed contact" >&2; return 0; } adb -s "$DEVICE" shell "content insert --uri $data_uri --bind raw_contact_id:i:$rid \ --bind mimetype:s:vnd.android.cursor.item/name --bind data1:s:'$CONTACT_NAME'" >/dev/null 2>&1