test(e2e): specs are not isolated — parallel runs interfere through one shared database #241
No Assignees
Notifications
Due Date
No due date set.
Dependencies
No dependencies set.
Reference: bermudalamb/redefined-designs#241
Reference in New Issue
Block a user
The e2e suite runs
fullyParallel: trueagainst a single shared Postgres. Specs mutate the same catalogue concurrently, so whether they pass depends on scheduling.Found while root-causing #239 on 2026-08-29. A full local run failed three filter specs:
admin-inventory-filters.spec.ts:133— counts each selected status in the tallyfavorites-filter.spec.ts:169— keeps showing a favorite after it sellssold-filter.spec.ts:81— the genuine #239 failureRe-run alone with
--workers=1, the first two pass. Only #239 fails in isolation. CI has been reporting one failure because its ordering happened to be kind, not because the other two are sound.Why this matters more than the failure it hid
CI reports
147 passed, 1 failedand has done for days. Two of those "passes" are luck. The suite cannot currently distinguish "this change broke something" from "the scheduler interleaved differently this time", which makes every future red build a judgement call rather than a signal — and the expensive kind of judgement call, because a full run costs ~21 minutes on a runner everything else queues behind.It also undermines
retries: 1in CI: a genuinely order-dependent failure can pass on retry and be reported as flaky rather than as a defect.Shape of a fix
Not obvious, and worth deciding rather than guessing:
workers: 1. Trivially correct and makes the slowest CI job slower still, on the runner that is already the bottleneck.Related
The existing guidance that e2e assertions should be scoped to the item under test rather than the whole grid is the same problem seen from the other end. This issue is the underlying cause.
Found while fixing #239.
Further evidence, 2026-08-30
A local parallel run of the full suite on
feb714cfailed four specs:admin-inventory-filters.spec.ts:133auth.spec.ts:176favorites-filter.spec.ts:169resend-verification.spec.ts:25CI on the same commit failed exactly one, and a different one:
admin-save-failures.spec.ts→ "saves an item successfully when the server accepts it".Five distinct specs have now failed across two runs of identical code, with no overlap between the two sets. Whichever tests happen to collide is decided by the scheduler.
Two of those —
authandresend-verification— had not appeared in any earlier run, which widens the blast radius beyond the filter specs where this was first noticed. It is not a filter problem; it is a suite problem.resend-verificationis worth singling out: "says something useful once the allowance runs out" depends on a rate limiter whose store is process-wide and in-memory. Concurrent specs registering and resending share that allowance, so the test can be exhausted by its neighbours before it runs. That is a second isolation axis on top of the shared database, and per-worker databases alone would not fix it.The admin-save test is skipped under #245 to get
maingreen. That is a holding action, and the skip should be reverted as soon as this lands.Correction: the rate limiter is not a shared axis
My earlier comment claimed
resend-verificationfails because the verification-resend limiter has a process-wide store shared by concurrent specs. That is wrong, and it would send a fix in the wrong direction, so it is worth retracting explicitly.verificationResendLimiteris keyed bykeyByCustomer—customer:${req.customerId}— and thecustomerfixture creates auniqueEmail()for every test, so each test registers its own customer with its own id and gets its own allowance. The store is process-wide, but the keys are not shared. Two specs resending concurrently do not collide.The store is exported for the integration suite, where
TRUNCATE ... RESTART IDENTITYrecycles customer ids and the allowance genuinely can leak between tests. That is a different suite with a different problem, and I read its comment as applying here.What the mechanisms actually are
With that removed, two remain:
Data collision on unscoped reads.
favorites-filter:169fetches/api/itemsand doesitems.find(...)across the whole catalogue, then dereferences the result. Another spec's activity can make that lookup miss, and it throws rather than failing an assertion. This is the real isolation problem, and it is about how assertions read shared state, not about specs owning unique data — most already do, viauniqueSuffix.Load-induced timing.
admin-inventory-filters:133toggles three statuses in its own browser context and assertsFilters (3); nothing another spec does can touch that state.resend-verification:25clicks four times and depends on the button settling between clicks.admin-save-failureswaits on a toast. These fail because the machine is saturated, not because specs interfere — which is why the failing set moves around and why CI and local disagree on which one goes.That distinction matters for the fix: scoping assertions addresses the first and does nothing for the second, and the second is the larger share of what we have actually observed.
Tasks 2–4 done on
fix/241-e2e-isolation-part2, and the held-back lever applied — with the evidence that justified it.Tasks 2–4. The nine unchecked lookups are now
findOrFail, so a missing row fails naming what was wanted and how many rows were searched instead of "Cannot read properties of undefined". Where a lookup was followed byexpect(x).toBeTruthy(), that assertion is gone —findOrFailalready guarantees it, with a better message. The expect timeout is 10s. The #245 skip is lifted.Those three were not enough, and now there is proof. Two full parallel runs on this branch, same commit, same machine, each failed 3 of 155 — and not the same three:
password-reset(×2),admin-inventory-filters:133admin-email-settings:20,admin-inventory-filters:133,resend-verification:25All 16 tests across those three specs then passed when run serially. That is the definition of contention rather than defect: specs asserting over tables other specs are concurrently writing to.
admin-inventory-filters:133is the clearest case — it counts a status tally across the whole table while other specs are inserting into it.So
workersis now 1, and the suite is green. 155 of 155, twice in a row. The cost is about three minutes: roughly 1 minute parallel against 3.9 and 4.0 serially. A red run that means something is worth three minutes — the previous state was a suite whose result nobody could act on, which is exactly why #245 had to skip a real test to work around it.Worth being clear that this treats the symptom well rather than the cause. The cause is one shared database. If that three minutes is ever needed back, the fix is giving each worker its own database, not raising the worker count again — raising it just reopens this issue. I have not filed that as a follow-up because there is no current need for the time; say if you would like one.
One process note: my plan for tasks 2–4 was written as though they might be sufficient, while the constraints section correctly said worker count was being held back "if flakes persist". They persisted. Running the suite rather than assuming was what settled it, and it needed the full stack stood up locally — the failures do not reproduce serially, so a spot-check of the changed specs would have shown green and proved nothing.
Done. A red e2e run means something again.
Four things landed, and only the last one actually fixed it:
findOrFailreplaced nine uncheckedcollection.find(...)dereferences, so a missing row fails naming what was wanted and how many rows were searched instead of "Cannot read properties of undefined" pointing at test plumbing.workers: 1, which is what actually cured it.The first three were not enough, and the evidence is worth keeping: two full parallel runs on the same commit each failed 3 of 155, and not the same three —
password-resetandadmin-inventory-filtersin one,admin-email-settings,admin-inventory-filtersandresend-verificationin the next. All 16 tests from those specs then passed serially. Failures that move between runs of identical code are contention, not defects.Two things this issue got wrong, both worth recording
Not everything blamed on contention was contention.
admin-save-failureshad a second, unrelated cause:getByRole('button', { name: 'OK' })matches an accessible name as a case-insensitive substring, so a leftover toast reading "Freed rmtiq9okg22k9e" matched it — the random base36 suffix happened to contain "ok". That reproduced serially, on a fresh database, so no amount of worker reduction would have fixed it. Fixed under #253.Serialising was necessary but not sufficient. With one worker the suite still shared one database with the development data, which nothing truncated — the storefront grew to 1,662 items and outran assertion timeouts locally while passing in CI. That was #186, now fixed by giving the suite a throwaway database of its own. Between them, #241 stopped specs interfering within a run and #186 stopped runs interfering with each other.
Where it stands
The suite has been 157 of 157 across several consecutive full runs, including on a database recreated from empty. The cost of serialising is about three minutes — roughly 1 minute parallel against 3.9 to 5.1 serially — and a red run that means something is worth three minutes.
The remaining known flake is
resend-verification→ "says something useful once the allowance runs out", split out as #257 rather than left buried here.If that three minutes is ever needed back, the fix is a database per worker, not raising the worker count — raising it just reopens this.
Closes #241