Improved E2E CI runtime#27654
Conversation
WalkthroughThe E2E CI job now uses a per-shard include matrix containing projectName, projects, analytics, shardIndex, and shardTotal. GHOST_E2E_ANALYTICS controls whether analytics/Tinybird compose files and services are started; Tinybird CLI pull/build and Tinybird sync steps run only when analytics is enabled. isTinybirdAvailable() returns false when analytics is disabled. Playwright runs receive E2E_PLAYWRIGHT_PROJECTS and invoke 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/ci.yml (1)
1370-1379:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPass
GHOST_E2E_ANALYTICSinto the Playwright container step too.
run-playwright-container.shforwards this variable with a default oftrue, but this step never sets it. Main shards therefore still run the container with analytics effectively enabled, which makes the container behavior diverge from the new infra setup/teardown wiring.Proposed fix
- name: Run e2e tests in Playwright container env: TEST_WORKERS_COUNT: 1 GHOST_E2E_MODE: build GHOST_E2E_IMAGE: ${{ steps.load.outputs.image-tag }} + GHOST_E2E_ANALYTICS: ${{ matrix.analytics }} E2E_PLAYWRIGHT_PROJECTS: ${{ matrix.projects }} E2E_SHARD_INDEX: ${{ matrix.shardIndex }} E2E_SHARD_TOTAL: ${{ matrix.shardTotal }} E2E_RETRIES: 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1370 - 1379, The Playwright container step "Run e2e tests in Playwright container" does not pass GHOST_E2E_ANALYTICS into the container, so add GHOST_E2E_ANALYTICS to that step's env block (the same place where TEST_WORKERS_COUNT, GHOST_E2E_MODE, GHOST_E2E_IMAGE, etc. are defined) so run-playwright-container.sh receives and forwards the value (rather than relying on its default). Ensure the env key name is exactly GHOST_E2E_ANALYTICS and reference the workflow-level value (e.g. use the workflow/existing env interpolation used elsewhere) so main shards get the intended setting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@e2e/scripts/infra-down.sh`:
- Around line 9-19: The stop command can miss tearing down analytics services
when ANALYTICS_ENABLED=false because compose.dev.analytics.yaml and the
analytics service names aren't included; update the teardown to always target
the analytics compose and service names by creating a stop-specific
compose_files_stop that includes -f compose.dev.yaml and -f
compose.dev.analytics.yaml (or otherwise ensure compose.dev.analytics.yaml is
passed for stop) and build a services_to_stop array that always contains
analytics, tb-cli, tinybird-local plus the existing services array, then call
docker compose "${compose_files_stop[@]}" stop "${services_to_stop[@]}"; adjust
the variables ANALYTICS_ENABLED, compose_files, services, and the docker compose
stop invocation accordingly.
---
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1370-1379: The Playwright container step "Run e2e tests in
Playwright container" does not pass GHOST_E2E_ANALYTICS into the container, so
add GHOST_E2E_ANALYTICS to that step's env block (the same place where
TEST_WORKERS_COUNT, GHOST_E2E_MODE, GHOST_E2E_IMAGE, etc. are defined) so
run-playwright-container.sh receives and forwards the value (rather than relying
on its default). Ensure the env key name is exactly GHOST_E2E_ANALYTICS and
reference the workflow-level value (e.g. use the workflow/existing env
interpolation used elsewhere) so main shards get the intended setting.
🪄 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: 7a4a7688-d832-4cc1-ae92-2d094a8dbcce
📒 Files selected for processing (7)
.github/workflows/ci.ymle2e/helpers/environment/environment-manager.tse2e/helpers/environment/service-availability.tse2e/scripts/infra-down.she2e/scripts/infra-up.she2e/scripts/prepare-ci-e2e-build-mode.she2e/scripts/run-playwright-container.sh
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #27654 +/- ##
==========================================
- Coverage 73.17% 73.17% -0.01%
==========================================
Files 1561 1561
Lines 127051 127051
Branches 15383 15379 -4
==========================================
- Hits 92970 92966 -4
- Misses 33105 33126 +21
+ Partials 976 959 -17
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:
|
JohnONolan
force-pushed
the
codex/speed-up-e2e-ci
branch
from
1b5ddd5 to
41e05c7
Compare
ref TryGhost#27654 - E2E CI was starting Tinybird for every shard, but only analytics tests need it - splitting main and analytics shards keeps normal test runs lean while preserving analytics coverage - teardown still stops analytics services so switching modes leaves a clean compose project
JohnONolan
force-pushed
the
codex/speed-up-e2e-ci
branch
from
41e05c7 to
9860995
Compare
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)
.github/workflows/ci.yml (1)
1371-1379:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winPropagate
GHOST_E2E_ANALYTICSinto the Playwright container run step.
run-playwright-container.shdefaults this var totruewhen unset, so Main shards currently lose the explicit analytics-disabled signal inside the container. Pass the matrix value through to keep behavior consistent end-to-end.Suggested patch
- name: Run e2e tests in Playwright container env: TEST_WORKERS_COUNT: 1 GHOST_E2E_MODE: build GHOST_E2E_IMAGE: ${{ steps.load.outputs.image-tag }} + GHOST_E2E_ANALYTICS: ${{ matrix.analytics }} E2E_PLAYWRIGHT_PROJECTS: ${{ matrix.projects }} E2E_SHARD_INDEX: ${{ matrix.shardIndex }} E2E_SHARD_TOTAL: ${{ matrix.shardTotal }} E2E_RETRIES: 2🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 1371 - 1379, The Playwright container run step is not passing the GHOST_E2E_ANALYTICS matrix value into the container, so the run-playwright-container.sh script defaults to true; update the CI job env block for the step that executes run-playwright-container.sh to include GHOST_E2E_ANALYTICS: ${{ matrix.GHOST_E2E_ANALYTICS }} so the variable is propagated into the container and the script receives the explicit analytics setting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In @.github/workflows/ci.yml:
- Around line 1371-1379: The Playwright container run step is not passing the
GHOST_E2E_ANALYTICS matrix value into the container, so the
run-playwright-container.sh script defaults to true; update the CI job env block
for the step that executes run-playwright-container.sh to include
GHOST_E2E_ANALYTICS: ${{ matrix.GHOST_E2E_ANALYTICS }} so the variable is
propagated into the container and the script receives the explicit analytics
setting.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 620e9eaf-6520-4bc8-917f-00e9e5a52548
📒 Files selected for processing (7)
.github/workflows/ci.ymle2e/helpers/environment/environment-manager.tse2e/helpers/environment/service-availability.tse2e/scripts/infra-down.she2e/scripts/infra-up.she2e/scripts/prepare-ci-e2e-build-mode.she2e/scripts/run-playwright-container.sh
✅ Files skipped from review due to trivial changes (3)
- e2e/helpers/environment/environment-manager.ts
- e2e/scripts/prepare-ci-e2e-build-mode.sh
- e2e/scripts/infra-down.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- e2e/scripts/run-playwright-container.sh
2 participants
What changed
Shave 2 minutes off E2E tests
E2E_PLAYWRIGHT_PROJECTSand for toggling analytics infra viaGHOST_E2E_ANALYTICS.Why
Every E2E shard was paying the setup cost for analytics infrastructure even though most tests run in the main Playwright project and do not need Tinybird. This keeps the existing coverage shape while removing repeated Tinybird setup from the main shards.