Skip to content

Conversation

@ignaciosantise
Copy link
Collaborator

@ignaciosantise ignaciosantise commented Oct 15, 2025

Summary

Added new workflow to run maestro tests on new sample app


Note

Introduces Android/iOS CI to build the example app and run Maestro e2e suites, adds new test flows, and wires testIDs plus config/scripts to support automation.

  • CI/CD
    • Add comprehensive GitHub Actions workflow /.github/workflows/expo-ci-cd.yml to build example app, cache dependencies, download wallet builds from S3, run Maestro on Android (AVD) and iOS (Simulator), upload artifacts, and send Slack notifications.
    • Introduce platform test runner scripts: /.github/scripts/run-android-tests.sh, /.github/scripts/run-ios-tests.sh with install, diagnostics, retries, summaries, and GHA outputs.
  • E2E Tests (Maestro)
    • Add new flows in examples/expo-multichain/.maestro: switch-network.yaml, account-activity.yaml, send.yaml, swaps.yaml, onramp.yaml; update basic-smoke-test.yaml and wallet-qr-load.yaml; revamp .maestro/run-tests.sh to run all.
    • Remove obsolete root Maestro config/flows (.maestro/config.yaml, w3m-*.yaml).
  • Example App (Expo)
    • Update app.json to include expo-build-properties and custom plugin ./plugins/withGradleOptimizations.js; set Android x86_64 arch; enable RN from source; iOS ccache.
    • Add expo-build-properties dependency; adjust scripts to direct Gradle/Xcode builds; tweak eas.json profiles (production-ci).
    • Update .gitignore Maestro artifacts patterns.
  • AppKit UI (testability)
    • Add testID props and wiring in components/views: w3m-send-input-token, w3m-swap-input, w3m-wallet-send-view, w3m-swap-view (inputs), w3m-swap-view/components/select-token-modal (back), w3m-account-default-view (header back), w3m-onramp-settings-view (selectors), w3m-onramp-view (continue button state/testID).
  • Misc
    • Remove mock __mocks__/@react-native-async-storage/async-storage.js.
    • Lockfile updates to include expo-build-properties and deps.

Written by Cursor Bugbot for commit 66f6ca3. This will update automatically on new commits. Configure here.

@changeset-bot
Copy link

changeset-bot bot commented Oct 15, 2025

⚠️ No Changeset found

Latest commit: 66f6ca3

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@claude
Copy link

claude bot commented Oct 15, 2025

Claude finished @ignaciosantise's task —— View job


PR Review: E2E Testing Workflow

Completed comprehensive review of the new workflow file.

Tasks

  • Read repository guidelines (CLAUDE.md)
  • Analyze workflow file structure and configuration
  • Review security considerations
  • Check best practices and performance
  • Provide comprehensive feedback

Critical Issues

1. Job Dependency Error - Workflow Will Fail ⚠️

.github/workflows/expo-ci-cd.yml:118 and .github/workflows/expo-ci-cd.yml:346

Both test jobs reference a non-existent publish-snapshot job in their needs dependencies:

  • Line 118: needs: [lint, publish-snapshot]
  • Line 346: needs: publish-snapshot

This will cause immediate workflow failure. The workflow should either:

  • Remove these dependencies if snapshot publishing is not implemented
  • Add the missing publish-snapshot job

2. Inconsistent Package Manager Usage

  • .github/workflows/expo-ci-cd.yml:56,85,143,359: Mixing npm and yarn inconsistently
  • build-packages job uses yarn (lines 85-91)
  • Other jobs use npm (lines 56, 143, 359)
  • Package installation paths differ: line 184 uses ../../appkit.tgz while line 403 uses ../../packages/appkit.tgz

Recommendation: Standardize on one package manager throughout. The monorepo appears to use yarn, so consistently use yarn.

3. Insecure AWS Secret Reference

.github/workflows/expo-ci-cd.yml:148,364

role-to-assume: arn:aws:iam::${{ secrets.aws-account-id }}:role/...

