feat(pos-app): Add Maestro E2E testing infrastructure #303
Conversation
- Add payment flow E2E test (new sale -> enter amount -> QR code -> open URL) - Add testID props to Button component and key interactive elements - Add GitHub Action workflow for running Maestro tests on Android - Update AGENTS.md with testing guidelines: - Avoid testing mocked components - Use testID for E2E tests - Minimal comments rule - No unused variables rule
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Claude finished @ignaciosantise's task in 1m 30s —— View job Review CompleteFound 3 issue(s)Issue 1: App ID mismatch between workflow and E2E testID: payment-app-id-mismatch-a7b3 The E2E test file uses Recommendation: Verify the correct app ID from Issue 2: Insufficient E2E test coverage for error scenariosID: payment-edge-case-coverage-2f91 The payment flow test only covers the happy path. Critical edge cases are not tested: invalid amounts (negative, zero with validation bypassed, exceeding limits), network failures during payment initiation, QR code generation failures, and merchant configuration errors. Recommendation: Add additional test files for error scenarios: # e2e/payment-flow-invalid-amount.yaml
- tapOn: { id: "key-0" }
- tapOn: { id: "charge-button" }
- assertVisible: { text: "Amount is required" }
# e2e/payment-flow-no-merchant-config.yaml
# Clear merchant settings first, then verify error handlingIssue 3: Hardcoded amount assertion may fail with formatting variationsID: payment-amount-assertion-fragile-8c42 The assertion Recommendation: Use a more flexible assertion pattern or verify the amount is visible within a specific container: # Option 1: Check for amount components separately
- assertVisible: "0.01"
- assertVisible: "USD"
# Option 2: Use regex if Maestro supports it
- assertVisible:
text: "0\\.01.*USD"Automated Checks🔒 External Domain URL Detected (Non-blocking)
This change introduces URLs pointing to external domains. Please verify that these external dependencies are intentional and review for potential security, privacy, or compliance implications. Approved company domains are: reown.com, walletconnect.com, walletconnect.org Note: Maestro is a legitimate E2E testing tool. Consider pinning to a specific version rather than using the latest version from the install script for better reproducibility and security. |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| @@ -0,0 +1,39 @@ | |||
| appId: com.reown.mobilepos | |||
There was a problem hiding this comment.
🤖 Auto Review Issue: App ID mismatch between workflow and E2E test
Severity: HIGH
Category: configuration
Tool: Claude Auto Review
Recommendation: Verify the correct app ID from dapps/pos-app/app.json and update the E2E test file to use the matching package name. The app ID should match the value in expo.android.package or the build configuration.
| @@ -0,0 +1,39 @@ | |||
| appId: com.reown.mobilepos | |||
There was a problem hiding this comment.
🤖 Auto Review Issue: Insufficient E2E test coverage for error scenarios
Severity: MEDIUM
Category: code_quality
Tool: Claude Auto Review
Recommendation: Add additional test files for error scenarios:
# e2e/payment-flow-invalid-amount.yaml
- tapOn: { id: "key-0" }
- tapOn: { id: "charge-button" }
- assertVisible: { text: "Amount is required" }
# e2e/payment-flow-no-merchant-config.yaml
# Clear merchant settings first, then verify error handling|
|
||
| # Step 6: Verify amount is $0.01 | ||
| - assertVisible: | ||
| text: "0.01 USD" |
There was a problem hiding this comment.
🤖 Auto Review Issue: Hardcoded amount assertion may fail with formatting variations
Severity: LOW
Category: code_quality
Tool: Claude Auto Review
Recommendation: Use a more flexible assertion pattern or verify the amount is visible within a specific container:
# Option 1: Check for amount components separately
- assertVisible: "0.01"
- assertVisible: "USD"
# Option 2: Use regex if Maestro supports it
- assertVisible:
text: "0\\.01.*USD"There was a problem hiding this comment.
Pull request overview
This PR adds Maestro E2E testing infrastructure to the POS app, enabling automated end-to-end testing of key user flows on Android. The implementation includes test files, component instrumentation with testIDs, a GitHub Actions workflow for CI, and updated developer guidelines.
- Adds Maestro E2E testing framework with payment flow test coverage
- Adds testID props to interactive components for test automation
- Implements CI workflow with Android emulator and Slack notifications
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
dapps/pos-app/e2e/payment-flow.yaml |
Defines E2E test for payment flow from sale creation to QR code generation |
dapps/pos-app/e2e/README.md |
Provides documentation for running and creating Maestro tests |
dapps/pos-app/components/button.tsx |
Adds testID prop support to Button component |
dapps/pos-app/components/numeric-keyboard.tsx |
Moves testID from child elements to parent Button for better E2E targeting |
dapps/pos-app/app/scan.tsx |
Changes QR tap behavior to open URL in browser and adds testID |
dapps/pos-app/app/index.tsx |
Adds testIDs to home screen buttons |
dapps/pos-app/app/amount.tsx |
Adds testID to charge button |
dapps/pos-app/AGENTS.md |
Documents testing guidelines and coding standards |
.github/workflows/ci_e2e_tests_pos_android.yaml |
Implements CI workflow for running E2E tests on Android emulator |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ## Adding New Tests | ||
|
|
||
| 1. Create a new `.yaml` file in this directory | ||
| 2. Start with `appId: com.reown.pos` |
There was a problem hiding this comment.
The appId in the example shows 'com.reown.pos' but the actual appId should be 'com.reown.mobilepos' as specified in line 46. This inconsistency could confuse developers adding new tests.
| 2. Start with `appId: com.reown.pos` | |
| 2. Start with `appId: com.reown.mobilepos` |
| await Linking.openURL(qrUri); | ||
| } catch { | ||
| showErrorToast("Failed to open payment URL"); |
There was a problem hiding this comment.
The function behavior has changed from copying the URL with user feedback (via showSuccessToast) to copying the URL and opening it in a browser. This is a breaking change in user experience: users will now have their browser opened automatically when they tap the QR code, rather than just copying the URL. Consider whether this behavior change is intentional and whether it should be documented in the PR description. If automatic browser opening is the desired behavior, the clipboard copy may be unnecessary since the user didn't explicitly request a copy action.
| await Linking.openURL(qrUri); | |
| } catch { | |
| showErrorToast("Failed to open payment URL"); | |
| showSuccessToast("Payment URL copied to clipboard"); | |
| } catch { | |
| showErrorToast("Failed to copy payment URL"); |
| - name: Create env file | ||
| run: | | ||
| touch dapps/pos-app/.env | ||
| echo ${{ vars.POS_DEV_ENV_FILE }} >> dapps/pos-app/.env |
There was a problem hiding this comment.
The command uses '>>' to append to the .env file, but other workflows in the repository use '>' to overwrite (see release-android-base.yaml line 114). Since the file is just created with 'touch', using '>' would be more consistent with existing patterns and more explicit about creating the file content. Additionally, if POS_DEV_ENV_FILE contains multiple lines, this echo command may not handle them correctly without proper quoting. Consider using: echo "${{ vars.POS_DEV_ENV_FILE }}" > dapps/pos-app/.env
| echo ${{ vars.POS_DEV_ENV_FILE }} >> dapps/pos-app/.env | |
| echo "${{ vars.POS_DEV_ENV_FILE }}" > dapps/pos-app/.env |
| disable-animations: true | ||
| script: | | ||
| adb install dapps/pos-app/android/app/build/outputs/apk/release/app-release.apk | ||
| $HOME/.maestro/bin/maestro test dapps/pos-app/e2e/ --format junit --output maestro-report.xml |
There was a problem hiding this comment.
The maestro command outputs to 'maestro-report.xml' without specifying a directory, which means it will be created in the current working directory. Since the script runs in the emulator context and the working directory is not explicitly set, the output file location may be unpredictable. Consider using an absolute path or explicitly setting the working directory, for example: '$HOME/.maestro/bin/maestro test dapps/pos-app/e2e/ --format junit --output ${{ github.workspace }}/maestro-report.xml'
| $HOME/.maestro/bin/maestro test dapps/pos-app/e2e/ --format junit --output maestro-report.xml | |
| $HOME/.maestro/bin/maestro test dapps/pos-app/e2e/ --format junit --output "${{ github.workspace }}/maestro-report.xml" |
| # Step 5: Tap QR code to open payment URL in browser | ||
| - tapOn: | ||
| id: "qr-code" | ||
|
|
||
| # Step 6: Verify amount is $0.01 | ||
| - assertVisible: | ||
| text: "0.01 USD" | ||
|
|
There was a problem hiding this comment.
After tapping the QR code (line 31), the test expects to assert that "0.01 USD" is visible (line 35). However, tapping the QR code opens an external browser via Linking.openURL (as modified in scan.tsx). Maestro may not be able to interact with or assert text in external browser applications, which could cause this test to fail. Consider either: (1) adding a wait/assertion that handles the app context switch, (2) verifying this behavior works in practice, or (3) restructuring the test to avoid relying on external browser interaction.
| # Step 5: Tap QR code to open payment URL in browser | |
| - tapOn: | |
| id: "qr-code" | |
| # Step 6: Verify amount is $0.01 | |
| - assertVisible: | |
| text: "0.01 USD" | |
| # Step 5: Verify amount is $0.01 while still in the POS app | |
| - assertVisible: | |
| text: "0.01 USD" | |
| # Step 6: Tap QR code to open payment URL in browser | |
| - tapOn: | |
| id: "qr-code" |
Summary
Changes
E2E Tests (
dapps/pos-app/e2e/)payment-flow.yaml- Tests the payment flow: New sale → Enter amount → QR code → Open payment URLtestID Support
testIDprop toButtoncomponentnew-sale-button,settings-button(home screen)charge-button(amount screen)key-0throughkey-9,key-dot,key-erase(numeric keyboard)qr-code(scan screen)GitHub Action (
.github/workflows/ci_e2e_tests_pos_android.yaml)AGENTS.md Updates
Test Plan
maestro test e2e/locally to verify flow