bermudalamb/redefined-designs
1
0
Fork 0
Code Issues 2 Pull Requests 1 Actions Packages Projects 3 Releases Wiki Activity

The e2e suite and the backend integration suite share one database, and the integration suite truncates it #116

New Issue
Closed
opened 2026-08-22 09:49:45 -05:00 by bermudalamb · 2 comments
bermudalamb commented 2026-08-22 09:49:45 -05:00
Owner
Copy Link
Copy Source

The Playwright e2e run and the backend integration run both use the same local Postgres database. The integration suite calls resetDb(), which truncates. Run the two together, or start integration while an e2e run is in flight, and rows disappear underneath the browser.

This is already written down in the tests as something to work around rather than fix. From frontend/tests/e2e/admin-theme.spec.ts:

// The Categories and Tags tabs render an empty state when there is nothing to
// show, and the shared dev database gets truncated by the backend integration
// suite. Seed what each test needs rather than depending on what happens to be
// there.

The workarounds have been accumulating:

  • admin-theme.spec.ts seeds its own category and tag on every test instead of trusting the fixtures.
  • email-templates.spec.ts is forced to test.describe.configure({ mode: 'serial' }) and carries an afterEach that DELETEs the stored template, because templates live in admin_settings and are global. It is the only spec in the suite that cannot run in parallel.
  • Most specs uniquify their fixture names with a Date.now() / Math.random() suffix. That handles collisions between runs. It does nothing about a truncation mid-run.

There is no per-run isolation for e2e at all. playwright.config.ts declares no globalSetup, nothing resets or namespaces the database, and fullyParallel: true is on.

Why this is worth fixing rather than working around once more

Each workaround is cheap on its own, and each was individually reasonable, which is precisely why there are now several. What they cost collectively is that a failure in this suite is ambiguous: it might be a real regression, or it might be another suite having truncated a table. That ambiguity is the expensive part, and it gets worse as the suite grows.

Options, roughly in increasing order of effort

  • Give e2e its own database, separate from the one integration truncates. Smallest change that removes the interference outright.
  • Add a globalSetup that migrates and seeds a known baseline before the run, so specs can stop defensively seeding.
  • Namespace the global state that exists. admin_settings is the one that forced serial mode, so fixing it lets the email-template spec go back to parallel.

Verification

Whichever is chosen, the check is behavioural rather than structural: email-templates.spec.ts no longer needs mode: 'serial', and admin-theme.spec.ts no longer needs its defensive seed. If either workaround still has to stay, the underlying problem has not been fixed.

The Playwright e2e run and the backend integration run both use the same local Postgres database. The integration suite calls `resetDb()`, which truncates. Run the two together, or start integration while an e2e run is in flight, and rows disappear underneath the browser. This is already written down in the tests as something to work around rather than fix. From `frontend/tests/e2e/admin-theme.spec.ts`: ``` // The Categories and Tags tabs render an empty state when there is nothing to // show, and the shared dev database gets truncated by the backend integration // suite. Seed what each test needs rather than depending on what happens to be // there. ``` The workarounds have been accumulating: - `admin-theme.spec.ts` seeds its own category and tag on every test instead of trusting the fixtures. - `email-templates.spec.ts` is forced to `test.describe.configure({ mode: 'serial' })` and carries an `afterEach` that DELETEs the stored template, because templates live in `admin_settings` and are global. It is the only spec in the suite that cannot run in parallel. - Most specs uniquify their fixture names with a `Date.now()` / `Math.random()` suffix. That handles collisions between runs. It does nothing about a truncation mid-run. There is no per-run isolation for e2e at all. `playwright.config.ts` declares no `globalSetup`, nothing resets or namespaces the database, and `fullyParallel: true` is on. ## Why this is worth fixing rather than working around once more Each workaround is cheap on its own, and each was individually reasonable, which is precisely why there are now several. What they cost collectively is that a failure in this suite is ambiguous: it might be a real regression, or it might be another suite having truncated a table. That ambiguity is the expensive part, and it gets worse as the suite grows. ## Options, roughly in increasing order of effort - Give e2e its own database, separate from the one integration truncates. Smallest change that removes the interference outright. - Add a `globalSetup` that migrates and seeds a known baseline before the run, so specs can stop defensively seeding. - Namespace the global state that exists. `admin_settings` is the one that forced serial mode, so fixing it lets the email-template spec go back to parallel. ## Verification Whichever is chosen, the check is behavioural rather than structural: `email-templates.spec.ts` no longer needs `mode: 'serial'`, and `admin-theme.spec.ts` no longer needs its defensive seed. If either workaround still has to stay, the underlying problem has not been fixed.
bermudalamb added this to the Code Quality and Hardening 2 project 2026-08-22 09:59:59 -05:00
bermudalamb self-assigned this 2026-08-22 10:00:17 -05:00
bermudalamb moved this to Review in Code Quality and Hardening 2 on 2026-08-24 13:15:25 -05:00
bermudalamb commented 2026-08-24 15:37:23 -05:00
Author
Owner
Copy Link
Copy Source

