ci(e2e): make the shared Playwright job actually run — and pass - #787
Merged
Conversation
The shared `E2E Tests (Playwright)` job has never run in this repo —
`enable-playwright` was simply never set, so the job resolved to `skipped`
on every build. Turning it on needed three things beyond the flag.
1. A CI-scoped Playwright config.
The shared workflow runs the suite with no `--project`, so every project in
whichever config it picks will run. The root config declares three, and two
of them are documented as unable to pass on a CI runner: `visual` ("a CI
Linux runner will not byte-match a dev-container baseline") and
`docs-capture`, which re-shoots every tutorial screenshot and already has
its own dedicated job. The workflow looks for
`${playwright-test-path}/playwright.config.ts` first and only falls back to
the repo root, so `playwright-test-path: tests/e2e` plus a config there
selects the regression project without touching the root config, which
stays the entry point for local runs and `--project visual|docs-capture`.
That config also writes its report and traces to the app root. The
workflow uploads `server/apps/<app>/playwright-report/` and
`.../test-results/`; the root config writes under `tests/e2e/`, so both
upload steps matched nothing and — with `if-no-files-found: ignore` —
uploaded an empty artifact. A red run would have had no report to read.
2. A fresh-install seed that fails loudly.
`occ app:enable opencatalogi` runs the InitializeSettings repair step,
which is meant to import the publication register into OpenRegister. It is
not a reliable fresh-install path and it fails silently twice over: an
IRepairStep runs with no user session, so OpenRegister's RBAC can deny the
import, and `run()` catches \Exception and downgrades it to a warning it
calls "Non-fatal" — `occ` still exits 0. It also imports with
`force: false`, the version-guarded path, which can record a new
configuration version without applying anything.
In that state the app enables cleanly, the SPA boots, and the register is
simply absent. The suite's symptom is ~14 failures reading
`create 14/55 failed: 404` and "seeding a catalog must succeed" — messages
that accuse the fixtures instead of the missing import.
`tests/e2e/ci-seed.sh` does the import explicitly over the admin HTTP API
(real session, passes RBAC) with `force: true`, then verifies the register
and the four schemas the fixtures resolve by slug actually exist. A bad
provision is now one loud step failure instead of a misleading wall.
3. Ignore the new artefact paths so a local CI-config run stays clean.
rubenvdlinde
requested review from
Rem-Dam,
SudoThijn,
WilcoLouwerse,
bbrands02,
remko48 and
rjzondervan
as code owners
August 2, 2026 20:53
…efore specs
Two failures from the first real CI run of the Playwright job, both with
causes outside the specs' own assertions.
publish-workflow "Publish gate" — the test asserts that an anonymous
visitor gets the catalog directory (200) with the draft publication
filtered out of it. An anonymously VISIBLE CATALOG is the precondition for
that, not the thing under test — and the fixture never created one. The
catalog schema grants public read conditionally:
authorization.read: [{ group: "public",
match: { published: { $lte: "$now" } } }, …]
and its own `published` property is documented as "when set to a date in
the past, this catalog becomes publicly accessible". `createCatalog()` does
not set it, so the catalog is correctly not public and
`PublicationsController::index` 404s on `getCatalogBySlug()` returning null
— which is exactly what CI reported.
It passed on the dev container only because that instance runs an older
OpenRegister whose list path returns objects the conditional public-read
grant should exclude. Verified directly: a catalog created there with
`published` unset is still readable anonymously (HTTP 200), contradicting
its own schema. So the precondition was being supplied by an RBAC leak.
The catalog is now created with a past `published`, which is the same
mechanism the sibling test in this file already relies on
(`publicatiedatum` in the past on the publication schema) — and that test
passed on CI's OpenRegister, so the mechanism is confirmed on the build
that enforces the grant. Nothing is skipped and no assertion is relaxed;
the test now sets up the state it was always assuming.
catalog-detail-page — timed out at 60s on attempt 1 waiting for
`cn-index-page`, then passed in 9.1s on retry, while every later spec ran
in 4-7s. It is simply whichever spec runs first: the shared workflow serves
Nextcloud with `php -S` and does not set PHP_CLI_SERVER_WORKERS, so the
built-in server has ONE worker and the SPA's boot requests serialise behind
each other, on top of a cold opcache and a first bundle parse.
The assertion is not wrong — it was measuring server warm-up. So the
warm-up happens in ci-seed.sh, the environment-preparation step, where it
belongs. Raising that spec's timeout would have hidden the cold start
inside the assertion and kept drifting upward.
The warm-up added in the previous commit fetched `/apps/opencatalogi/js/opencatalogi-main.js` and logged `-> 200`, which looked fine and warmed nothing useful. Nextcloud serves an app's assets from whichever apps directory it was installed into — `/apps/<app>/` on the CI runner, `/custom_apps/<app>/` in the docker dev images — and asking for the wrong one does not 404. It returns HTTP 200 with Content-Type text/html: the NC error page, served through index.php. Verified on a live instance: with the bundle moved aside, that URL still answered `200 text/html` (40 KB) instead of the 7.6 MB bundle. So the src is now read out of the rendered app page and the response is checked for being JavaScript, which also reports its real size. On CI that check is a GATE rather than a warm-up. A missing bundle is the single most likely way this job could "succeed" dishonestly: the SPA never mounts, and because the asset request still returns 200 the failure surfaces only as a wall of selector timeouts with a misleading cause — while every status-code check in the pipeline reports success. Verified both directions on a live instance: bundle moved aside -> 10 of 10 UI specs failed and this check exits 1; bundle restored -> 10 of 10 passed and the check reports `200 text/javascript 7664588`.
…link was empty
WOO-517 has been red since it was written, identically on a seeded dev
container and on a fresh CI install. It is a fixture defect, not the
product defect it looked like.
OpenRegister does not derive a slug for a publication. Created without
one, it comes back with `@self.slug === null`, so
const pubSlug = pub.raw['@self']?.slug ?? pub.raw.slug ?? ''
resolved to '' and the document was linked as `publication: { slug: "" }`.
OpenCatalogi resolves a document's owning publication by that slug, and
`PublicationQueryService::resolveDocumentPublicationSummary()` returns null
immediately on `$slug === ''`; the assembler then drops the row with "No
linked publication — MUST NOT appear (SCH-PFTS-003)". The document could
therefore never surface — with or without `_content=true` — and the
product was doing exactly what its spec says.
Two things this hid:
- The failure read as a broken content-search chain. It is not. Verified
against a live instance that OpenRegister's own content search resolves
the marker correctly: `/apps/openregister/api/objects/{reg}/{doc}
?_search=<marker>&_content_search=true` returns the document, and the
chunk row and its tsquery match are both present in the database.
Everything from extraction through `ChunkMapper::searchByKeyword()`
works; only the document→publication link was missing.
- The test's negative assertion was vacuous. "A body-text-only match MUST
NOT surface without _content=true" passed because documents never
surfaced at all, not because the metadata arm excluded it. With the link
established, that assertion now discriminates.
Evidence, same instance, same pair: publication created WITHOUT a slug ->
OC search returns the publication only (`total: 1`); WITH an explicit slug
-> `total: 2`, the second row carrying `@self.schema === "document"`.
An explicit assertion on the resolved slug is added so that if OpenRegister
ever changes slug handling this fails on the cause rather than ten seconds
later on "the marker did not surface".
rubenvdlinde
added a commit
that referenced
this pull request
Aug 3, 2026
…e speak (#790) Two follow-ups to #787, both about the same failure mode: the suite quietly repairing or swallowing the one condition that would make a green run meaningless. 1. globalSetup rebuilt the bundle on CI, which makes a broken build invisible — and the suite untestable. `ensureBundleBuilt()` runs `npm run build` whenever `js/opencatalogi-main.js` is absent. On CI the workflow's own "Build app frontend" step has already run by then, so a missing bundle means that step produced nothing; rebuilding turns it into a green run instead of a build failure. It also defeated the positive control for #787. Run 30791459241 deleted the bundle before the specs and still passed 82/82 — not a false green, but a neutralised control. The log shows it plainly: POSITIVE CONTROL ACTIVE — bundle moved aside. The suite MUST now fail. [playwright globalSetup] bundle missing at …/js/opencatalogi-main.js; running 'npm run build' once… > NODE_ENV=production webpack --config webpack.config.js --progress The control only became valid once it truncated the file instead of removing it, since this function checks existence and never contents. So on CI a missing bundle is now a thrown error naming the step to look at. Locally the rebuild stays — there it is a real convenience, because a fresh checkout has no js/ and nothing else will build it. 2. The bundle gate died before it could explain itself. `set -euo pipefail` plus `grep | head | sed` in a command substitution meant that when the bundle src was not found — exactly the case the gate exists to diagnose — the script aborted on grep's exit 1 at that line. Exit status was still non-zero so the gate "worked", but it emitted none of the three error lines saying what to check. Verified both ways with the bundle moved aside: before, a bare non-zero exit; after, "could not locate the bundle src in the rendered app page" plus the full diagnosis, still exit 1.
2 tasks
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.
What
Turns on the shared
E2E Tests (Playwright)job for OpenCatalogi. It was never enabled here —enable-playwrightwas unset, so the job resolved toskippedon every build since the workflow gained it.This is phase 1 of a fleet-wide e2e enablement programme: make the job genuinely green on one repo, then roll the recipe out. Nothing outside this repo is enabled by this PR.
Why it needed more than the flag
The workflow passes no
--project. Whichever config it picks, every project in it runs. The root config declares three, two of which are documented in their own headers as unable to pass on a CI runner:visual— "PNG baselines are host-font/GPU specific, so a CI Linux runner will not byte-match a dev-container baseline"docs-capture— re-shoots every tutorial screenshot; already has its own dedicatedJourneydoc CapturejobThe run step looks for
${playwright-test-path}/playwright.config.tsfirst and only falls back to the repo root. Soplaywright-test-path: tests/e2eplus a config there selects the regression project, and the root config is untouched — it stays the entry point for local runs,npm run test:e2e:docs, and--project visual.The report was being uploaded from the wrong place. The workflow uploads
server/apps/<app>/playwright-report/and.../test-results/. The root config writes undertests/e2e/. Both upload steps matched nothing, andif-no-files-found: ignoremeant they said so quietly — a failing run would have produced an empty artifact and no report to read. The CI config writes to the app root.A fresh install has no publication register, and nothing says so.
occ app:enable opencatalogirunsInitializeSettings, which is supposed to import it. It is not a reliable fresh-install path, and it fails silently on two independent counts:IRepairStepruns with no user session, so OpenRegister's RBAC can deny the import — andrun()catches\Exceptionand downgrades it to a warning it explicitly labels "Non-fatal".occ app:enablestill exits 0.loadSettings(force: false), the version-guarded path, which can record a new configuration version without applying anything.The app then enables cleanly, the SPA boots, and the register is simply absent. The suite's symptom in that state is a wall of
create 14/55 failed: 404andseeding a catalog must succeed— messages that accuse the fixtures rather than the missing import.tests/e2e/ci-seed.sh(wired asplaywright-seed-command) does the import explicitly over the admin HTTP API — which has a real session and passes RBAC — withforce: true, then verifies the register and the four schemas the fixtures resolve by slug actually exist. A bad provision becomes one loud step failure instead of ~14 misleading spec failures. It refuses to run against an unset target outside CI, because on a dev boxlocalhost:8080is the shared container and this script performs admin writes.Not done here
No test is skipped, no assertion weakened, no timeout raised.