The frontend suite's 5s default timeout is a release blocker, not a safety net #151

New issue

Open

opened 2026-08-08 20:12:41 +00:00 by claude-bot · 0 comments

claude-bot commented

2026-08-08 20:12:41 +00:00

Severity: MEDIUM — it has now failed a release

Filed after the fact, because the fix is already in (fddc0ba) and the references in vite.config.ts and the changelog needed somewhere real to point.

What happened

Two different tests have timed out at vitest's 5000ms default on a contended CI runner and passed on a re-run of the same commit:

PhotoBrowserPage.test.tsx:460 — the virtualizer windowing test. CI run 73. Fixed in #147 with a per-test timeout.
ReviewWorkspacePage.test.tsx:243 — "names the reviewer who got there first and what they decided". CI run 85.

Neither test is slow. Both runs reported more time inside tests than the whole run took in wall clock — run 73: tests 121.07s against 88.93s; run 85: tests 46.37s against 31.87s. That is a runner sharing its cores with something else, not a test doing more work than it should.

Why it is worth more than its size

publish gates on the frontend job (needs: [backend, frontend, e2e]). So a timing flake on a tag push means no image is built — and because the run fails rather than the publish step, the symptom is a version tag that exists with nothing behind it in the registry.

That is exactly what happened to v0.3.3: tagged, frontend flaked, publish never ran, and the deployment's pin pointed at an image that did not exist. The tag had to be moved onto the fix.

What was done, and the judgement that was wrong

#147 considered three options and put "raise testTimeout globally" last, arguing it was the least targeted and would slow the feedback on a genuinely hung test. That reasoning is sound in isolation and did not survive contact with a second instance: the per-test timeout fixed one test and not the cause, and the cause then cost a release.

testTimeout is now 20s globally (frontend/vite.config.ts). The trade is small and the asymmetry is the point:

a genuinely hung test now takes 20s to fail instead of 5, once, against a suite whose actual test time is about ten seconds;
the default's cost is a failed release for a reason unconnected to the change, which has happened twice in one day.

#147's per-test timeout is deliberately left in place. It carries the explanation of why that particular fixture is six hundred photographs, which is worth keeping whatever the global default is.

If this recurs anyway

A 20s budget on a runner that stalls for longer is still a flake, just a rarer one. The next lever is not a bigger number — it is finding out why the runner is contended, since the same host builds the images and runs the backend and e2e suites, and a tag push starts all of them plus a Docker build. Worth looking at forgejo-runner-ansible in iac-repo before raising this again.

References

frontend/vite.config.ts — the setting and its reasoning
#147 (the first instance, and the narrower fix)
CI runs 73 and 85

## Severity: MEDIUM — it has now failed a release Filed after the fact, because the fix is already in (`fddc0ba`) and the references in `vite.config.ts` and the changelog needed somewhere real to point. ## What happened Two different tests have timed out at vitest's 5000ms default on a contended CI runner and passed on a re-run of the same commit: - `PhotoBrowserPage.test.tsx:460` — the virtualizer windowing test. CI run 73. Fixed in #147 with a per-test timeout. - `ReviewWorkspacePage.test.tsx:243` — *"names the reviewer who got there first and what they decided"*. CI run 85. **Neither test is slow.** Both runs reported more time inside `tests` than the whole run took in wall clock — run 73: `tests 121.07s` against `88.93s`; run 85: `tests 46.37s` against `31.87s`. That is a runner sharing its cores with something else, not a test doing more work than it should. ## Why it is worth more than its size `publish` gates on the frontend job (`needs: [backend, frontend, e2e]`). So a timing flake on a **tag** push means no image is built — and because the run fails rather than the publish step, the symptom is a version tag that exists with nothing behind it in the registry. That is exactly what happened to **v0.3.3**: tagged, frontend flaked, `publish` never ran, and the deployment's pin pointed at an image that did not exist. The tag had to be moved onto the fix. ## What was done, and the judgement that was wrong #147 considered three options and put "raise `testTimeout` globally" last, arguing it was the least targeted and would slow the feedback on a genuinely hung test. That reasoning is sound in isolation and did not survive contact with a second instance: the per-test timeout fixed one test and not the cause, and the cause then cost a release. `testTimeout` is now **20s globally** (`frontend/vite.config.ts`). The trade is small and the asymmetry is the point: - a genuinely hung test now takes 20s to fail instead of 5, once, against a suite whose actual test time is about ten seconds; - the default's cost is a failed release for a reason unconnected to the change, which has happened twice in one day. #147's per-test timeout is deliberately left in place. It carries the explanation of why that particular fixture is six hundred photographs, which is worth keeping whatever the global default is. ## If this recurs anyway A 20s budget on a runner that stalls for longer is still a flake, just a rarer one. The next lever is not a bigger number — it is finding out why the runner is contended, since the same host builds the images and runs the backend and e2e suites, and a tag push starts all of them plus a Docker build. Worth looking at `forgejo-runner-ansible` in iac-repo before raising this again. ## References - `frontend/vite.config.ts` — the setting and its reasoning - #147 (the first instance, and the narrower fix) - CI runs 73 and 85

claude-bot referenced this issue

2026-08-08 20:34:03 +00:00

Authorization should come from Authentik: drop the email_verified gate, take roles from groups #149