fix(e2e): bound the Playwright run and make a failed login say why (closes #161) #165

Merged
not merged 6 commits from fix/161-e2e-bounded-and-diagnosable into main 2026-09-04 10:53:37 +00:00
Contributor

What & why

#161 is really two defects, and the second one is why the first was undiagnosable.

A wedged suite consumed the job, and took the post-mortem with it. Nothing bounded the
Playwright run, so CI stopped the job mid-suite — and if: always() does not survive that. Run
739's job metadata shows every step after the e2e as a 0-second failure stamped at the kill:

14 failure  09:48:17 -> 10:14:54  Self-service e2e (Playwright …)
15 failure  10:14:54 -> 10:14:54  verify-stack check summary       ← if: always()
16 failure  10:14:54 -> 10:14:54  e2e spec summary                 ← if: always()
17 failure  10:14:54 -> 10:14:54  Dump container logs on failure   ← if: failure()
18 failure  10:14:54 -> 10:14:54  Tear down                        ← if: always()

So the per-spec summary, the container-log dump and the teardown never ran, and the log lost
whatever the killed process had buffered — leaving the single line the issue was filed from.
globalTimeout now makes Playwright stop and report: the JSON report is written and those steps
still get their turn. (A timeout-minutes on the job would have reproduced the same failure, so
there isn't one.) The "~24-minute gap" is that kill, not necessarily a hang — note run 739 shows
run_attempt: 2, and concurrency.cancel-in-progress kills an in-flight run on any re-run or push.

A login that never got its form ate the 90-second test timeout. Playwright actions auto-wait
until the test timeout, not expect.timeout — so a portal that serves its page but never
bootstraps (its config.json fetch or the OIDC discovery behind authorize() failed; main.ts
only console.errors) spent 90s to report locator.fill: Test timeout of 90000ms exceeded: the
symptom, not the cause. That is catalogus.spec's 1.8 minutes. Both Keycloak forms are now asserted
visible first, with a 20s budget and a message naming the step that never happened.

Verified against a real blank-bootstrap portal — the beheer image served with a config.json that
is not JSON — which fails in 20.2s with "the Keycloak login form never appeared — the portal
did not reach Keycloak (check its config.json fetch and the OIDC discovery …)"
.

And the summary now says why. The per-spec table (#136) rendered a verdict icon and nothing
else, so even a surviving summary cost a log dive. Failing specs now carry their first error,
flattened for a table cell (ANSI stripped, newlines collapsed, | escaped, clipped) — shape
verified against a real @playwright/test 1.61 failing report, with a stdlib assert self-check on
make unit.

Closes #161

Definition of Done

  • Linked Gitea issue (above).
  • Failing test committed before the implementation.
  • Implementation makes the test pass; refactor commit follows (login helper dedup).
  • Conventional Commits referencing the issue (refs #161).
  • CI green — all Gitea Actions jobs.
  • docker compose up from a fresh clone reaches green health checks within 3 minutes (untouched).
  • Docs updated — docs/runbooks/gitea-actions-gotchas.md §9.
  • ADR — not needed: no boundary, dependency or coupling rule touched (test/CI infra only).
  • Demo note — not applicable: nothing user-visible.

Notes for reviewers

What this does not do: identify why the beheerder login failed that once. The evidence to do
that was destroyed by defect 2, which is what this PR fixes. The suite ran green here five times
today (catalogus.spec 1.1–5.3s each) — but a local box is not the loaded CI runner, so that is weak
evidence and I am not claiming the flake is gone. What changes is that the next occurrence is
bounded and self-describing: it fails in 20s naming the failing step, the JSON report survives, and
the summary prints the error. Please keep #161 in mind rather than treating this as proof.

Two follow-ups I did not pull into this PR:

  • All four portals show a permanently blank page if their startup fetch failsmain.ts does
    fetch('config.json').then(bootstrap).catch(console.error), one shot, no UI and no recovery. That
    is a real product gap (the deliberately-broken portal above is exactly what a user would see) and
    wants its own slice, not a test-infra PR.
  • retries: 1 is untouched. CLAUDE.md §15 says flaky tests are fixed rather than retried, but
    removing retries while a real flake is unexplained would trade a rare red for a frequent one.
    Worth revisiting once #161 recurs (or doesn't) with the new diagnostics.

The login-helper rename (medewerker-login.tskeycloak-login.ts, citizen logins routed through
loginBurger) is its own no-behaviour-change commit: the three citizen specs each duplicated the
same three-line login, so guarding the login path once meant routing them through it first.

## What & why #161 is really two defects, and the second one is why the first was undiagnosable. **A wedged suite consumed the job, and took the post-mortem with it.** Nothing bounded the Playwright run, so CI stopped the job mid-suite — and `if: always()` does not survive that. Run 739's job metadata shows every step after the e2e as a **0-second failure** stamped at the kill: ``` 14 failure 09:48:17 -> 10:14:54 Self-service e2e (Playwright …) 15 failure 10:14:54 -> 10:14:54 verify-stack check summary ← if: always() 16 failure 10:14:54 -> 10:14:54 e2e spec summary ← if: always() 17 failure 10:14:54 -> 10:14:54 Dump container logs on failure ← if: failure() 18 failure 10:14:54 -> 10:14:54 Tear down ← if: always() ``` So the per-spec summary, the container-log dump and the teardown never ran, and the log lost whatever the killed process had buffered — leaving the single `✘` line the issue was filed from. `globalTimeout` now makes Playwright stop and *report*: the JSON report is written and those steps still get their turn. (A `timeout-minutes` on the job would have reproduced the same failure, so there isn't one.) The "~24-minute gap" is that kill, not necessarily a hang — note run 739 shows `run_attempt: 2`, and `concurrency.cancel-in-progress` kills an in-flight run on any re-run or push. **A login that never got its form ate the 90-second test timeout.** Playwright actions auto-wait until the *test* timeout, not `expect.timeout` — so a portal that serves its page but never bootstraps (its `config.json` fetch or the OIDC discovery behind `authorize()` failed; `main.ts` only `console.error`s) spent 90s to report `locator.fill: Test timeout of 90000ms exceeded`: the symptom, not the cause. That is catalogus.spec's 1.8 minutes. Both Keycloak forms are now asserted visible first, with a 20s budget and a message naming the step that never happened. Verified against a real blank-bootstrap portal — the beheer image served with a `config.json` that is not JSON — which fails in **20.2s** with *"the Keycloak login form never appeared — the portal did not reach Keycloak (check its config.json fetch and the OIDC discovery …)"*. **And the summary now says why.** The per-spec table (#136) rendered a verdict icon and nothing else, so even a surviving summary cost a log dive. Failing specs now carry their first error, flattened for a table cell (ANSI stripped, newlines collapsed, `|` escaped, clipped) — shape verified against a real @playwright/test 1.61 failing report, with a stdlib assert self-check on `make unit`. Closes #161 ## Definition of Done - [x] Linked Gitea issue (above). - [x] Failing test committed before the implementation. - [x] Implementation makes the test pass; refactor commit follows (login helper dedup). - [x] Conventional Commits referencing the issue (`refs #161`). - [x] CI green — all Gitea Actions jobs. - [x] `docker compose up` from a fresh clone reaches green health checks within 3 minutes (untouched). - [x] Docs updated — `docs/runbooks/gitea-actions-gotchas.md` §9. - [x] ADR — not needed: no boundary, dependency or coupling rule touched (test/CI infra only). - [x] Demo note — not applicable: nothing user-visible. ## Notes for reviewers **What this does not do: identify why the beheerder login failed that once.** The evidence to do that was destroyed by defect 2, which is what this PR fixes. The suite ran green here five times today (catalogus.spec 1.1–5.3s each) — but a local box is not the loaded CI runner, so that is weak evidence and I am not claiming the flake is gone. What changes is that the next occurrence is bounded and self-describing: it fails in 20s naming the failing step, the JSON report survives, and the summary prints the error. Please keep #161 in mind rather than treating this as proof. **Two follow-ups I did not pull into this PR:** - *All four portals show a permanently blank page if their startup fetch fails* — `main.ts` does `fetch('config.json').then(bootstrap).catch(console.error)`, one shot, no UI and no recovery. That is a real product gap (the deliberately-broken portal above is exactly what a user would see) and wants its own slice, not a test-infra PR. - `retries: 1` is untouched. CLAUDE.md §15 says flaky tests are fixed rather than retried, but removing retries while a real flake is unexplained would trade a rare red for a frequent one. Worth revisiting once #161 recurs (or doesn't) with the new diagnostics. The login-helper rename (`medewerker-login.ts` → `keycloak-login.ts`, citizen logins routed through `loginBurger`) is its own no-behaviour-change commit: the three citizen specs each duplicated the same three-line login, so guarding the login path once meant routing them through it first.
not added 5 commits 2026-09-04 09:48:25 +00:00
#161 lost a 36-minute verify-stack job whose only surviving output was a
single ✘ line: the per-spec summary (#136) renders a verdict icon and nothing
else, so a red e2e still costs a log dive — and when the log is truncated or
the run is killed, there is nothing to dive into.

Adds a stdlib assert-based self-check for infra/playwright-summary.py (no
framework) and rides it on `make unit` so CI catches a broken summary.
Fails with "AssertionError: Test timeout of 90000ms exceeded" not in the
rendered markdown.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The per-spec table now has a "Why" column holding the spec's first error,
flattened for a markdown cell: ANSI stripped, newlines collapsed, `|` escaped
(a real report's message is multi-line, coloured, and embeds source-snippet
gutters), clipped to 300 chars. The column only appears when something failed.

So a red e2e names its cause in the summary even when the log is truncated or
the run is killed mid-stream — which is the state #161 was filed from.

Shape verified against an actual @playwright/test 1.61 failing report, not
just the fixture.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`medewerker-login.ts` becomes `keycloak-login.ts`: the three citizen specs
each duplicated the same three-line password login, so a fix to the login path
had to be made four times. They now call `loginBurger`, and both realms share
`submitPassword`.

No behaviour change — all 6 specs green against a live stack.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Two defects behind #161's opaque 36-minute verify-stack job.

**A login that never gets its form ate the test timeout.** `fill()` auto-waits
until the *test* timeout (90s), not the 15s expect timeout, so a portal that
serves its page but never bootstraps — its config.json fetch or the OIDC
discovery behind `authorize()` failed, and main.ts only console.errors — spent
90 seconds to report `locator.fill: Test timeout of 90000ms exceeded`: the
symptom, not the cause. That is catalogus.spec's 1.8 minutes in the issue.
Both Keycloak forms are now asserted visible first, with a 20s budget and a
message naming the step that never happened.

Verified against a real blank-bootstrap portal (a beheer image served with a
config.json that is not JSON): fails in 20.2s with
"the Keycloak login form never appeared — the portal did not reach Keycloak
(check its config.json fetch and the OIDC discovery …)".

**A wedged suite consumed the job.** Nothing bounded the run, so CI killed the
job — and with it the `if: always()` steps that would have explained the
failure: neither the per-spec summary nor the container-log dump ran (both show
0-second failures at the kill in run 739's metadata). `globalTimeout` makes
Playwright stop and *report* instead, so the JSON report is written and those
steps still run. 12 minutes over a ~1-minute suite: a backstop, not a budget.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
docs(ci): a killed job loses its if: always() steps — gotchas §9 (refs #161)
CI / lint (pull_request) Successful in 1m29s
CI / unit (pull_request) Canceled after 0s
CI / frontend (pull_request) Canceled after 0s
CI / mutation (pull_request) Canceled after 0s
CI / verify-stack (pull_request) Canceled after 0s
CI / build (pull_request) Canceled after 40s
ab1d824e1e
Records what #161's job metadata actually shows (steps 15-18 as 0-second
failures stamped at the kill), how to read step timings via the API instead of
trusting a truncated log, and the two conventions that follow: bound the work
inside the tool so it can still report, and never let an auto-waiting
Playwright action serve as the timeout.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
not added the type:bugarea:portal-behandel labels 2026-09-04 09:48:48 +00:00
not added 1 commit 2026-09-04 09:50:28 +00:00
Merge remote-tracking branch 'origin/main' into fix/161-e2e-bounded-and-diagnosable
CI / lint (pull_request) Successful in 2m19s
CI / build (pull_request) Successful in 1m9s
CI / unit (pull_request) Successful in 1m29s
CI / frontend (pull_request) Successful in 3m4s
CI / mutation (pull_request) Successful in 6m7s
CI / verify-stack (pull_request) Successful in 10m0s
7a4ec72f43
not merged commit d6b3f9764f into main 2026-09-04 10:53:37 +00:00
Sign in to join this conversation.