ci(playwright): make the e2e job concurrent, told-where-to-seed, and readable - #124
Merged
Conversation
…readable Four defects found while making the shared `E2E Tests (Playwright)` job pass end-to-end on a repo for the first time (opencatalogi #787). All four are in this job, not in any caller, so no caller can work around them. 1. `php -S` runs ONE worker. The PHP built-in server is single-worker by default, so every request a Nextcloud SPA fires on boot is serialised behind the one before it. Measured on opencatalogi: whichever spec happened to run first blew its 60s test timeout waiting for the index page and then passed in 9.1s on retry, while every later spec ran in 4-7s — a guaranteed retry on every run, and a plain failure for any repo not using `retries`. Nothing about those assertions was wrong; they were measuring server warm-up. `PHP_CLI_SERVER_WORKERS: 8` makes the server concurrent. 2. The seed step is not told which instance to seed. "Seed test data" declared no `env:` at all, while "Run Playwright tests" right below it exports BASE_URL/ADMIN_USER/ADMIN_PASSWORD. A seed script is normally driving the app's own admin API, so it needs both — and with nothing exported the only way to write one was to hardcode `http://localhost:8080`. That literal is the SHARED dev container on a developer box, which makes the same script unsafe to run anywhere but CI. The journeydoc capture job already exports the full set to its own seed step; this matches it. 3. The test step exports only BASE_URL. A survey of the 21 fleet repos found 15 whose Playwright config resolves the target as `process.env.NEXTCLOUD_URL || 'http://localhost:8080'` and never reads BASE_URL. They do not fail here — they fall through to the literal, which happens to be this runner's own `php -S`. So the suite runs against the right host for the wrong reason, and CI silently certifies a config that points at the shared dev container the moment anyone runs it locally. Exporting every name the fleet actually uses means the variable, not the fallback, selects the target. (Repos should still drop the literal — openregister/doriath/shillinq have the resolver that throws instead.) 4. Both upload steps matched nothing for most repos. They list only `server/apps/<app>/playwright-report/` and `.../test-results/`, but the scaffolded config in most fleet repos writes under `tests/e2e/`. With `if-no-files-found: ignore`, that failed quietly and uploaded an empty artifact — so a red run left no report and no traces to read, exactly when they are needed. Both paths are now listed; unmatched ones are ignored. Still open, not fixed here because it needs a new input and a decision: the run step passes no `--project`, so every project in the chosen config runs — including `visual` (pixel baselines that a CI Linux runner cannot byte-match) and `docs-capture` (screenshot re-shoots that have their own job). opencatalogi worked around it with a CI-only config at `tests/e2e/playwright.config.ts`, which the existing config lookup finds first. A `playwright-project` input would let callers say this directly.
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
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.
Four defects in the shared
E2E Tests (Playwright)job, all found while making it pass end to end on a repo for the first time — ConductionNL/opencatalogi#787, now green at 82 passed / 0 failed, with a bundle-move-aside control confirming the specs genuinely load the app. All four are in this job, so no caller can work around them.1.
php -Sruns one workerSingle-worker by default, so every request a Nextcloud SPA fires on boot serialises. Measured: whichever spec ran first blew its 60s timeout waiting for the index page, then passed in 9.1s on retry, while every later spec ran in 4–7s. A guaranteed retry on every run — and a plain failure for any repo not using
retries. →PHP_CLI_SERVER_WORKERS: 8.2. The seed step isn't told which instance to seed
Seed test datadeclared noenv:, whileRun Playwright testsdirectly below exportsBASE_URL/ADMIN_USER/ADMIN_PASSWORD. A seed script normally drives the app's own admin API, so the only way to write one was to hardcodehttp://localhost:8080— the shared dev container on a developer box. The journeydoc capture job already exports the full set to its own seed step; this matches it.3. The test step exports only
BASE_URLSurveying the 21 fleet repos: 15 resolve the target as
process.env.NEXTCLOUD_URL || 'http://localhost:8080'and never readBASE_URL. They don't fail — they fall through to the literal, which happens to be this runner's ownphp -S. So the suite runs against the right host for the wrong reason, and CI silently certifies a config that points at the shared dev container the moment anyone runs it locally. Repos should still drop the literal (openregister, doriath, shillinq, planix, scholiq, zaakafhandelapp have the resolver that throws); this just stops CI depending on it.4. Both upload steps matched nothing for most repos
They list only
server/apps/<app>/playwright-report/and.../test-results/, but the scaffolded config in most fleet repos writes undertests/e2e/. Withif-no-files-found: ignorethat failed quietly and uploaded an empty artifact — a red run left no report and no traces, exactly when you need them.Not fixed here — needs a new input and a decision
The run step passes no
--project, so every project in the chosen config runs, includingvisual(pixel baselines a CI Linux runner cannot byte-match — the configs say so themselves) anddocs-capture(screenshot re-shoots with their own dedicated job). opencatalogi worked around it with a CI-only config attests/e2e/playwright.config.ts, which the existing lookup finds before the repo root. Aplaywright-projectinput would let callers say this directly — happy to add it if you'd prefer that shape.Also worth knowing (not a change here)
A newly created branch is skipped entirely by the caller guard
github.event.created != true. The run appears in the list and reportsskipped, so a first push looks like it ran CI and ran nothing. It takes a second push to trigger.