[CI] Add basic Argent end-to-end tests - #4318
Conversation
There was a problem hiding this comment.
Pull request overview
Adds an Argent-driven “Basic Tap” end-to-end test flow and CI workflows to run it on Android and iOS, plus small UI/test hooks in the example app so the flow can reliably locate elements and assert gesture callback ordering.
Changes:
- Add Android and iOS GitHub Actions workflows to build the Expo example app and run the
simple-tap-testArgent flow. - Update the “Tap” example to expose a deterministic tap counter, log ordered gesture lifecycle events, and add a
testIDfor the tappable box. - Add missing
testIDs for console modal buttons and rename “Simple Gestures” entries to “Basic …” to match the test flow.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| apps/common-app/src/new_api/simple/tap/index.tsx | Adds tap counter UI, ordered logging, and testID for E2E automation. |
| apps/common-app/src/new_api/index.tsx | Renames “Simple Gestures” examples to “Basic …” for discoverability/E2E selection. |
| apps/common-app/src/console/ConsoleModal.tsx | Adds testIDs needed for console interactions in the E2E flow. |
| .github/workflows/ios-e2e.yml | New CI workflow to build iOS simulator app and run Argent tap flow. |
| .github/workflows/android-e2e.yml | New CI workflow to build Android app, boot emulator, and run Argent tap flow. |
| .argent/flows/simple-tap-test.yaml | Defines the Argent flow steps/assertions for the “Basic Tap” E2E test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
958acd2 to
085d4c1
Compare
69c7aba to
25e2de2
Compare
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe PR adds Argent flows for tap and long-press examples, instruments gesture callbacks and counts, adds a tool-server action, and introduces reusable Android and iOS E2E workflows with caller integration. ChangesGesture example instrumentation
Argent server and platform workflows
Workflow integration and flows
Sequence Diagram(s)sequenceDiagram
participant Workflow
participant EmulatorOrSimulator
participant ArgentServer
participant ArgentFlow
participant GestureExample
Workflow->>EmulatorOrSimulator: install Expo artifact
Workflow->>ArgentServer: start and await readiness
ArgentFlow->>EmulatorOrSimulator: open gesture example
ArgentFlow->>GestureExample: perform tap or long press
GestureExample->>ArgentFlow: expose count and callback logs
Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
b0f6c76 to
b417151
Compare
Add `android-e2e.yml` and `ios-e2e.yml`: reusable workflows that boot an emulator / simulator, install the app built by the build workflow and run the Argent flows against it. `android.yml` and `ios.yml` run them after the build. Installing and starting the Argent tool-server is shared by both platforms, so it lives in a composite action. The server is started in the foreground and polled for readiness — `--detach` SIGKILLs the whole process group after a hard 15s timeout, which a cold CI runner (first run downloads the simulator-server binary) does not meet.
b417151 to
4f80277
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (10)
apps/common-app/src/common.tsx (1)
144-160: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueMemoize the returned worklet.
logMessageandlogMessageWorkletare recreated on every render. The returned worklet therefore has a new identity each render, so it is re-serialized to the worklet runtime and it invalidates any gesture object that closes over it. Wrap both inuseCallbackto keep a stable identity.♻️ Proposed refactor
export function useIndexedLogger() { const messageCounter = useRef(0); - const logMessage = (message: string) => { + const logMessage = useCallback((message: string) => { messageCounter.current += 1; const indexedMessage = `${messageCounter.current}. ${message}`; console.log(indexedMessage); - }; + }, []); - const logMessageWorklet = (message: string) => { - 'worklet'; - // Schedule log on the JS thread so the console interceptor can pick it up - scheduleOnRN(logMessage, message); - }; - - return logMessageWorklet; + return useCallback( + (message: string) => { + 'worklet'; + // Schedule log on the JS thread so the console interceptor can pick it up + scheduleOnRN(logMessage, message); + }, + [logMessage] + ); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@apps/common-app/src/common.tsx` around lines 144 - 160, Update useIndexedLogger to wrap both logMessage and logMessageWorklet in useCallback with appropriate dependencies, preserving the existing counter and scheduling behavior while keeping the returned worklet identity stable across renders..github/workflows/ios-e2e.yml (1)
18-21: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd an explicit
permissions:block.The
e2ejob uses the default token permissions. Restrict it tocontents: read.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ios-e2e.yml around lines 18 - 21, Add an explicit permissions block to the e2e job, setting contents access to read only while preserving the existing runner and timeout configuration.Source: Linters/SAST tools
.github/workflows/android-e2e.yml (3)
146-147: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRemove the debug log level before merge, or track it.
The comment says to drop
simulator-server-log: debugonce the workflow is green. Debug logging on every run enlarges the log output permanently. Do you want me to open an issue to track the removal?🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/android-e2e.yml around lines 146 - 147, Remove the temporary simulator-server-log: debug setting and its associated reminder comment from the Android E2E workflow before merging.
16-19: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd an explicit
permissions:block.The
e2ejob inherits the default token permissions. It only needs to read the repository and download artifacts. Addpermissions: contents: readto the job.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/android-e2e.yml around lines 16 - 19, Update the e2e job configuration to add an explicit permissions block granting only contents: read, while preserving its existing runner and timeout settings.Source: Linters/SAST tools
134-139: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low value
find | head -1can fail the step underpipefail.GitHub runs
shell: bashsteps with-eo pipefail. Whenhead -1exits after the first line,findcan receive SIGPIPE and report a non-zero status, which fails the assignment before the explicit empty check runs. Terminatefinddeterministically instead.♻️ Proposed change
- APK=$(find "$RUNNER_TEMP/apk" -name '*.apk' | head -1) + APK=$(find "$RUNNER_TEMP/apk" -name '*.apk' -print -quit)🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/android-e2e.yml around lines 134 - 139, Update the APK discovery assignment in the Android E2E workflow to avoid the `find | head -1` pipeline under `pipefail`; use a deterministic single-result selection that does not cause `find` to receive SIGPIPE, while preserving the existing empty-APK validation and error handling..github/workflows/ios.yml (1)
33-45: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winAdd
permissions: {}to theconfigjob.The job only writes a literal string to
$GITHUB_OUTPUT. It needs no token scope at all.🔒 Proposed change
config: if: github.repository == 'software-mansion/react-native-gesture-handler' + permissions: {} runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ios.yml around lines 33 - 45, Add an empty permissions declaration to the config job containing the Pin versions step, granting it no GitHub token scopes while preserving its existing output behavior.Source: Linters/SAST tools
.argent/flows/simple-long-press-test.yaml (2)
48-49: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueBoth flows assert absolute log indices because
useIndexedLoggernever resets its counter. Clearing the console removes the rendered entries but leavesmessageCounteruntouched, so the final assertions must count every callback emitted earlier in the flow. One added or removed gesture callback breaks both flows with a misleading failure. Resetting the counter when the console is cleared would let each block assert from index 1.
.argent/flows/simple-long-press-test.yaml#L48-L49: after the counter reset lands, change9. onBeginand10. onFinalizeto1. onBeginand2. onFinalize..argent/flows/simple-tap-test.yaml#L48-L49: apply the same change to the tap flow.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.argent/flows/simple-long-press-test.yaml around lines 48 - 49, Reset useIndexedLogger’s messageCounter when the console is cleared so subsequent logs start at index 1. In .argent/flows/simple-long-press-test.yaml lines 48-49 and .argent/flows/simple-tap-test.yaml lines 48-49, update the final assertions from 9. onBegin/10. onFinalize to 1. onBegin/2. onFinalize.
11-11: 🩺 Stability & Availability | 🔵 Trivial | 💤 Low valueFixed
wait: 1000steps add 4 seconds and can still be too short.The TODO records the intent to use
wait: idle. Fixed sleeps are the usual source of E2E flakiness on slow runners. Do you want me to open an issue to track the replacement?Also applies to: 16-16, 30-30, 41-41
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.argent/flows/simple-long-press-test.yaml at line 11, Replace the fixed 1000ms waits in the simple long-press flow with wait: idle at all four referenced steps, removing the obsolete TODO comments while preserving the existing action sequence..github/actions/argent-server/action.yml (2)
29-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueThe comment contradicts the code.
Lines 29-32 state the server runs "In the foreground". Line 48 starts it with
nohup ... &, which is the background. The relevant distinction is--detachversus a plain background process, not foreground versus background. Reword the comment.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/argent-server/action.yml around lines 29 - 32, Update the comment near the server startup command to remove the claim that the process runs in the foreground. Describe the relevant behavior as using a plain backgrounded process without Docker’s --detach mode, preserving the explanation about timeout handling and server output.
21-23: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPin the Argent CLI version.
npx@swmansion/argentinit --yesresolves the latest published package version on each run. A new release can change CLI behavior and break CI without any repository change. Pin an explicit version.</validation_result>
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/actions/argent-server/action.yml around lines 21 - 23, Pin the Argent CLI invocation in the “Install Argent” step to an explicit `@swmansion/argent` package version instead of resolving the latest release, while preserving the existing init --yes arguments.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.argent/flows/simple-tap-test.yaml:
- Around line 42-44: Update the echo step in the tap test near the long-press
action to describe that the tap count remains at 2, matching the subsequent
assert for “Tap count: 2”; leave the long-press and assertion steps unchanged.
In @.github/workflows/android-e2e.yml:
- Around line 31-32: Disable credential persistence on the actions/checkout@v4
step in both .github/workflows/android-e2e.yml lines 31-32 and
.github/workflows/ios-e2e.yml lines 24-25 by setting persist-credentials to
false under with.
In @.github/workflows/ios-e2e.yml:
- Around line 32-38: Remove the fallback logic in the Xcode selection step
around XCODE_APP. When the requested XCODE_VERSION directory is missing, print
the existing diagnostic information and fail the step immediately with a nonzero
exit status; do not select another installed Xcode or continue the workflow.
In `@apps/common-app/src/new_api/simple/tap/index.tsx`:
- Around line 36-39: Replace the stale count update in onActivate with a
scheduled callback that invokes setCount using a functional increment, so rapid
activations are accumulated correctly. Apply this change in
apps/common-app/src/new_api/simple/tap/index.tsx lines 36-39 and
apps/common-app/src/new_api/simple/longPress/index.tsx lines 41-47.
---
Nitpick comments:
In @.argent/flows/simple-long-press-test.yaml:
- Around line 48-49: Reset useIndexedLogger’s messageCounter when the console is
cleared so subsequent logs start at index 1. In
.argent/flows/simple-long-press-test.yaml lines 48-49 and
.argent/flows/simple-tap-test.yaml lines 48-49, update the final assertions from
9. onBegin/10. onFinalize to 1. onBegin/2. onFinalize.
- Line 11: Replace the fixed 1000ms waits in the simple long-press flow with
wait: idle at all four referenced steps, removing the obsolete TODO comments
while preserving the existing action sequence.
In @.github/actions/argent-server/action.yml:
- Around line 29-32: Update the comment near the server startup command to
remove the claim that the process runs in the foreground. Describe the relevant
behavior as using a plain backgrounded process without Docker’s --detach mode,
preserving the explanation about timeout handling and server output.
- Around line 21-23: Pin the Argent CLI invocation in the “Install Argent” step
to an explicit `@swmansion/argent` package version instead of resolving the latest
release, while preserving the existing init --yes arguments.
In @.github/workflows/android-e2e.yml:
- Around line 146-147: Remove the temporary simulator-server-log: debug setting
and its associated reminder comment from the Android E2E workflow before
merging.
- Around line 16-19: Update the e2e job configuration to add an explicit
permissions block granting only contents: read, while preserving its existing
runner and timeout settings.
- Around line 134-139: Update the APK discovery assignment in the Android E2E
workflow to avoid the `find | head -1` pipeline under `pipefail`; use a
deterministic single-result selection that does not cause `find` to receive
SIGPIPE, while preserving the existing empty-APK validation and error handling.
In @.github/workflows/ios-e2e.yml:
- Around line 18-21: Add an explicit permissions block to the e2e job, setting
contents access to read only while preserving the existing runner and timeout
configuration.
In @.github/workflows/ios.yml:
- Around line 33-45: Add an empty permissions declaration to the config job
containing the Pin versions step, granting it no GitHub token scopes while
preserving its existing output behavior.
In `@apps/common-app/src/common.tsx`:
- Around line 144-160: Update useIndexedLogger to wrap both logMessage and
logMessageWorklet in useCallback with appropriate dependencies, preserving the
existing counter and scheduling behavior while keeping the returned worklet
identity stable across renders.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: a360b7af-7694-42a6-9338-8ff0f937c372
📒 Files selected for processing (12)
.argent/flows/simple-long-press-test.yaml.argent/flows/simple-tap-test.yaml.github/actions/argent-server/action.yml.github/workflows/android-e2e.yml.github/workflows/android.yml.github/workflows/ios-e2e.yml.github/workflows/ios.ymlapps/common-app/src/common.tsxapps/common-app/src/console/ConsoleModal.tsxapps/common-app/src/new_api/index.tsxapps/common-app/src/new_api/simple/longPress/index.tsxapps/common-app/src/new_api/simple/tap/index.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ios.yml (1)
33-45: 🔒 Security & Privacy | 🟠 Major | ⚡ Quick winRemove inherited token permissions from
config.This job only writes a local step output. It does not require GitHub API access. Set
permissions: {}on the job to prevent repository defaults from granting unused token permissions.Proposed fix
config: if: github.repository == 'software-mansion/react-native-gesture-handler' + permissions: {} runs-on: ubuntu-latest🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ios.yml around lines 33 - 45, Add a job-level permissions: {} setting to the config job, alongside its existing if, runs-on, and outputs configuration, so the Pin versions step retains local output behavior without inheriting GitHub token permissions.Source: Linters/SAST tools
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In @.github/workflows/ios.yml:
- Around line 33-45: Add a job-level permissions: {} setting to the config job,
alongside its existing if, runs-on, and outputs configuration, so the Pin
versions step retains local output behavior without inheriting GitHub token
permissions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro Plus
Run ID: c87e10e2-ef6e-4cbb-be51-4f248c1fa75d
📒 Files selected for processing (4)
.argent/flows/simple-long-press-test.yaml.argent/flows/simple-tap-test.yaml.github/workflows/ios-e2e.yml.github/workflows/ios.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .argent/flows/simple-tap-test.yaml
4dc7a32 to
94ed13e
Compare
94ed13e to
31bf731
Compare
Description
Test plan
Status checks