Checked before starting, and the scope is narrower than the body says

Two of this issue's premises have gone stale, and its two success criteria turn out to belong to different problems.

Locally, the suites already use separate databases

Suite Container Port Database
Integration redefined-designs-test-db 55432 redefined_test (tmpfs, disposable)
e2e backend redefined-designs-local-db 55500 redefined_local

scripts/start-local.ps1 takes its own container and database, and its port comment is explicit that 55500 is "deliberately not 55432". Corroborated by the data: redefined_local holds over a thousand accumulated items while the integration database is migrated from scratch on every run.

So the truncation this issue describes does not happen locally any more. The comment in admin-theme.spec.ts about "the shared dev database" predates that split.

In CI they genuinely do share one

.gitea/workflows/sonarqube.yml sets both:

TEST_PGDATABASE: redefined_test   # the integration suite
PGDATABASE: redefined_test        # the backend the e2e run drives

One database. The integration step truncates it, and the e2e run then starts against whatever it left. Sequential rather than concurrent, so nothing races — but the e2e suite inherits an empty database, which is exactly the state admin-theme.spec.ts defends against.

This is the whole of the remaining problem, and it is CI-only.

The two success criteria are not the same problem

email-templates.spec.ts no longer needs mode: 'serial'

That one is not about the integration suite at all. The spec explains itself: the templates live in admin_settings, which is global per database, and the e2e workers race each other over it. The integration suite is not running at the time. Making it parallel needs a database per Playwright worker — a much larger change than this issue describes, and one worth its own decision rather than being smuggled in here.

admin-theme.spec.ts no longer needs its defensive seed

That one does follow from the CI fix, though it is worth asking whether it should. A test seeding what it needs is ordinarily good practice, not a workaround; the objection here is only that it was forced by another suite's truncation.

Why this is not being fixed right now

The remaining fix is a CI-only change — give the e2e backend its own database in the workflow — and CI is currently broken and parked under #154, whose cause is not yet understood. Changing the database wiring of the very subsystem that is misbehaving, with no way to verify the result, would be guessing.

Proposed sequence: settle #154 first, then make this change and confirm it in a green run.

Revised scope, for whenever it is picked up

  • Give the e2e backend its own PGDATABASE in sonarqube.yml, created and migrated alongside the integration one
  • Re-evaluate admin-theme.spec.ts's defensive seed afterwards rather than assuming it must go
  • Split the email-templates serial-mode question out into its own issue, since per-worker databases is a different piece of work with a different cost
## Checked before starting, and the scope is narrower than the body says Two of this issue's premises have gone stale, and its two success criteria turn out to belong to different problems. ### Locally, the suites already use separate databases | Suite | Container | Port | Database | | --- | --- | --- | --- | | Integration | `redefined-designs-test-db` | 55432 | `redefined_test` (tmpfs, disposable) | | e2e backend | `redefined-designs-local-db` | 55500 | `redefined_local` | `scripts/start-local.ps1` takes its own container and database, and its port comment is explicit that 55500 is "deliberately not 55432". Corroborated by the data: `redefined_local` holds over a thousand accumulated items while the integration database is migrated from scratch on every run. So the truncation this issue describes does not happen locally any more. The comment in `admin-theme.spec.ts` about "the shared dev database" predates that split. ### In CI they genuinely do share one `.gitea/workflows/sonarqube.yml` sets both: ```yaml TEST_PGDATABASE: redefined_test # the integration suite PGDATABASE: redefined_test # the backend the e2e run drives ``` One database. The integration step truncates it, and the e2e run then starts against whatever it left. Sequential rather than concurrent, so nothing races — but the e2e suite inherits an empty database, which is exactly the state `admin-theme.spec.ts` defends against. **This is the whole of the remaining problem, and it is CI-only.** ### The two success criteria are not the same problem > `email-templates.spec.ts` no longer needs `mode: 'serial'` That one is not about the integration suite at all. The spec explains itself: the templates live in `admin_settings`, which is global per database, and **the e2e workers race each other** over it. The integration suite is not running at the time. Making it parallel needs a database per Playwright worker — a much larger change than this issue describes, and one worth its own decision rather than being smuggled in here. > `admin-theme.spec.ts` no longer needs its defensive seed That one does follow from the CI fix, though it is worth asking whether it should. A test seeding what it needs is ordinarily good practice, not a workaround; the objection here is only that it was forced by another suite's truncation. ### Why this is not being fixed right now The remaining fix is a CI-only change — give the e2e backend its own database in the workflow — and **CI is currently broken and parked under #154**, whose cause is not yet understood. Changing the database wiring of the very subsystem that is misbehaving, with no way to verify the result, would be guessing. Proposed sequence: settle #154 first, then make this change and confirm it in a green run. ### Revised scope, for whenever it is picked up - Give the e2e backend its own `PGDATABASE` in `sonarqube.yml`, created and migrated alongside the integration one - Re-evaluate `admin-theme.spec.ts`'s defensive seed afterwards rather than assuming it must go - Split the `email-templates` serial-mode question out into its own issue, since per-worker databases is a different piece of work with a different cost
bermudalamb commented 2026-09-02 18:27:27 -05:00
Author
Owner
Copy Link
Copy Source