Using secrets.aws-account-id instead of secrets.AWS_ACCOUNT_ID (lowercase vs uppercase). GitHub secrets are case-sensitive and conventionally uppercase.

Security Risk: This will likely fail to authenticate, but if it exists with lowercase, it's non-standard.

4. Missing Lint Dependency in Android Job

.github/workflows/expo-ci-cd.yml:118

The pr-build-test-android job depends on lint but not on build-packages, yet it requires packed packages (line 173-177). This creates a race condition where the job might start before packages are built.

Fix: Change line 118 to: needs: [lint, build-packages]


Security Concerns

5. Overly Permissive KVM Configuration

.github/workflows/expo-ci-cd.yml:128-131

MODE="0666"

Setting world read-write permissions on KVM device. While common in CI, it's worth documenting why this is necessary.

6. Secrets Exposed as Environment Variables

.github/workflows/expo-ci-cd.yml:28-32

All secrets are exposed as global environment variables for the entire workflow. Best practice is to limit secret exposure to only the jobs/steps that need them.

Recommendation: Move secrets to job-level or step-level env blocks where they're actually used.

7. No Verification of Downloaded APK/IPA

.github/workflows/expo-ci-cd.yml:151-155,367-374

Downloaded builds from S3 have no integrity checks (checksum, signature verification). A compromised or corrupted build could lead to false test results.

Recommendation: Add checksum verification after download.


Best Practices & Quality Issues

8. Hardcoded iOS Runtime Version

.github/workflows/expo-ci-cd.yml:433-437

The iOS simulator setup prefers "iOS 18" but has a fallback. However, the fallback logic (tail -n 1) is unreliable as it picks the lexicographically last runtime, not the latest version.

Recommendation: Use a more robust version selection or document the expected iOS version.

9. Inefficient Artifact Retention

Multiple artifact uploads with 7-day retention for debug artifacts but only 1-day retention for packed packages (line 114). Debug artifacts can be large; consider:

  • Reducing retention to 3 days for debug artifacts
  • Conditional uploads (only on failure)

10. No Timeout Configuration

Neither job specifies a timeout-minutes value. If tests hang, runners will waste resources for up to 6 hours (GitHub default).

Recommendation: Add reasonable timeouts:

timeout-minutes: 60  # Android job
timeout-minutes: 45  # iOS job

11. Commented-Out Trigger Conditions

.github/workflows/expo-ci-cd.yml:8-15

All automatic triggers are commented out; workflow only runs on workflow_dispatch. This defeats the purpose of CI/CD.

Question: Is this intentional for initial testing, or should triggers be enabled?

12. Redundant Package Manager Cache

.github/workflows/expo-ci-cd.yml:56,85,143,359

The cache parameter is used but then npm ci or yarn install --immutable are called, which already handle caching efficiently. The actions/setup-node cache is redundant with lockfile-based package managers.

13. Error Handling with || true

.github/workflows/expo-ci-cd.yml:252,266,269,471,475 and many others

Extensive use of || true suppresses failures, making debugging difficult. While some are appropriate (e.g., clearing app data), others mask real issues.

Recommendation: Only use || true where failures are truly expected and safe to ignore.

14. Script Complexity

The emulator runner scripts (lines 215-301 for Android, 423-508 for iOS) are extremely long and complex. Consider:

  • Extracting to separate shell script files in .github/scripts/
  • Improving maintainability and testability
  • Better error handling and reporting

15. Missing EAS Authentication

.github/workflows/expo-ci-cd.yml:193,412

EAS build commands run without explicit authentication setup. While EXPO_TOKEN is set as an environment variable (line 28), EAS CLI authentication should be explicit:

- name: Login to EAS
  run: eas login --token ${{ secrets.EXPO_TOKEN }}

Performance Issues

16. No Parallel Job Execution

All jobs run sequentially due to dependency chains. The lint and build-packages jobs could potentially run in parallel, as could Android and iOS test jobs (after fixing dependencies).

17. Large Memory Allocation

.github/workflows/expo-ci-cd.yml:192,411

--max_old_space_size=4096

