Skip to content

Commit b8b9742

Browse files
committed
test(expo): address Sean's iOS review feedback
P1 (cache honesty): the compat-gate pin steps mutate the working tree (packages/expo/app.plugin.js, packages/expo/android/build.gradle), but the bin-hash that keys the .app / .apk cache was computed from `git ls-tree -r HEAD`, which reads committed blobs. A stale cache hit could install an OLD SDK ref while claiming to test a new one. Fold clerk_ios_ref, clerk_android_ref, and clerk_android_snapshot_suffix into the hash on both jobs so the cache key reflects what's actually being built. P2 (Swift tests on production code): pulled the two pure predicates the existing tests were mirroring — `isSuccessfulAuth` (from ClerkAuthWrapperViewController.viewDidDisappear) and the presentWhenReady guard (from ClerkAuthNativeView) — into ClerkAuthLogic.swift in the pod source files. ClerkViewFactory.swift now imports ClerkExpo and calls ClerkAuthSessionLogic; ClerkExpoModule.swift's view layer now calls ClerkPresentationLogic. The XCTest files `@testable import ClerkExpo` and exercise the same public symbols production runs against — no more duplicated logic to drift. The maxPresentationAttempts constant lives on the production type so a bump can't silently break the test. P2 (forgot-password OAuth automated): split google-sso-from-forgot-password into two files. The original bug was that tapping "Sign in with Google" from the forgot-password screen was a silent no-op on iOS — that part is now automated by asserting the system OAuth presentation appears ("Wants to Use" / "accounts.google.com" / "Continue" / "google.com"). The actual Google credentialed completion still needs real OAuth and lives in google-sso-from-forgot-password-manual.yaml with the `manual` tag, so the default workflow exclude (manual,skip) keeps it out of CI but it's runnable as a one-off. P2 (local scripts pre-filter): Maestro's `--exclude-tags` is a no-op when explicit file paths are passed, which run-ios.sh / run-android.sh / run-regressions.sh all do. Added scripts/lib/filter-flows.sh — a shared helper that scans each flow's YAML frontmatter for tag-list entries matching the excluded set — and routed all three scripts through it before they invoke maestro.
1 parent ea70088 commit b8b9742

13 files changed

Lines changed: 308 additions & 228 deletions

.github/workflows/mobile-e2e.yml

