fix(e2e): pick the seeded customer by name, not first option - #675
Merged
Conversation
Scheduled CI run 31767912809 failed date-format.spec.ts:22 on all
three attempts: the worklog customer pick takes the FIRST combobox
option, but the option list is name-sorted and shared run-wide. A
throwaway admin-spec customer (E2EInline_*, global, without projects)
that leaked past its best-effort delete sorted before the seeded
'Freizeit' customer, so createWorklogEntry booked a customer without
projects and the dependent project combobox rendered empty ("Keine
Treffer") -- deterministically on every retry, because the leaked row
persists in db-e2e for the rest of the shard. The identical failure
already occurred in scheduled run 29629898230 (2026-07-18), a month
before the composer.lock bump the failing SHA carries, which
exonerates dependency PR #672.
Reproduced locally at e553352 by inserting a global, project-less
'E2EInline_*-draft' customer into db-e2e: byte-identical failure
signature. With this fix the affected specs pass repeatedly against
the still-polluted database.
Fix: createWorklogEntry and the three blind ArrowDown/first-option
customer picks in worklog-grid-editing.spec.ts filter the combobox to
the seeded bookable customer (SEEDED_BOOKABLE_CUSTOMER = 'Freizeit')
before picking. Project/activity stay first-option -- they are scoped
to that customer and therefore deterministic.
Claude-Session: https://claude.ai/code/session_01AcqcEjgwcQfp3vpnFa3gh6
Signed-off-by: Sebastian Mendel <github@sebastianmendel.de>
Member
Author
|
Self-review (Copilot review unavailable — monthly quota exhausted; diff reviewed manually plus an adversarial review pass). Checked on head 4ac64bd: e2e-only change, no app code touched. The root cause is well-evidenced — the failure artifact shows a leaked The red required |
|
CybotTM
added a commit
that referenced
this pull request
Aug 15, 2026
## What this closes [#675](#675) diagnosed a scheduled-run failure down to a leaked throwaway customer and hardened the *consumers* — `createWorklogEntry` now picks the seeded bookable customer by name instead of taking the first option. The leak itself was left open: `e2e/admin-inline-edit.spec.ts` creates a global, project-less `E2EInline_<ts>_<rand>` customer and deletes it in a `finally` that drives the row's Delete button and swallows every error. A test that dies mid-UI — with a modal covering that button, or the row detached — leaves the row behind, and because `db-e2e` is a persistent volume and the customer list is name-sorted, that row sorts ahead of `Freizeit` and pollutes every later spec in the shard that reads the list. This PR closes it at the source. `e2e/admin/admin-ui.spec.ts` turned out to be the worse of the two: its UI delete *is* the test's final assertion and it had no `finally` at all, so any failure before that point leaked unconditionally. ## Defense in depth **Per-test cleanup that survives a failed test.** `deleteThrowawayCustomers()` resolves the row over `GET /getAllCustomers` and removes it with `POST /customer/delete` — the same JSON endpoint the admin UI uses. No page state is involved, so it works from a `finally` after any UI failure; an open modal can no longer block it, which is why the `Escape`-then-delete dance in the inline-edit spec could go away. It matches on the name prefix, so the renamed variants (`-edited`, `-draft`, `_Renamed`) are covered without the caller enumerating which name actually landed. **A cleanup failure is reported, not swallowed.** A row that could not be removed produces a `console.error` line and a `cleanup-failed` TestInfo annotation, so CI surfaces it instead of it becoming the next shard's mystery flake. Nothing in the helper throws, so a `finally` calling it can never replace the test's own error with a teardown error. The verdict comes from re-reading the customer list rather than from the delete's status code: `/customer/delete` answers 422 both for a genuine failure and for a row the parallel worker already removed, and reporting on the status alone made every concurrent sweep cry leak (observed, then fixed). **A sweep so a leak cannot outlive the spec.** `sweepStaleThrowawayCustomers()` runs in the `beforeEach` of both specs that create throwaway rows and drops ones left by an earlier run that crashed hard enough to skip its own `finally`, so a shard heals itself instead of needing a manual `db-e2e` cleanup. Only rows whose embedded timestamp is older than ten minutes are touched — `fullyParallel` is on with two workers per CI shard, so the sibling worker may have a fresh throwaway row live at that very moment, and the per-test timeout is 30s. The UI delete in `admin-ui.spec.ts` deliberately stays a UI click: exercising it is that test's subject. The new `finally` is only a safety net around it. ## Sibling check `.admin-crud-toolbar`/Add-modal entity creation exists in exactly the two specs fixed here — no other spec creates an admin entity, so the customer-shaped leak has no further instances. The worklog specs (`worklog-crud`, `worklog-grid-editing`, `date-format` via `cleanupWorklogEntries`, and `summary-refresh` via `cleanupStamped`) create throwaway *entries* rather than customers. They share the best-effort shape, but the exposure differs in kind: entries are per-user, are matched by their own unique stamp rather than positionally, and are already cleaned by a prefix sweep rather than by exact identity — so a leaked entry cannot reorder a relation list the way a leaked customer does. `cleanupWorklogEntries` does still scrape the DOM rather than the API, which is the same root cause in a milder form; that is worth a separate change, and it is not folded in here because the specs it serves are the ones `e2e/AGENTS.md` marks as CI-authoritative and therefore not verifiable on this branch. ## Verification Ran against a local e2e stack (`ghcr.io/netresearch/timetracker:e2e`, compose profile `e2e`, own project namespace, `E2E_BASE_URL=http://localhost:8768`), asserting on the `customers` table directly after each run. *Baseline on unmodified `origin/main` (3e2a2fe):* `admin-inline-edit` 13/13 pass; `date-format` 2/2 fail on `getByLabel(/Date format|Datumsformat/i)` never resolving — a pre-existing local-vs-CI divergence, identical before and after this change. *The leak reproduced and closed, same tree and containers, A/B via a temporary probe that opens the Edit modal and then throws:* with the pre-fix cleanup the row survives in `db-e2e` (`E2EInline_1786772591271_324639` left behind); with this one the table holds only the three seed rows, and the test still reports `Error: simulated mid-test failure` rather than a teardown error. *The sweep:* a planted stale row (`E2EInline_9999999999_999999-draft`, plus an `E2ECustomer_..._Renamed`) is removed on the next run, while a row stamped with the current `Date.now()` is left untouched — so the age bound that protects a concurrent worker is doing real work rather than the sweep simply deleting everything. *Shard-1-like combination* (`admin-inline-edit` + `admin/admin-ui` + `date-format`, `--workers=2`) with a stale row planted first: 18 passed, the 2 pre-existing `date-format` failures unchanged, the planted row swept, and the `customers` table back to exactly its three seed rows — no cross-spec residue. The diff is confined to `e2e/`.
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.
Root cause
The scheduled run 31767912809 failed
e2e/date-format.spec.ts:22identically on all three attempts. The failure artifact'serror-context.mdshows the page state at failure time: the worklog row's customer cell already holdsE2EInline_1786679426486_746937-draft— a throwaway customer created byadmin-inline-edit.spec.ts("the Edit button opens the modal seeded with the in-progress inline value", which ran 03:50:24–03:50:36 in the same shard; the name's embedded timestamp1786679426486= 03:50:26 UTC) — and the open Projekt combobox shows "Keine Treffer". That throwaway customer is created Global and owns no projects; itsfinally-block delete is explicitly best-effort (failures swallowed), and in this run it leaked.CustomerRepository::getCustomersByUserorders by name ASC, soE2EInline_*sorts before the seeded bookable customerFreizeit— andcreateWorklogEntry'spickFirstOption(page, row, 'customer')blindly picks the first option. Result: a customer without projects is booked, the dependent project combobox is empty, andawait expect(option).toBeVisible({ timeout: 8000 })fails. The leaked row persists in db-e2e for the rest of the shard, which is why all three in-run attempts failed identically — and why the next day's fresh stack was green again.Not a composer regression
The framing "the only diff to the last green run is composer.lock (PR #672)" turned out to be a red herring: PR #672's own CI ran all four E2E shards green (run 31696793473), and the byte-identical failure (same spec, same locator, same three-attempt pattern, same trailing
page.waitForResponse: Test ended) already occurred in scheduled run 29629898230 on 2026-07-18 at SHA8bee8881— a month before #672 merged. In that run the admin customer test had visibly flaked right before date-format failed. This is a cross-test data-pollution race, present whenever an admin spec's throwaway customer outlives its best-effort cleanup within a shard.Change
e2e/helpers/worklog.ts: newSEEDED_BOOKABLE_CUSTOMERconstant (Freizeit, the one seeded customer both e2e users can book, pere2e/AGENTS.md/sql/testdata.sql) and apickOptionByTexthelper that filters the combobox by name before picking.createWorklogEntrynow selects the customer by name; project/activity keep first-option picks because they are scoped to that customer and therefore deterministic.e2e/worklog-grid-editing.spec.ts: the three remaining blind customer picks (twoarrowEnter()guided-flow closures and the focus-retention test at line 236) filter toSEEDED_BOOKABLE_CUSTOMERbeforeArrowDown— the focus-retention test provably fails the same way under a leaked customer (verified locally, see below).No app code changed; the timing/waiting logic of the helpers is unchanged. Leaks can still happen (the admin cleanup remains best-effort by design), but they can no longer redirect the worklog specs onto a project-less customer.
Verification
Local e2e stack at
e5533522(CI-equivalent:ghcr.io/netresearch/timetracker:e2eimage, compose profile e2e, fresh seed). Injected the pollution the artifact shows (INSERT INTO customers (name, active, global) VALUES ('E2EInline_9999999999_123456-draft', 1, 1)), then rannpx playwright test e2e/date-format.spec.ts:22on the unmodified tree: reproduced the CI failure byte-for-byte (locator('.combobox-content .combobox-item').first()→element(s) not found, thenpage.waitForResponse: Test ended). With the fix applied and the pollution still in the database:e2e/date-format.spec.ts:22passed 3/3 single runs, and the full consumer set (worklog-grid-editing,worklog-crud,session-expiry,date-format— every spec usingcreateWorklogEntry) passed 17/17 with CI-like--workers=2 --retries=2. The same polluted-DB run also exposed and confirmed the focus-retention test as a second victim of the same class before its fix.Evidence links