Fixed, though not in the shape this issue expected — and its verification criterion turned out to be wrong, so that is worth recording rather than quietly ignoring.

What was done

The first option listed here: give e2e its own database, separate from the one integration truncates. start-local.ps1 -E2eDb runs the stack against redefined-designs-e2e-db on 55501 with tmpfs storage, so it starts empty every run and the integration suite on 55432 cannot touch it (#186). Different container, different port, different credentials — the mistake is impossible rather than discouraged.

Two things came out of doing it that were not visible from here:

The e2e helper was reading a different database than the app. support/db.ts opens its own connection for password-reset tokens, and nothing tied it to the stack's choice — so -E2eDb moved the application while the helper kept defaulting to redefined_local. -Suite all was worse and had been for longer: the integration suite sets TEST_PGPORT and PowerShell keeps it, so the e2e run inherited the integration port with redefined_local credentials, which cannot connect to anything. start-local.ps1 now records what it used in .local/database.json and run-tests.ps1 reads it (#273).

A stale backend can outlive the database it was serving. start-local.ps1 would say "something is already listening on 3000; leaving it alone" and carry on, so a recreated database left the old process serving rows that no longer existed. That was the actual cause of the intermittent resend-verification failure — recycled customer ids inheriting stale in-memory rate-limit buckets (#257).

The verification criterion here is wrong now

This issue says the check is that "admin-theme.spec.ts no longer needs its defensive seed". That has been overtaken. The seed existed because the shared database was truncated underneath the run; that is fixed — but the e2e database now starts empty on every run, so there is no ambient data at all and the seed is needed more than before, not less. Its comment has been corrected to say so, and the seed stays. Seeding what a test needs is the right habit either way.

email-templates.spec.ts keeps mode: 'serial' deliberately. It guards genuinely shared state in admin_settings, and it is only redundant because workers: 1 (#241). Removing it would make that spec silently depend on the worker count.

So both workarounds remain, and the underlying problem is still fixed. The criterion tested the wrong thing.

What is not covered

Namespacing the global state in admin_settings, the third option listed. Not needed: resetDb clears the email_ and intake_ rows between tests, and the suite is serial. Worth revisiting only if the worker count ever goes up.

Closes #116

Fixed, though not in the shape this issue expected — and its verification criterion turned out to be wrong, so that is worth recording rather than quietly ignoring. ## What was done The first option listed here: **give e2e its own database, separate from the one integration truncates**. `start-local.ps1 -E2eDb` runs the stack against `redefined-designs-e2e-db` on 55501 with tmpfs storage, so it starts empty every run and the integration suite on 55432 cannot touch it (#186). Different container, different port, different credentials — the mistake is impossible rather than discouraged. Two things came out of doing it that were not visible from here: **The e2e helper was reading a different database than the app.** `support/db.ts` opens its own connection for password-reset tokens, and nothing tied it to the stack's choice — so `-E2eDb` moved the application while the helper kept defaulting to `redefined_local`. `-Suite all` was worse and had been for longer: the integration suite sets `TEST_PGPORT` and PowerShell keeps it, so the e2e run inherited the integration port with `redefined_local` credentials, which cannot connect to anything. `start-local.ps1` now records what it used in `.local/database.json` and `run-tests.ps1` reads it (#273). **A stale backend can outlive the database it was serving.** `start-local.ps1` would say "something is already listening on 3000; leaving it alone" and carry on, so a recreated database left the old process serving rows that no longer existed. That was the actual cause of the intermittent resend-verification failure — recycled customer ids inheriting stale in-memory rate-limit buckets (#257). ## The verification criterion here is wrong now This issue says the check is that *"`admin-theme.spec.ts` no longer needs its defensive seed"*. That has been overtaken. The seed existed because the shared database was truncated underneath the run; that is fixed — but the e2e database now **starts empty on every run**, so there is no ambient data at all and the seed is needed *more* than before, not less. Its comment has been corrected to say so, and the seed stays. Seeding what a test needs is the right habit either way. `email-templates.spec.ts` keeps `mode: 'serial'` deliberately. It guards genuinely shared state in `admin_settings`, and it is only redundant because `workers: 1` (#241). Removing it would make that spec silently depend on the worker count. So both workarounds remain, and the underlying problem is still fixed. The criterion tested the wrong thing. ## What is not covered Namespacing the global state in `admin_settings`, the third option listed. Not needed: `resetDb` clears the `email_` and `intake_` rows between tests, and the suite is serial. Worth revisiting only if the worker count ever goes up. Closes #116
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
No items
No labels
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Code Quality and Hardening 2
Review
Assignees
Clear assignees
calamb (Chelsea Lamb) bermudalamb (Thom Lamb)
No Assignees bermudalamb
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: bermudalamb/redefined-designs#116