Lines changed: 27 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -238,13 +238,27 @@ jobs:
238238
# the workflow file (which encodes the quickstart-modification rules),
239239
# and the quickstart source itself. node_modules, android/, and ios/
240240
# are excluded (they're build outputs / regenerated by prebuild).
241+
#
242+
# Also fold the compat-gate ref inputs into the hash. The "Pin clerk-*"
243+
# steps above mutate the working tree (app.plugin.js, build.gradle),
244+
# but `git ls-tree -r HEAD` reads committed blobs and would miss those
245+
# patches — a stale cache hit could then install the OLD SDK refs while
246+
# claiming to test the new ones. Including the input strings here makes
247+
# the cache key honest about what's actually being built.
241248
id: bin-hash
249+
env:
250+
CLERK_IOS_REF: ${{ inputs.clerk_ios_ref }}
251+
CLERK_ANDROID_REF: ${{ inputs.clerk_android_ref }}
252+
CLERK_ANDROID_SNAPSHOT_SUFFIX: ${{ inputs.clerk_android_snapshot_suffix }}
242253
run: |
243254
expo_tree=$(git ls-tree -r HEAD packages/expo .github/workflows/mobile-e2e.yml | grep -v "/dist/")
244255
qs_tree=$(git -C clerk-expo-quickstart ls-tree -r HEAD NativeComponentQuickstart | grep -vE "node_modules|/android/|/ios/")
245-
hash=$(printf '%s\n%s\n' "$expo_tree" "$qs_tree" | sha256sum | cut -c1-16)
256+
hash=$(printf '%s\n%s\nios=%s\nandroid=%s\nsnapshot=%s\n' \
257+
"$expo_tree" "$qs_tree" \
258+
"$CLERK_IOS_REF" "$CLERK_ANDROID_REF" "$CLERK_ANDROID_SNAPSHOT_SUFFIX" \
259+
| sha256sum | cut -c1-16)
246260
echo "hash=$hash" >> "$GITHUB_OUTPUT"
247-
echo "Binary source hash: $hash"
261+
echo "Binary source hash: $hash (ios=$CLERK_IOS_REF android=$CLERK_ANDROID_REF snapshot=$CLERK_ANDROID_SNAPSHOT_SUFFIX)"
248262
249263
- name: Restore Android APK cache
250264
# On hit, the entire build path below is skipped. Cache key includes
@@ -637,13 +651,22 @@ jobs:
637651
grep -nE "clerkAndroid(Api|Ui)Version" "$file" | head
638652
639653
- name: Compute binary source hash
654+
# See the matching step in the Android job for the rationale on folding
655+
# the compat-gate ref inputs into the hash — same logic applies here.
640656
id: bin-hash
657+
env:
658+
CLERK_IOS_REF: ${{ inputs.clerk_ios_ref }}
659+
CLERK_ANDROID_REF: ${{ inputs.clerk_android_ref }}
660+
CLERK_ANDROID_SNAPSHOT_SUFFIX: ${{ inputs.clerk_android_snapshot_suffix }}
641661
run: |
642662
expo_tree=$(git ls-tree -r HEAD packages/expo .github/workflows/mobile-e2e.yml | grep -v "/dist/")
643663
qs_tree=$(git -C clerk-expo-quickstart ls-tree -r HEAD NativeComponentQuickstart | grep -vE "node_modules|/android/|/ios/")
644-
hash=$(printf '%s\n%s\n' "$expo_tree" "$qs_tree" | sha256sum | cut -c1-16)
664+
hash=$(printf '%s\n%s\nios=%s\nandroid=%s\nsnapshot=%s\n' \
665+
"$expo_tree" "$qs_tree" \
666+
"$CLERK_IOS_REF" "$CLERK_ANDROID_REF" "$CLERK_ANDROID_SNAPSHOT_SUFFIX" \
667+
| sha256sum | cut -c1-16)
645668
echo "hash=$hash" >> "$GITHUB_OUTPUT"
646-
echo "Binary source hash: $hash"
669+
echo "Binary source hash: $hash (ios=$CLERK_IOS_REF android=$CLERK_ANDROID_REF snapshot=$CLERK_ANDROID_SNAPSHOT_SUFFIX)"
647670
648671
- name: Restore iOS .app cache
649672
id: app-cache
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
# MANUAL REGRESSION: full end-to-end Google OAuth completion from the
2+
# forgot-password screen. The launch-only portion is automated in
3+
# google-sso-from-forgot-password.yaml; this variant adds the steps after the
4+
# system permission dialog, which requires real Google credentials and a real
5+
# OAuth completion, neither of which exist on CI.
6+
#
7+
# Tagged `manual` so the default workflow's `--exclude-tags manual,skip` keeps
8+
# it out of automated runs. To run locally:
9+
# maestro test \
10+
# --exclude-tags androidOnly \
11+
# integration/mobile/flows/sign-in/google-sso-from-forgot-password-manual.yaml
12+
appId: com.clerk.clerkexpoquickstart
13+
tags:
14+
- regression
15+
- manual
16+
- iosOnly
17+
---
18+
- runFlow: ../common/open-app.yaml
19+
- runFlow: ../common/assert-signed-out.yaml
20+
- tapOn:
21+
text: "Enter your email or username"
22+
- inputText: ${CLERK_TEST_EMAIL}
23+
- tapOn:
24+
text: "Continue"
25+
index: 0
26+
- waitForAnimationToEnd:
27+
timeout: 3000
28+
- tapOn:
29+
text: "Forgot password?"
30+
- waitForAnimationToEnd:
31+
timeout: 3000
32+
- tapOn:
33+
text: "Sign in with Google"
34+
- waitForAnimationToEnd:
35+
timeout: 8000
36+
# Manual step: complete Google sign-in. After return, assert home screen.
37+
- runFlow: ../common/assert-signed-in.yaml
Lines changed: 33 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,21 @@
1-
# REGRESSION: iOS OAuth (SSO) sign-in failed silently when initiated from
2-
# the forgot-password screen of the native AuthView.
1+
# REGRESSION (automated portion): iOS OAuth sign-in launched silently from the
2+
# forgot-password screen of the native AuthView — tapping "Sign in with Google"
3+
# did nothing visible, with no ASWebAuthenticationSession opening. This flow
4+
# verifies the LAUNCH only: navigate to forgot-password, tap the SSO button,
5+
# and assert that the system OAuth presentation actually appears.
36
#
4-
# The quickstart app does not have a custom forgot-password screen, so this
5-
# flow navigates within the native AuthView to reach the forgot-password step
6-
# and then initiates Google SSO from there.
7-
#
8-
# NOTE: This flow requires a real Google OAuth flow. Marked as manual + regression.
7+
# The post-launch Google account-picker can't run on CI without a stubbed /
8+
# test-mode OAuth provider, so the flow cancels the system dialog after
9+
# verifying it appeared. The full-completion variant lives in
10+
# google-sso-from-forgot-password-manual.yaml.
911
appId: com.clerk.clerkexpoquickstart
1012
tags:
1113
- regression
12-
- manual
1314
- iosOnly
1415
---
1516
- runFlow: ../common/open-app.yaml
1617
- runFlow: ../common/assert-signed-out.yaml
17-
# Enter an email to get to the password screen where "Forgot password?" is available
18+
# Enter an email to get to the password screen where "Forgot password?" is available.
1819
- tapOn:
1920
text: "Enter your email or username"
2021
- inputText: ${CLERK_TEST_EMAIL}
@@ -23,15 +24,32 @@ tags:
2324
index: 0
2425
- waitForAnimationToEnd:
2526
timeout: 3000
26-
# Now on the password screen, tap "Forgot password?" to reach that step
27+
# Now on the password screen, tap "Forgot password?" to reach that step.
2728
- tapOn:
2829
text: "Forgot password?"
2930
- waitForAnimationToEnd:
3031
timeout: 3000
31-
# Tap Google SSO from the forgot-password context
32+
- takeScreenshot: debug-sso-forgot-password-pre-tap
33+
# This is the regression-critical step. Before the fix this tap was a silent
34+
# no-op. After the fix iOS opens an ASWebAuthenticationSession, which first
35+
# shows a system permission dialog ("<App> Wants to Use <domain> to Sign In",
36+
# with Cancel/Continue buttons). We assert ANY of the system-provided strings
37+
# is visible — that's enough to prove OAuth actually launched.
3238
- tapOn:
3339
text: "Sign in with Google"
34-
- waitForAnimationToEnd:
35-
timeout: 8000
36-
# Manual step: complete Google sign-in. After return, assert home screen.
37-
- runFlow: ../common/assert-signed-in.yaml
40+
- extendedWaitUntil:
41+
visible: "Wants to Use|accounts\\.google\\.com|google\\.com|Continue"
42+
timeout: 10000
43+
- takeScreenshot: debug-sso-forgot-password-launched
44+
# Cancel the OAuth flow — completing the Google sign-in needs real credentials
45+
# we don't have on CI. We've already verified what the regression cares about
46+
# (the launch). Be lenient about what cancel-target is on screen since the
47+
# exact label varies between the iOS permission dialog (Cancel) and the
48+
# loaded Safari sheet (Done / Cancel).
49+
- runFlow:
50+
when:
51+
visible: "Cancel|Done"
52+
commands:
53+
- tapOn:
54+
text: "Cancel|Done"
55+
optional: true
Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
#!/usr/bin/env bash
2+
# Pre-filters Maestro flow files by tag exclusion.
3+
#
4+
# Background: `maestro test --exclude-tags X,Y` only filters when invoked
5+
# against a directory. When you pass explicit file paths (which run-ios.sh /
6+
# run-android.sh / run-regressions.sh all do, because flows live in nested
7+
# directories that Maestro does not auto-recurse into), the --exclude-tags
8+
# flag is silently ignored and every file is executed regardless of tag.
9+
#
10+
# This helper closes that hole by scanning each file for an exact tag match
11+
# in its YAML frontmatter and dropping any file whose tag list intersects
12+
# the excluded set, before the file list ever reaches Maestro.
13+
#
14+
# Usage:
15+
# source ./lib/filter-flows.sh
16+
# filtered=$(filter_flows "manual,skip,androidOnly" "${FLOW_FILES[@]}")
17+
# readarray -t KEEP <<< "$filtered" # or pipe to xargs -n 1
18+
#
19+
# Stdout: kept file paths (one per line, in input order).
20+
# Stderr: a one-line "Skip <path> (...)" notice for each dropped file.
21+
22+
filter_flows() {
23+
local excluded_csv="$1"; shift
24+
if [[ -z "$excluded_csv" ]]; then
25+
printf '%s\n' "$@"
26+
return 0
27+
fi
28+
local pattern
29+
pattern=$(printf '%s' "$excluded_csv" | sed 's/,/|/g')
30+
local f
31+
for f in "$@"; do
32+
if [[ ! -f "$f" ]]; then
33+
echo "Skip $f (file not found)" >&2
34+
continue
35+
fi
36+
# Match a list-item tag entry in the YAML frontmatter, e.g.
37+
# tags:
38+
# - manual
39+
# We deliberately anchor on `^\s*-\s*<tag>\s*$` so a flow body that
40+
# happens to contain the word "manual" doesn't get filtered out.
41+
if grep -qE "^[[:space:]]*-[[:space:]]*(${pattern})[[:space:]]*\$" "$f"; then
42+
echo "Skip $f (matches excluded tag in ${excluded_csv})" >&2
43+
else
44+
printf '%s\n' "$f"
45+
fi
46+
done
47+
}

