[E2E] Mobile test with firebase test lab#4602
Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis pull request migrates Patrol mobile integration tests from a local Android emulator to Firebase Test Lab. Jenkins now selects the Possibly related PRs
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![codescene-delta-analysis[bot]](https://avatars.githubusercontent.com/in/53921?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
scripts/patrol-integration-test-with-docker.sh (1)
18-20:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftFirebase Test Lab cannot reach the backend URLs hardcoded to
10.0.2.2.
10.0.2.2only points to the host machine from a local QEMU emulator. Once the suite is launched withgcloud firebase test android run, the app/instrumentation execute on a remote Firebase device, soBASIC_AUTH_URLandRESET_SERVER_URLboth target an unreachable host. That makes the migrated E2E path fail before it can talk to the backend. Route these URLs to an endpoint reachable from Firebase Test Lab instead of the emulator loopback.Also applies to: 44-50, 63-82
🤖 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 `@scripts/patrol-integration-test-with-docker.sh` around lines 18 - 20, The script currently hardcodes emulator-only loopback addresses into jmap.properties (the sed replacements for url.prefix and websocket.url.prefix) which Firebase Test Lab devices cannot reach; update the script to use a reachable backend endpoint (e.g., a public hostname or an environment variable like BACKEND_HOST/CLOUD_BACKEND_URL) instead of 10.0.2.2, and apply the same change to the other replacements that set BASIC_AUTH_URL and RESET_SERVER_URL (and the blocks noted around lines 44-50 and 63-82) so all URLs injected by the script are configurable and resolve from remote Firebase devices.
🤖 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 `@JenkinsfileMobile`:
- Around line 27-29: The Jenkins-only credential wiring around withCredentials
only affects the Jenkins call to scripts/patrol-integration-test-with-docker.sh,
but the same script is also invoked by
.github/workflows/patrol-integration-test.yaml without
GOOGLE_APPLICATION_CREDENTIALS or FIREBASE_PROJECT_ID; fix by either (A)
updating the GitHub Actions workflow to provide the same env/secret (export
GOOGLE_APPLICATION_CREDENTIALS and FIREBASE_PROJECT_ID or use actions/checkout +
gcloud auth) when it calls scripts/patrol-integration-test-with-docker.sh, or
(B) split the Jenkins-specific gcloud/firebase steps out of
scripts/patrol-integration-test-with-docker.sh into a new script (e.g.,
patrol-integration-test-with-docker-jenkins.sh), update JenkinsfileMobile to
call the new Jenkins-only script inside withCredentials, and leave the original
script unchanged for GitHub Actions.
---
Outside diff comments:
In `@scripts/patrol-integration-test-with-docker.sh`:
- Around line 18-20: The script currently hardcodes emulator-only loopback
addresses into jmap.properties (the sed replacements for url.prefix and
websocket.url.prefix) which Firebase Test Lab devices cannot reach; update the
script to use a reachable backend endpoint (e.g., a public hostname or an
environment variable like BACKEND_HOST/CLOUD_BACKEND_URL) instead of 10.0.2.2,
and apply the same change to the other replacements that set BASIC_AUTH_URL and
RESET_SERVER_URL (and the blocks noted around lines 44-50 and 63-82) so all URLs
injected by the script are configurable and resolve from remote Firebase
devices.
🪄 Autofix (Beta)
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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 35b20018-59b5-4868-80c2-ea12017bebc3
📒 Files selected for processing (2)
JenkinsfileMobilescripts/patrol-integration-test-with-docker.sh
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
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)
JenkinsfileMobile (1)
48-50:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winUpdate stale failure message reference.
The failure message references
emulator.log, but since the pipeline no longer runs a local Android emulator, this file won't exist. Update the message to reflect Firebase Test Lab debugging resources instead.failure { - echo 'Pipeline failed. Review the console log and emulator.log for details.' + echo 'Pipeline failed. Review the console log for details.' }🤖 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 `@JenkinsfileMobile` around lines 48 - 50, The failure block's echo message still references a now-nonexistent emulator.log; update the failure { echo '...' } message in the JenkinsfileMobile to mention Firebase Test Lab debugging resources and artifact locations instead of emulator.log (e.g., "Pipeline failed. Review the console log and Firebase Test Lab artifacts and logs for details."). Ensure the new message clearly directs users to console logs and Firebase Test Lab artifacts/output for debugging.
🧹 Nitpick comments (1)
JenkinsfileMobile (1)
13-14: 💤 Low valueConsider removing unused emulator environment variables.
API_LEVELandAVD_NAMEwere used for local Android emulator configuration. With the migration to Firebase Test Lab, the emulator stage has been removed and these variables are no longer referenced. Removing them would reduce confusion about the pipeline's actual behavior.environment { FLUTTER_VERSION = '3.38.9' - API_LEVEL = '30' - AVD_NAME = 'pixel_6' FIREBASE_PROJECT_ID = 'twake-195010' }🤖 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 `@JenkinsfileMobile` around lines 13 - 14, Remove the now-unused emulator environment variables API_LEVEL and AVD_NAME from the JenkinsfileMobile environment/block since the emulator stage was removed in favor of Firebase Test Lab; locate the API_LEVEL and AVD_NAME declarations and delete them (and any no-longer-used comments or references) to avoid confusion and ensure the pipeline only contains variables actually consumed by stages.
🤖 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 `@JenkinsfileMobile`:
- Around line 48-50: The failure block's echo message still references a
now-nonexistent emulator.log; update the failure { echo '...' } message in the
JenkinsfileMobile to mention Firebase Test Lab debugging resources and artifact
locations instead of emulator.log (e.g., "Pipeline failed. Review the console
log and Firebase Test Lab artifacts and logs for details."). Ensure the new
message clearly directs users to console logs and Firebase Test Lab
artifacts/output for debugging.
---
Nitpick comments:
In `@JenkinsfileMobile`:
- Around line 13-14: Remove the now-unused emulator environment variables
API_LEVEL and AVD_NAME from the JenkinsfileMobile environment/block since the
emulator stage was removed in favor of Firebase Test Lab; locate the API_LEVEL
and AVD_NAME declarations and delete them (and any no-longer-used comments or
references) to avoid confusion and ensure the pipeline only contains variables
actually consumed by stages.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
JenkinsfileMobile (1)
26-28:⚠️ Potential issue | 🔴 Critical | 🏗️ Heavy liftThe Firebase credential wiring only covers Jenkins, but the script is also called by GitHub Actions.
The
withCredentialsblock providesGOOGLE_APPLICATION_CREDENTIALSand line 13 providesFIREBASE_PROJECT_ID, butscripts/patrol-integration-test-with-docker.shis also invoked by.github/workflows/patrol-integration-test.yaml(as noted in the past review comment). That workflow path will fail when the script tries to authenticate with gcloud (line 72 of the script) because those environment variables won't be set.Fix by either:
- (A) Update
.github/workflows/patrol-integration-test.yamlto provide the same Firebase credentials and project ID before calling the script, or- (B) Split the Firebase Test Lab logic into a separate Jenkins-only script (e.g.,
patrol-integration-test-with-docker-jenkins.sh), update this Jenkinsfile to call the new script, and leave the original script for GitHub Actions.🤖 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 `@JenkinsfileMobile` around lines 26 - 28, The Jenkinsfile currently wraps the test invocation in withCredentials to set GOOGLE_APPLICATION_CREDENTIALS and FIREBASE_PROJECT_ID, but scripts/patrol-integration-test-with-docker.sh is also invoked by .github/workflows/patrol-integration-test.yaml so the GitHub Actions run will fail when the script calls gcloud (around line 72); fix by either (A) updating .github/workflows/patrol-integration-test.yaml to export the same FIREBASE_PROJECT_ID and provide GOOGLE_APPLICATION_CREDENTIALS (via actions/upload-artifact or secrets and appropriate action) before calling scripts/patrol-integration-test-with-docker.sh, or (B) split the Test Lab auth steps out into a Jenkins-only script (e.g., scripts/patrol-integration-test-with-docker-jenkins.sh), update the Jenkinsfile to call that new script inside withCredentials, and leave scripts/patrol-integration-test-with-docker.sh unchanged for GitHub Actions.
🤖 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.
Duplicate comments:
In `@JenkinsfileMobile`:
- Around line 26-28: The Jenkinsfile currently wraps the test invocation in
withCredentials to set GOOGLE_APPLICATION_CREDENTIALS and FIREBASE_PROJECT_ID,
but scripts/patrol-integration-test-with-docker.sh is also invoked by
.github/workflows/patrol-integration-test.yaml so the GitHub Actions run will
fail when the script calls gcloud (around line 72); fix by either (A) updating
.github/workflows/patrol-integration-test.yaml to export the same
FIREBASE_PROJECT_ID and provide GOOGLE_APPLICATION_CREDENTIALS (via
actions/upload-artifact or secrets and appropriate action) before calling
scripts/patrol-integration-test-with-docker.sh, or (B) split the Test Lab auth
steps out into a Jenkins-only script (e.g.,
scripts/patrol-integration-test-with-docker-jenkins.sh), update the Jenkinsfile
to call that new script inside withCredentials, and leave
scripts/patrol-integration-test-with-docker.sh unchanged for GitHub Actions.
There was a problem hiding this comment.
Our agent can fix these. Install it.
No application code in the PR — skipped Code Health checks.
Quality Gate Profile: The Bare Minimum
Install CodeScene MCP: safeguard and uplift AI-generated code. Catch issues early with our IDE extension and CLI tool.
1 participant
Summary by CodeRabbit