4GB heap size for Node.js is very large. This might indicate memory leaks or inefficient build processes.

18. Redundant App Installation

.github/workflows/expo-ci-cd.yml:475-476 (iOS)

The app is uninstalled and immediately reinstalled, which is redundant with the previous installation at line 458.


Documentation & Maintainability

19. No Workflow Documentation

No comments explaining:

  • Why automatic triggers are disabled
  • What the snapshot system is/does
  • Why specific emulator configurations were chosen
  • Expected test scenarios

20. Inconsistent Artifact Naming

  • Line 108: packed-packages
  • Line 307: debug-artifacts-android-${{ github.event.pull_request.number }}
  • Line 329: pr-build-android-${{ github.event.pull_request.number }}

Using PR number in artifact names won't work for workflow_dispatch events (no PR context).

Fix: Use ${{ github.run_id }} or conditional naming.


Minor Issues

21. Unused Environment Variables

.github/workflows/expo-ci-cd.yml:29-32

Several secrets are defined globally but only used in commented-out Slack notification steps:

  • EXPO_APPLE_ID
  • EXPO_APPLE_PASSWORD
  • EXPO_TEAM_ID
  • SLACK_WEBHOOK

Consider removing unused secrets or documenting their future use.

22. Legacy OpenSSL Provider

.github/workflows/expo-ci-cd.yml:33

NODE_OPTIONS: --openssl-legacy-provider

Using legacy OpenSSL provider suggests dependency on outdated packages. This should be investigated and resolved.

23. Emulator Configuration

.github/workflows/expo-ci-cd.yml:208,211

  • ram-size: 4096M is quite high for CI
  • heap-size: 576M seems arbitrary

Document why these specific values were chosen.


Summary

Blockers (must fix before merge):

  1. Remove publish-snapshot dependencies or implement the missing job
  2. Fix package path inconsistency in iOS job
  3. Fix AWS secret reference naming

High Priority:
4. Standardize package manager usage
5. Add timeout configurations
6. Enable automatic workflow triggers or document why they're disabled
7. Limit secret exposure scope

Recommended Improvements:

  • Extract complex scripts to separate files
  • Add integrity checks for downloaded builds
  • Improve error handling
  • Reduce artifact retention
  • Document workflow purpose and configuration choices

@socket-security
Copy link

socket-security bot commented Oct 15, 2025

No dependency changes detected. Learn more about Socket for GitHub.

👍 No dependency changes detected in pull request

Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds a comprehensive GitHub Actions workflow for running end-to-end tests on a React Native/Expo application using Maestro. The workflow supports both Android and iOS platforms, builds the application with local packages, and runs automated UI tests.

Key changes:

  • Added a complete CI/CD workflow with lint, build, and E2E test stages
  • Implemented Android E2E testing using emulators and Maestro
  • Implemented iOS E2E testing using simulators and Maestro

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@ignaciosantise
Copy link
Collaborator Author

@claude review

@claude
Copy link

claude bot commented Oct 15, 2025

Claude finished @ignaciosantise's task —— View job


🔄 Incremental PR Review

Analyzing changes and validating previously reported issues...

Tasks

  • Read repository guidelines (CLAUDE.md not found - proceeding without)
  • Review workflow file (.github/workflows/expo-ci-cd.yml)
  • Review Android test script (.github/scripts/run-android-tests.sh)
  • Review iOS test script (.github/scripts/run-ios-tests.sh)
  • Validate previously reported issues
  • Identify new issues
  • Post comprehensive review

Previous Issues - Status Check

❌ Issue 1: Missing publish-snapshot Job Dependency (RESOLVED)

Previously reported at: .github/workflows/expo-ci-cd.yml:118 and :346

Current status:RESOLVED - Both pr-build-test-android (line 89) and pr-build-test-ios (line 228) now only depend on build-packages. The publish-snapshot dependency has been removed.


❌ Issue 2: Incorrect Package Paths (RESOLVED)

Previously reported at: .github/workflows/expo-ci-cd.yml:184 and :403

