Allow Codecov to fail in CI without failing the pipeline. #2477
Merged
AlexSkrypnyk merged 2 commits intomainfrom Apr 27, 2026
Merged
Allow Codecov to fail in CI without failing the pipeline. #2477AlexSkrypnyk merged 2 commits intomainfrom
AlexSkrypnyk merged 2 commits intomainfrom
Conversation
WalkthroughThe pull request updates the demo database source URL to a newer release, modifies GitHub Actions workflows to make Codecov upload failures non-fatal, refactors maintenance documentation to centralize test asset updates, adds a Redis cache flush step to a test, and introduces a new automation script for rebuilding and publishing test assets. Changes
Suggested Labels
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly Related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
![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
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.vortex/docs/content/contributing/maintenance/template.mdx:
- Around line 269-275: Update the "destination-images" caveat: it currently
claims the image drevops/vortex-dev-mariadb-drupal-data-demo-11.x:latest must
exist locally, but the script .vortex/tests/update-test-assets (the pull logic
around the block that fetches
drevops/vortex-dev-mariadb-drupal-data-demo-11.x:<tag>) actually pulls the
specified <tag> from Docker Hub first, so change the wording to state that the
source tag must be pushed to Docker Hub (e.g., "Ensure drevops/...-11.x:<tag> is
already pushed to Docker Hub; the script pulls that tag from the registry")
rather than requiring a local :latest image.
In @.vortex/tests/update-test-assets:
- Around line 275-276: The build step currently swallows all failures by using
"run('ahoy fe || true', $cwd)", which masks real build regressions; change it to
only skip the front-end build when the real precondition (theme not configured)
is true and otherwise run the build normally. Replace the unconditional
"run('ahoy fe || true', $cwd)" with a guarded check around the step/ run call
(e.g., query the theme configuration flag or existence of theme assets) and call
"run('ahoy fe', $cwd)" only when the theme is configured, letting the step fail
on non-zero exit for real build errors; ensure you update the same step/ run
invocation in this file.
- Around line 407-412: When handling the --tag branch, reject an explicitly
empty string value the same as --tag= by validating $tag after assignment; after
$tag = $args[$i + 1]; add a check if ($tag === '') and call usage_error($script,
$valid_modes, "Option '--tag' requires a value."); so that variables $arg,
$args, $i, $tag and the usage_error(...) call enforce a fast fail for empty tag
values.
- Around line 382-389: The seed_image function currently downloads and executes
an unpinned seed.sh from SEED_URL which risks running arbitrary upstream
changes; update this to use a versioned/checked asset (or vendor the script) and
verify its checksum before making it executable and running it: change SEED_URL
to a specific release URL or add a bundled seed.sh, add a constant expected
checksum (e.g., SEED_SHA256) and after downloading compute and compare the
file's sha256, abort with an error if it doesn't match, only then run chmod +x
and execute the script in seed_image; ensure the verification and failure paths
are implemented inside seed_image (or a helper) so run(sprintf('curl ...')) is
replaced by a secure fetch+verify flow.
🪄 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: Repository UI
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 66ba7eba-15af-48ec-9c25-d85d25aab30d
⛔ Files ignored due to path filters (21)
.vortex/installer/tests/Fixtures/handler_process/_baseline/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/code_coverage_provider_codecov/.github/workflows/build-test-deploy.ymlis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/db_download_source_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/db_download_source_container_registry/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/db_download_source_ftp/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/db_download_source_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/db_download_source_s3/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___acquia/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/hosting_project_name___lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_disabled_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_download_source_url/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_circleci/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/migration_enabled_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_database_lagoon/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/provision_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/starter_drupal_cms_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/starter_drupal_profile/.envis excluded by!.vortex/installer/tests/Fixtures/**.vortex/installer/tests/Fixtures/handler_process/theme_custom_non_vortex/.envis excluded by!.vortex/installer/tests/Fixtures/**
📒 Files selected for processing (8)
.env.github/workflows/build-test-deploy.yml.github/workflows/vortex-test-common.yml.github/workflows/vortex-test-docs.yml.github/workflows/vortex-test-installer.yml.vortex/docs/content/contributing/maintenance/template.mdx.vortex/tests/phpunit/Functional/AhoyWorkflowTest.php.vortex/tests/update-test-assets
Comment on lines
+269
to
+275
| ### Caveats | ||
|
|
||
| ### Updating *test* database *container image* | ||
| - Demo modes are **destructive** against the current repository: containers | ||
| are reset and `.data/db.sql` is removed before the rebuild. | ||
| - Test modes wipe and recreate `/tmp/star-wars` on each run. | ||
| - `destination-images` expects `drevops/vortex-dev-mariadb-drupal-data-demo-11.x:latest` | ||
| to already exist locally - run `demo-image` (or `all`) first. |
There was a problem hiding this comment.
The destination-images caveat no longer matches the script.
The implementation pulls drevops/vortex-dev-mariadb-drupal-data-demo-11.x:<tag> first in .vortex/tests/update-test-assets, Lines 139-146, so the prerequisite is “that source tag is already pushed to Docker Hub”, not “:latest exists locally”. As written, destination-images --tag rc1 is documented incorrectly.
Suggested fix
-- `destination-images` expects `drevops/vortex-dev-mariadb-drupal-data-demo-11.x:latest`
- to already exist locally - run `demo-image` (or `all`) first.
+- `destination-images` pulls `drevops/vortex-dev-mariadb-drupal-data-demo-11.x:<tag>`
+ first, so that source tag must already be published to Docker Hub.
+ Run `demo-image --tag <tag>` (or `all --tag <tag>`) before using it standalone.📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| ### Caveats | |
| ### Updating *test* database *container image* | |
| - Demo modes are **destructive** against the current repository: containers | |
| are reset and `.data/db.sql` is removed before the rebuild. | |
| - Test modes wipe and recreate `/tmp/star-wars` on each run. | |
| - `destination-images` expects `drevops/vortex-dev-mariadb-drupal-data-demo-11.x:latest` | |
| to already exist locally - run `demo-image` (or `all`) first. | |
| ### Caveats | |
| - Demo modes are **destructive** against the current repository: containers | |
| are reset and `.data/db.sql` is removed before the rebuild. | |
| - Test modes wipe and recreate `/tmp/star-wars` on each run. | |
| - `destination-images` pulls `drevops/vortex-dev-mariadb-drupal-data-demo-11.x:<tag>` | |
| first, so that source tag must already be published to Docker Hub. | |
| Run `demo-image --tag <tag>` (or `all --tag <tag>`) before using it standalone. |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/docs/content/contributing/maintenance/template.mdx around lines 269
- 275, Update the "destination-images" caveat: it currently claims the image
drevops/vortex-dev-mariadb-drupal-data-demo-11.x:latest must exist locally, but
the script .vortex/tests/update-test-assets (the pull logic around the block
that fetches drevops/vortex-dev-mariadb-drupal-data-demo-11.x:<tag>) actually
pulls the specified <tag> from Docker Hub first, so change the wording to state
that the source tag must be pushed to Docker Hub (e.g., "Ensure
drevops/...-11.x:<tag> is already pushed to Docker Hub; the script pulls that
tag from the registry") rather than requiring a local :latest image.
Comment on lines
+275
to
+276
| step('Build front-end assets (skipped if theme not configured)'); | ||
| run('ahoy fe || true', $cwd); |
There was a problem hiding this comment.
Don't mask real front-end build failures with || true.
This treats “theme not configured” and an actual asset-build regression the same way, so the script can continue publishing refreshed dumps/images from a broken build. Guard the step with the real precondition instead of swallowing every non-zero exit.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/tests/update-test-assets around lines 275 - 276, The build step
currently swallows all failures by using "run('ahoy fe || true', $cwd)", which
masks real build regressions; change it to only skip the front-end build when
the real precondition (theme not configured) is true and otherwise run the build
normally. Replace the unconditional "run('ahoy fe || true', $cwd)" with a
guarded check around the step/ run call (e.g., query the theme configuration
flag or existence of theme assets) and call "run('ahoy fe', $cwd)" only when the
theme is configured, letting the step fail on non-zero exit for real build
errors; ensure you update the same step/ run invocation in this file.
Comment on lines
+382
to
+389
| function seed_image(string $cwd, string $filename, string $image): void { | ||
| step('Download seed.sh'); | ||
| run(sprintf('curl -fsSL -o seed.sh %s', escapeshellarg(SEED_URL)), $cwd); | ||
| run('chmod +x seed.sh', $cwd); | ||
|
|
||
| step(sprintf('Seed and push image to %s', $image)); | ||
| run(sprintf('./seed.sh .data/%s %s', escapeshellarg($filename), escapeshellarg($image)), $cwd); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's find and examine the file in question
find . -name "update-test-assets" -type f 2>/dev/nullRepository: drevops/vortex
Length of output: 93
🏁 Script executed:
# Search for SEED_URL definition
rg "SEED_URL" -B 2 -A 2Repository: drevops/vortex
Length of output: 40
🏁 Script executed:
# Check the full context of the file around lines 382-389
if [ -f ".vortex/tests/update-test-assets" ]; then
wc -l ".vortex/tests/update-test-assets"
fiRepository: drevops/vortex
Length of output: 95
🏁 Script executed:
# Read the file to find SEED_URL definition and context around lines 382-389
cat -n ".vortex/tests/update-test-assets" | grep -A 5 -B 5 "SEED_URL"Repository: drevops/vortex
Length of output: 1210
🏁 Script executed:
# Read the specific section around lines 382-389
sed -n '370,400p' ".vortex/tests/update-test-assets"Repository: drevops/vortex
Length of output: 1019
🏁 Script executed:
# Check if there's any checksum or signature verification in the file
rg -i "checksum|sha|hash|signature|verify|gpg" ".vortex/tests/update-test-assets"Repository: drevops/vortex
Length of output: 343
🏁 Script executed:
# Look for all seed.sh references in the file
grep -n "seed\.sh" ".vortex/tests/update-test-assets"Repository: drevops/vortex
Length of output: 412
🏁 Script executed:
# Check if seed.sh is vendored elsewhere in the repo
find . -name "seed.sh" -type f 2>/dev/nullRepository: drevops/vortex
Length of output: 40
🏁 Script executed:
# Check the entire context of the seed_image function usage to understand the risk
rg "seed_image" -B 3 -A 3 ".vortex/tests/update-test-assets"Repository: drevops/vortex
Length of output: 1422
Pin or verify seed.sh before executing it.
Fetching from /releases/latest/ makes this workflow non-reproducible and executes whatever the newest upstream release serves at runtime. A bad upstream release changes behavior here without any code change in this repo. Prefer a versioned asset plus checksum verification, or vendor the script.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/tests/update-test-assets around lines 382 - 389, The seed_image
function currently downloads and executes an unpinned seed.sh from SEED_URL
which risks running arbitrary upstream changes; update this to use a
versioned/checked asset (or vendor the script) and verify its checksum before
making it executable and running it: change SEED_URL to a specific release URL
or add a bundled seed.sh, add a constant expected checksum (e.g., SEED_SHA256)
and after downloading compute and compare the file's sha256, abort with an error
if it doesn't match, only then run chmod +x and execute the script in
seed_image; ensure the verification and failure paths are implemented inside
seed_image (or a helper) so run(sprintf('curl ...')) is replaced by a secure
fetch+verify flow.
Comment on lines
+407
to
+412
| if ($arg === '--tag') { | ||
| if (!isset($args[$i + 1])) { | ||
| usage_error($script, $valid_modes, "Option '--tag' requires a value."); | ||
| } | ||
| $tag = $args[$i + 1]; | ||
| $i += 2; |
There was a problem hiding this comment.
Reject --tag '' the same way as --tag=.
--tag '' passes parsing today, then fails much later when Docker receives an invalid image reference like repo:. Fail fast here with the same usage error you already use for an empty --tag= value.
Suggested fix
if ($arg === '--tag') {
if (!isset($args[$i + 1])) {
usage_error($script, $valid_modes, "Option '--tag' requires a value.");
}
$tag = $args[$i + 1];
+ if ($tag === '') {
+ usage_error($script, $valid_modes, "Option '--tag' requires a value.");
+ }
$i += 2;
continue;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ($arg === '--tag') { | |
| if (!isset($args[$i + 1])) { | |
| usage_error($script, $valid_modes, "Option '--tag' requires a value."); | |
| } | |
| $tag = $args[$i + 1]; | |
| $i += 2; | |
| if ($arg === '--tag') { | |
| if (!isset($args[$i + 1])) { | |
| usage_error($script, $valid_modes, "Option '--tag' requires a value."); | |
| } | |
| $tag = $args[$i + 1]; | |
| if ($tag === '') { | |
| usage_error($script, $valid_modes, "Option '--tag' requires a value."); | |
| } | |
| $i += 2; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.vortex/tests/update-test-assets around lines 407 - 412, When handling the
--tag branch, reject an explicitly empty string value the same as --tag= by
validating $tag after assignment; after $tag = $args[$i + 1]; add a check if
($tag === '') and call usage_error($script, $valid_modes, "Option '--tag'
requires a value."); so that variables $arg, $args, $i, $tag and the
usage_error(...) call enforce a fast fail for empty tag values.
|
Code coverage (threshold: 90%) Per-class coverage |
This comment has been minimized.
This comment has been minimized.
2 similar comments
This comment has been minimized.
This comment has been minimized.
Member
Author
|
Code coverage (threshold: 90%) Per-class coverage |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #2477 +/- ##
==========================================
- Coverage 79.97% 79.51% -0.46%
==========================================
Files 129 122 -7
Lines 6895 6736 -159
Branches 47 3 -44
==========================================
- Hits 5514 5356 -158
+ Misses 1381 1380 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
Release Notes
Chores
Documentation
Tests