Build the E2E image once and reuse it across CI shards#27050
Conversation
ref https://github.com/TryGhost/Ghost/actions/workflows/ci.yml Built the public app bundles once and moved the E2E overlay image build out of each shard so CI stops repeating the same work eight times.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughCI now builds a dedicated E2E Docker image and separates E2E app bundle creation from image assembly. New jobs Possibly related issues
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 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 |
…-once # Conflicts: # .github/workflows/ci.yml
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
🧹 Nitpick comments (1)
e2e/scripts/prepare-ci-e2e-job.sh (1)
16-22: Consider validatingGHOST_E2E_IMAGEwhen skipping build.When
SKIP_IMAGE_BUILD=true, the script doesn't validate thatGHOST_E2E_IMAGEis actually set. While the CI workflow always provides it, manual invocations could silently fall back toghost-e2e:localwhich may not exist.🛡️ Optional: Add validation for manual usage
echo "Preparing CI E2E job" echo "E2E image: ${GHOST_E2E_IMAGE:-ghost-e2e:local}" echo "Skip image build: ${SKIP_IMAGE_BUILD}" if [[ "$SKIP_IMAGE_BUILD" != "true" ]]; then echo "Base image: ${GHOST_E2E_BASE_IMAGE}" +else + if [[ -z "${GHOST_E2E_IMAGE:-}" ]]; then + echo "Warning: GHOST_E2E_IMAGE not set, using default ghost-e2e:local" >&2 + fi fi🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@e2e/scripts/prepare-ci-e2e-job.sh` around lines 16 - 22, When SKIP_IMAGE_BUILD is true the script must validate the GHOST_E2E_IMAGE value; add a guard in prepare-ci-e2e-job.sh after the SKIP_IMAGE_BUILD check that inspects the GHOST_E2E_IMAGE environment variable (and ensure it is not empty or the fallback literal "ghost-e2e:local") and if invalid print an error and exit non‑zero. Update the branch that currently prints "Skip image build" to perform this validation so manual runs fail fast instead of silently using the local default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@e2e/scripts/prepare-ci-e2e-job.sh`:
- Around line 16-22: When SKIP_IMAGE_BUILD is true the script must validate the
GHOST_E2E_IMAGE value; add a guard in prepare-ci-e2e-job.sh after the
SKIP_IMAGE_BUILD check that inspects the GHOST_E2E_IMAGE environment variable
(and ensure it is not empty or the fallback literal "ghost-e2e:local") and if
invalid print an error and exit non‑zero. Update the branch that currently
prints "Skip image build" to perform this validation so manual runs fail fast
instead of silently using the local default.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f64c678c-8385-4428-830c-499e69279b8d
📒 Files selected for processing (2)
.github/workflows/ci.ymle2e/scripts/prepare-ci-e2e-job.sh
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
1084-1223: Consider mirroring the main-branch Slack hook on the new E2E build jobs.If either new build job fails,
job_e2e_testsnever starts, so the existing E2E Slack notification at Lines 1314-1319 won’t fire. Adding the usualslack-buildstep here would keep early E2E-pipeline failures visible.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1084 - 1223, Add the same Slack notification step used for main-branch E2E builds to both job_build_e2e_public_apps and job_build_e2e_image so failures are reported immediately; specifically, add a step named "slack-build" (matching the existing slack step id/name) that runs on failure (use if: failure() or the project's existing conditional), uses the existing slack action (the repo's .github/actions/slack-build or the same uses: and with: signature as the other E2E Slack step), and wires the same inputs/secrets/labels (e.g., channel, status, build metadata) so the notification format matches the existing E2E Slack hook.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 1124-1228: The job_required_tests dependency list must include the
new E2E build jobs so failures block the aggregate gate: update the
job_required_tests "needs" to add job_build_e2e_image and
job_build_e2e_public_apps (these are the new upstream jobs) alongside existing
entries; ensure job_e2e_tests remains dependent on job_build_e2e_image but also
list the two build jobs in job_required_tests.needs so their failures cause
job_required_tests to fail rather than skip.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 1084-1223: Add the same Slack notification step used for
main-branch E2E builds to both job_build_e2e_public_apps and job_build_e2e_image
so failures are reported immediately; specifically, add a step named
"slack-build" (matching the existing slack step id/name) that runs on failure
(use if: failure() or the project's existing conditional), uses the existing
slack action (the repo's .github/actions/slack-build or the same uses: and with:
signature as the other E2E Slack step), and wires the same inputs/secrets/labels
(e.g., channel, status, build metadata) so the notification format matches the
existing E2E Slack hook.
🪄 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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 607bd36f-0d73-4eef-a8e4-9c5c2ed57f9d
📒 Files selected for processing (1)
.github/workflows/ci.yml
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #27050 +/- ##
==========================================
- Coverage 73.26% 73.26% -0.01%
==========================================
Files 1530 1530
Lines 121767 121768 +1
Branches 14710 14710
==========================================
- Hits 89218 89215 -3
- Misses 31555 31557 +2
- Partials 994 996 +2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
1 participant
Summary
Why
Each shard currently runs
yarn workspace @tryghost/e2e build:appsandbuild:dockerduringprepare-ci-e2e-job.sh, which duplicates the same work eight times.Validation
.github/workflows/ci.ymlwith Ruby YAMLbash -n e2e/scripts/prepare-ci-e2e-job.shNotes