Harden Android test CI inputs - #8945
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: defaults 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:
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 |
There was a problem hiding this comment.
🟢 Ready to approve
The changes are narrowly scoped, add appropriate validation/guards, and include a targeted regression test for the disposal race.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
This PR hardens the Android Firebase Test Lab CI path by validating untrusted workflow inputs before forwarding them into Make/Gradle, and fixes a Riverpod teardown race where AvailableServersNotifier could attempt a delayed refresh after disposal (now covered by a regression test).
Changes:
- Add bash-side validation for
inputs.test_targetin the Firebase Test Lab workflow before invoking Make. - Quote
-Ptargetusage in Makefile Android integration targets to avoid argument splitting. - Prevent post-disposal refresh work in
AvailableServersNotifier, with a regression test covering disposal during the first fetch.
File summaries
| File | Description |
|---|---|
test/features/vpn/provider/available_servers_notifier_test.dart |
Adds a regression test to ensure probe-settle refresh completes cleanly when disposed mid-fetch. |
Makefile |
Quotes -Ptarget when calling Gradle for Android integration test targets. |
lib/features/vpn/provider/available_servers_notifier.dart |
Adds ref.mounted guards to stop refresh work after provider disposal. |
.github/workflows/firebase-test-lab.yml |
Validates and safely forwards ANDROID_INTEGRATION_TARGET from workflow inputs. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
…8933) * Set up Android testing. * Update the Firebase workflow. * Update firebase-test-lab.yml * Set up Gradle. * Remove comment. * Fail fast on GCP auth and use dedicated FTL results bucket Authenticate to Google Cloud and verify Firebase Test Lab access right after checkout so credential problems fail in seconds instead of after the ~30 minute APK build. Upload results to our own bucket (lantern-android-ftl-results) since the default Test Lab bucket only grants access to project editors. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * added report issue smoke test support * trim comments * added support for setting screen smoke test * update target * Added plant smoke test. * added logs and update test * update device list * update firebase workflow * code review updates * Smoke test workflow changes * Harden Android test CI inputs (#8945) * code review updates --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com> Co-authored-by: atavism <atavism@users.noreply.github.com>
Safely validate and pass Firebase Test Lab entrypoints through the workflow and Make targets. Also prevent delayed available-server refreshes after Riverpod disposal, with regression coverage for the teardown race.