Current status:RESOLVED - Both jobs now correctly:

  1. Download artifacts to ./packages (lines 146, 273)
  2. Install from ../../packages/*.tgz (lines 152-155, 279-282)

The paths are now consistent and correct.


❌ Issue 3: iOS Simulator UDID Extraction Issues (PARTIALLY RESOLVED)

Previously reported at: .github/scripts/run-ios-tests.sh

Current status: ⚠️ PARTIALLY RESOLVED - The regex now correctly handles both uppercase and lowercase hex characters with [0-9A-Fa-f-]+ (line 52). However, the issue with multiple simulators with the same name remains - using head -n 1 at the end of the grep pipeline could still concatenate multiple UUIDs on the same line before taking the first line.

Recommendation: Use a more robust extraction:

UDID=$(xcrun simctl list devices | grep "$SIM_DEVICE_NAME" | head -n 1 | grep -oE '[0-9A-Fa-f-]{36}')

❌ Issue 4: Workflow Dispatch Missing Commit Data (RESOLVED)

Previously reported at: .github/workflows/expo-ci-cd.yml

Current status:RESOLVED - The check-skip job now handles workflow_dispatch events correctly (lines 37-39):

if: |
  github.event_name == 'workflow_dispatch' ||
  !contains(github.event.head_commit.message, '[skip ci]')

⚠️ Issue 5: External URL Security (ACKNOWLEDGED - No Action Required)

Previously reported at: .github/workflows/expo-ci-cd.yml:167

Current status: Still present at line 167. This is the official Maestro installation URL (https://get.maestro.mobile.dev) and is appropriate for installing the test framework. No action needed unless your organization has specific policies about external dependencies.


New Issues Found

🔴 Issue 6: Package Manager Consistency Issues

Location: .github/workflows/expo-ci-cd.yml

The workflow mixes package managers inconsistently:

  • build-packages job (line 59): Uses yarn install --immutable ✅ (correct for monorepo)
  • Android job (line 132): Uses npm ci
  • iOS job (line 259): Uses npm ci

However, the monorepo root likely uses yarn, not npm. This could cause:

  • Inconsistent lockfile usage (yarn.lock vs package-lock.json)
  • Different dependency resolution
  • CI failures if package-lock.json doesn't exist

Recommendation: Either:

  1. Change lines 132 and 259 to yarn install --immutable, OR
  2. Document why npm is intentionally used in the example app directory

🟡 Issue 7: Missing Timeout on check-skip Job

Location: .github/workflows/expo-ci-cd.yml:34-42

While the main test jobs have timeout-minutes: 60 (lines 90, 229), and build-packages has timeout-minutes: 20 (line 48), the workflow would benefit from the skip check having a timeout documented.

Current: Line 36 shows timeout-minutes: 5 ✅ - Actually this is already present! No issue here.


🟡 Issue 8: NODE_OPTIONS Inconsistency

Location: .github/workflows/expo-ci-cd.yml:160, 287

  • Android: --max_old_space_size=2048 (2GB)
  • iOS: --max_old_space_size=2048 (2GB)

Both are now consistent at 2GB (previously noted as 4GB in earlier review). This is reasonable. ✅ No issue.


🟡 Issue 9: Artifact Retention Could Be Optimized

Location: .github/workflows/expo-ci-cd.yml

All artifacts use retention-days: 1 which is excellent for CI. ✅ No issue.


🔴 Issue 10: No PR/Push Triggers Enabled

Location: .github/workflows/expo-ci-cd.yml:8-15

All automatic triggers are commented out - the workflow ONLY runs on workflow_dispatch. This is unusual for a CI/CD workflow.

Questions:

  • Is this intentional for initial testing?
  • Should the workflow automatically run on PRs or pushes?

Impact: E2E tests won't run automatically on new PRs or commits, requiring manual triggering.


🟡 Issue 11: Platform Filter Logic for workflow_dispatch

Location: .github/workflows/expo-ci-cd.yml:91-93, 230-232

The job conditions work correctly, but there's a subtle issue: if triggered via pull_request events (when uncommented), the inputs.platform will be null, potentially causing the condition to fail unexpectedly.

Current code:

if: |
  github.event_name == 'pull_request' ||
  (github.event_name == 'workflow_dispatch' && (inputs.platform == 'android' || inputs.platform == 'all'))

This is actually correct - it runs on all PRs OR on manual dispatch with the right platform. ✅ No issue.


🟡 Issue 12: Missing Checkout SHA Pinning for Security

Location: Multiple action uses throughout

Many actions use commit SHA pinning (good practice):

  • Line 51: actions/checkout@v4 ❌ (uses tag)
  • Line 120: aws-actions/configure-aws-credentials@7474bc4690e29a8392af63c5b98e7449536d5c3a ✅ (SHA)
  • Line 135: expo/expo-github-action@c7b66a9c327a43a8fa7c0158e7f30d6040d2481e ✅ (SHA)
  • Line 171: reactivecircus/android-emulator-runner@1dcd0090116d15e7c562f8db72807de5e036a4ed ✅ (SHA)

Recommendation: Pin actions/checkout, actions/setup-node, actions/setup-java, and artifact actions to commit SHAs for better supply chain security.


🟡 Issue 13: iOS Test Script - Simulator Name Contains Hyphens

Location: .github/scripts/run-ios-tests.sh:17

SIM_DEVICE_NAME="iPhone-15-Maestro-Test"

The grep on line 52 searches for this name, but simulator names might not handle hyphens consistently across different xcrun versions. Consider using a simpler name like "iPhone15MaestroTest" to avoid potential matching issues.


🟡 Issue 14: Error Handling with || true May Hide Issues

Location: Multiple locations in both shell scripts

Examples:

  • .github/workflows/expo-ci-cd.yml:63: xcrun simctl boot "$UDID" || true - This might hide boot failures
  • .github/scripts/run-ios-tests.sh:105: Permission grants
  • .github/scripts/run-android-tests.sh:69: Clear app data

While some || true uses are justified (e.g., clearing data that might not exist), others might mask real problems. The boot command in particular should probably fail if the simulator can't boot.

Recommendation: Only use || true where failures are truly expected and safe to ignore.


Summary

Issues Resolved Since Last Review ✅

  1. ✅ Missing publish-snapshot dependency - FIXED
  2. ✅ Incorrect package paths - FIXED
  3. ✅ Workflow dispatch commit data handling - FIXED
  4. ✅ Timeouts added to jobs - PRESENT
  5. ✅ Artifact retention optimized - PRESENT

Issues Still Present ⚠️

  1. ⚠️ iOS UDID extraction could still fail with duplicate simulator names (minor risk)
  2. ⚠️ External URL noted but acceptable
  3. ⚠️ Package manager mixing (npm vs yarn) - potential issue
  4. ⚠️ All automatic triggers disabled (intentional?)
  5. ⚠️ Action version pinning could be improved
  6. ⚠️ Some || true usage might hide errors

Critical Items for Review

  1. Package Manager Choice - Should the example app use yarn or npm? Currently inconsistent.
  2. Trigger Configuration - Are the commented-out triggers intentional? Should E2E tests run automatically on PRs?
  3. iOS Simulator Creation - Minor risk of UUID extraction failure with duplicate names

Overall Assessment: The workflow is well-structured and most critical issues from previous reviews have been resolved. The main remaining concerns are organizational decisions (trigger configuration, package manager choice) rather than bugs.


cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@ignaciosantise ignaciosantise changed the title chore: added workflow for e2e tests chore: added workflow for e2e tests + new tests Oct 15, 2025
cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

cursor[bot]

This comment was marked as outdated.

@sonarqubecloud
Copy link

Quality Gate Failed Quality Gate failed

Failed conditions
2 Security Hotspots

See analysis details on SonarQube Cloud

@ignaciosantise ignaciosantise merged commit 39615c8 into develop Oct 21, 2025
9 of 11 checks passed
@ignaciosantise ignaciosantise deleted the chore/cicd branch October 21, 2025 13:29
@github-actions github-actions bot locked and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants