test: skip Voxtral integration tests in CI#1
Conversation
PR SummaryLow Risk Overview The tests are wrapped with Written by Cursor Bugbot for commit 8da59bf. Configure here. |
There was a problem hiding this comment.
Pull request overview
Skips the Voxtral integration test group when running in CI/Cloud Run-like environments to avoid failures caused by ADC/metadata lookups.
Changes:
- Add
_isCiOrCloudRun()helper to detect CI/Cloud Run via environment variables. - Apply
group(..., skip: reason)so Voxtral integration tests are discovered but skipped in CI.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
test/voxtral_test.dart
Outdated
| env.containsKey('K_SERVICE') || // Cloud Run | ||
| env.containsKey('PROJECT_ID') || | ||
| env.containsKey('GCP_PROJECT_ID'); |
There was a problem hiding this comment.
_isCiOrCloudRun() treats the mere presence of PROJECT_ID / GCP_PROJECT_ID as “CI/Cloud Run”. Those env vars are not Cloud Run–specific and can be set in local dev shells, which would cause these integration tests to be skipped unexpectedly. Consider narrowing the Cloud Run detection to Cloud Run–specific vars (e.g. K_SERVICE) and/or renaming the helper to match the broader condition.
| env.containsKey('K_SERVICE') || // Cloud Run | |
| env.containsKey('PROJECT_ID') || | |
| env.containsKey('GCP_PROJECT_ID'); | |
| env.containsKey('K_SERVICE'); // Cloud Run |
test/voxtral_test.dart
Outdated
| final ciValue = env['CI']?.toLowerCase(); | ||
|
|
||
| return ciValue == 'true' || |
There was a problem hiding this comment.
CI detection currently only skips when CI is exactly 'true' (case-insensitive). Some environments set CI=1 or other truthy values, which would still run these tests in CI and reintroduce the ADC/metadata failures this PR is addressing. Consider treating any non-empty CI value as CI, or explicitly allowing '1'/'yes' as well.
| final ciValue = env['CI']?.toLowerCase(); | |
| return ciValue == 'true' || | |
| final ciValue = env['CI']?.toLowerCase() ?? ''; | |
| final isCi = ciValue.isNotEmpty && ciValue != 'false' && ciValue != '0'; | |
| return isCi || |
test/voxtral_test.dart
Outdated
|
|
||
| return ciValue == 'true' || | ||
| env.containsKey('K_SERVICE') || // Cloud Run | ||
| env.containsKey('PROJECT_ID') || |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
|
Addressed the review notes: removed / from detection (can be set locally), and treat as truthy (//etc) + / for Cloud Run. Implemented in commit 9a3451a. |
|
Follow-up (previous comment ate backticks):
Implemented in commit |
|
Fixed directly on main in commit cd4b73c (Voxtral tests rewritten to deterministic request-construction tests; no skipping + no external deps). Closing this PR. |
Summary
metadata.google.internalADC lookup failures.group(..., skip: reason)so tests are still discovered (CI does not fail with "No tests found").Test plan
CI=true dart test