WEB-1218: [CI/CD] Remove the duplicated E2E readiness gates from playwright.yml - #3968
Conversation
…wright.yml The readiness ceremony for the E2E stack existed four times over: the compose healthchecks, four bash steps in this workflow, the functional probe in playwright/global-setup.ts, and a fourth copy in the nightly canary that was cloned from this file. The four bash steps do no work. docker compose up -d blocks until every `depends_on: condition: service_healthy` dependency reports healthy, so Postgres and Fineract are already up when that step returns — the run log for the last green build on dev shows `Container e2e-fineract Healthy` at 18:14:15 and the compose step returning at 18:14:16, after which the four steps measured 0s, 0s, 2s and 0s. The 2s step is the only one that checked anything, and pollFineract() in playwright/utils/readiness.ts makes the same authenticated request to /api/v1/offices moments later in the same job — with a bounded retry schedule, an actionable error message, and a container-log dump on failure, none of which the bash loop had. The web-app container has no compose healthcheck, so its readiness is covered by pollWebApp() for the same reason. Readiness is therefore left to compose (ordering) and global-setup.ts (functional probe), and a comment on the compose step records why no wait steps follow it. Two pieces of drift go with them: the test command now calls the existing `playwright:ci` npm script instead of restating its flags, and the teardown drops `docker system prune -f`, which reclaims nothing on an ephemeral runner. One deliberate behavioural difference: the deleted step grepped the offices response for the literal string `Head Office`, where the probe asserts a non-empty array. Tightening that belongs in readiness.ts, where it can carry a unit test, rather than in workflow YAML.
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Team Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
Note
|
2 participants
Description
The readiness ceremony for the E2E stack currently exists four times over:
docker-compose.e2e.yml— the real healthchecks (pg_isready; Fineractwget /actuator/health,start_period: 120s, 60 retries) plusdepends_on: condition: service_healthy..github/workflows/playwright.yml— fourWait for …/Verify …bash steps.playwright/global-setup.ts—pollFineract()/pollWebApp()fromplaywright/utils/readiness.ts(WEB-1012): deadline-bounded, functional, and dumps container logs on failure. Runs in CI only, viaglobalSetup: process.env.CI ? … : undefined..github/workflows/e2e-nightly.yml(WEB-1184, WEB-1184: [CI/CD] Add a nightly Fineract drift canary #3917, open) — a fourth copy of Initial codebase #2, because it was cloned from this workflow step-for-step.Copy #2 does no work.
docker compose up -dblocks until everydepends_on: condition: service_healthydependency reports healthy, so Postgres and Fineract are already up by the time that step returns. The last green run ondev(34050859524) showsContainer e2e-postgres Healthyat 18:13:19 andContainer e2e-fineract Healthyat 18:14:15, withStart E2E infrastructurereturning at 18:14:16 — after which the four steps measured:The 2s step is the only one that checks anything, and
pollFineract()makes the same authenticated request to/api/v1/officesmoments later in the same job — with a bounded retry schedule against a 90s ceiling, an actionable error message, anddumpDockerLogs()on failure, none of which the bash loop had. Theweb-appservice has no compose healthcheck, soup -dreturns when it is Started rather than serving; that gap is covered bypollWebApp(), which is whyglobal-setup.tsis kept rather than readiness being removed wholesale.So this deletes copy #2 and leaves readiness owned by compose (ordering) and
global-setup.ts(functional probe), with a comment on the compose step recording why no wait steps follow it.This is not a performance change — the deleted steps cost ~2s in total. The value is that there is now one place where "is the stack ready?" is answered, one failure mode to debug instead of two that look unrelated, and no untested shell duplicating logic that is unit-tested in
readiness.spec.ts. It also unblocks the de-duplication that #3917 commits to in its own description, so that PR can drop its copy on rebase instead of landing a fourth one.Two smaller pieces of drift go with them:
playwright:cinpm script instead of restating its flags inline, so the local and CI invocations cannot diverge.docker system prune -f, which reclaims nothing on an ephemeral runner.One deliberate behavioural difference, worth a reviewer's attention: the deleted step grepped the offices response for the literal string
Head Office, whereasdefaultFineractProbeasserts a non-empty array. Marginally weaker on the seed office's name, stronger on everything else. If exact parity is wanted, tightening that belongs inreadiness.tswhere it can carry a unit test, not in workflow YAML.Failure mode after this change: if the stack never becomes healthy, the job fails at
up -dwithdependency failed to start: container e2e-fineract is unhealthyinstead of timing out inside a bash loop.Dump Docker logs on failurestill fires, and the healthcheck ceiling (~12 min) stays well inside the 45-minute job timeout.Related issues and discussion
WEB-1218
Related: #3917 (WEB-1184) is the fourth copy referred to above and will drop it on rebase.
Screenshots, if any
N/A — CI workflow change, no UI surface.
Checklist
Please make sure these boxes are checked before submitting your pull request - thanks!
If you have multiple commits please combine them into one commit by squashing them.
Read and understood the contribution guidelines at
web-app/.github/CONTRIBUTING.md.