integration/mobile/scripts/run-android.sh

Lines changed: 18 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ set -euo pipefail
44

55
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
66
FLOWS_DIR="$SCRIPT_DIR/../flows"
7+
# shellcheck disable=SC1091
8+
source "$SCRIPT_DIR/lib/filter-flows.sh"
79

810
if [[ -f "$SCRIPT_DIR/../config/.env" ]]; then
911
set -a
@@ -18,20 +20,28 @@ if ! command -v maestro >/dev/null 2>&1; then
1820
fi
1921

2022
echo "==> Running all non-manual flows on Android..."
21-
# Maestro does not auto-recurse into subdirectories. Pass each flow file
22-
# explicitly to pick up flows/sign-in/, flows/profile/, etc. Skip the
23-
# flows/common/ directory — those are subflows invoked via runFlow.
24-
# Use while-read to stay compatible with macOS bash 3.2 (no mapfile).
25-
FLOW_FILES=()
23+
ALL_FLOWS=()
2624
while IFS= read -r f; do
27-
FLOW_FILES+=("$f")
25+
ALL_FLOWS+=("$f")
2826
done < <(find "$FLOWS_DIR" -type f -name "*.yaml" ! -path "*/common/*")
2927

28+
# Maestro's --exclude-tags is a no-op when explicit file paths are passed,
29+
# so pre-filter the list ourselves before handing it off.
30+
KEEP=()
31+
while IFS= read -r f; do
32+
[[ -z "$f" ]] && continue
33+
KEEP+=("$f")
34+
done < <(filter_flows "iosOnly,manual,skip" "${ALL_FLOWS[@]}")
35+
36+
if [[ ${#KEEP[@]} -eq 0 ]]; then
37+
echo "No flows to run after tag filtering." >&2
38+
exit 0
39+
fi
40+
3041
maestro --platform android test \
31-
--exclude-tags iosOnly,manual,skip \
3242
-e CLERK_TEST_EMAIL="${CLERK_TEST_EMAIL}" \
3343
-e CLERK_TEST_PASSWORD="${CLERK_TEST_PASSWORD}" \
3444
-e CLERK_TEST_EMAIL_SECONDARY="${CLERK_TEST_EMAIL_SECONDARY:-}" \
3545
-e CLERK_TEST_PASSWORD_SECONDARY="${CLERK_TEST_PASSWORD_SECONDARY:-}" \
3646
"$@" \
37-
"${FLOW_FILES[@]}"
47+
"${KEEP[@]}"

integration/mobile/scripts/run-ios.sh

Lines changed: 20 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ set -euo pipefail
44

55
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
66
FLOWS_DIR="$SCRIPT_DIR/../flows"
7+
# shellcheck disable=SC1091
8+
source "$SCRIPT_DIR/lib/filter-flows.sh"
79

810
if [[ -f "$SCRIPT_DIR/../config/.env" ]]; then
911
set -a
@@ -18,20 +20,31 @@ if ! command -v maestro >/dev/null 2>&1; then
1820
fi
1921

2022
echo "==> Running all non-manual flows on iOS..."
21-
# Maestro does not auto-recurse into subdirectories. Pass each flow file
22-
# explicitly to pick up flows/sign-in/, flows/profile/, etc. Skip the
23-
# flows/common/ directory — those are subflows invoked via runFlow.
23+
# Collect every flow file under flows/, excluding the common/ subflow dir.
2424
# Use while-read to stay compatible with macOS bash 3.2 (no mapfile).
25-
FLOW_FILES=()
25+
ALL_FLOWS=()
2626
while IFS= read -r f; do
27-
FLOW_FILES+=("$f")
27+
ALL_FLOWS+=("$f")
2828
done < <(find "$FLOWS_DIR" -type f -name "*.yaml" ! -path "*/common/*")
2929

30+
# Maestro's --exclude-tags is a no-op when explicit file paths are passed,
31+
# so pre-filter the list ourselves before handing it off. See
32+
# scripts/lib/filter-flows.sh for the why.
33+
KEEP=()
34+
while IFS= read -r f; do
35+
[[ -z "$f" ]] && continue
36+
KEEP+=("$f")
37+
done < <(filter_flows "androidOnly,manual,skip" "${ALL_FLOWS[@]}")
38+
39+
if [[ ${#KEEP[@]} -eq 0 ]]; then
40+
echo "No flows to run after tag filtering." >&2
41+
exit 0
42+
fi
43+
3044
maestro --platform ios test \
31-
--exclude-tags androidOnly,manual,skip \
3245
-e CLERK_TEST_EMAIL="${CLERK_TEST_EMAIL}" \
3346
-e CLERK_TEST_PASSWORD="${CLERK_TEST_PASSWORD}" \
3447
-e CLERK_TEST_EMAIL_SECONDARY="${CLERK_TEST_EMAIL_SECONDARY:-}" \
3548
-e CLERK_TEST_PASSWORD_SECONDARY="${CLERK_TEST_PASSWORD_SECONDARY:-}" \
3649
"$@" \
37-
"${FLOW_FILES[@]}"
50+
"${KEEP[@]}"

integration/mobile/scripts/run-regressions.sh

Lines changed: 26 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ set -euo pipefail
66
SCRIPT_DIR="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
77
FLOWS_DIR="$SCRIPT_DIR/../flows"
88
PLATFORM="${1:-both}"
9+
# shellcheck disable=SC1091
10+
source "$SCRIPT_DIR/lib/filter-flows.sh"
911

1012
REGRESSION_FLOWS=(
1113
"$FLOWS_DIR/sign-in/google-sso-from-forgot-password.yaml"
@@ -22,29 +24,42 @@ if [[ -f "$SCRIPT_DIR/../config/.env" ]]; then
2224
set +a
2325
fi
2426

27+
# Maestro's --exclude-tags is a no-op when explicit file paths are passed.
28+
# Pre-filter the regression list for each platform before invoking Maestro
29+
# so a flow tagged `manual` or `skip` (or for the other platform) cannot
30+
# sneak in just because it's listed above.
2531
run_on() {
2632
local platform_name="$1"
27-
shift
33+
local exclude_tags="$2"
34+
shift 2
35+
2836
echo "==> Running regression flows on $platform_name..."
29-
for flow in "${REGRESSION_FLOWS[@]}"; do
30-
if [[ -f "$flow" ]]; then
31-
maestro test "$@" "$flow"
32-
else
33-
echo "Skipping missing flow: $flow"
34-
fi
37+
KEEP=()
38+
while IFS= read -r f; do
39+
[[ -z "$f" ]] && continue
40+
KEEP+=("$f")
41+
done < <(filter_flows "$exclude_tags" "${REGRESSION_FLOWS[@]}")
42+
43+
if [[ ${#KEEP[@]} -eq 0 ]]; then
44+
echo "No regression flows to run on $platform_name after filtering." >&2
45+
return 0
46+
fi
47+
48+
for flow in "${KEEP[@]}"; do
49+
maestro test "$@" "$flow"
3550
done
3651
}
3752

3853
case "$PLATFORM" in
3954
ios)
40-
run_on "iOS" --device "${MAESTRO_DEVICE:-iPhone 16 Pro}" --exclude-tags androidOnly
55+
run_on "iOS" "androidOnly,manual,skip" --device "${MAESTRO_DEVICE:-iPhone 16 Pro}"
4156
;;
4257
android)
43-
run_on "Android" --exclude-tags iosOnly
58+
run_on "Android" "iosOnly,manual,skip"
4459
;;
4560
both)
46-
run_on "iOS" --device "${MAESTRO_DEVICE:-iPhone 16 Pro}" --exclude-tags androidOnly
47-
run_on "Android" --exclude-tags iosOnly
61+
run_on "iOS" "androidOnly,manual,skip" --device "${MAESTRO_DEVICE:-iPhone 16 Pro}"
62+
run_on "Android" "iosOnly,manual,skip"
4863
;;
4964
*)
5065
echo "Usage: $0 [ios|android|both]" >&2

0 commit comments

Comments
 (0)