Merge branch 'main' into feature/223-drafting-worker

frontend/playwright.config.ts

@@ -2,7 +2,28 @@ import { defineConfig, devices } from '@playwright/test';
 
 export default defineConfig({
   testDir: './tests/e2e',
+  // Kept true so a file's tests are independent units, but with one worker it
+  // no longer interleaves them. See `workers` below.
   fullyParallel: true,
+  // One worker, because the suite shares a single database.
+  //
+  // This was the lever #241 deliberately held back, to see first whether better
+  // failure messages and a wider assertion timeout were enough. They were not.
+  // Measured on this branch, same commit, same machine: parallel runs failed 3
+  // of 155 twice over, and the three were not the same three — password-reset
+  // and admin-inventory-filters in one run, admin-email-settings,
+  // admin-inventory-filters and resend-verification in the next. Run serially,
+  // all 16 of those same tests passed; the full suite then passed 155 of 155
+  // twice in a row.
+  //
+  // So the failures were contention, not defects: specs asserting over tables
+  // that other specs were concurrently writing to. The cost is about 3 minutes
+  // — roughly 1 minute parallel against 3.9 and 4.0 serially — and a red run
+  // that means something is worth three minutes.
+  //
+  // The cheaper fix, if that time is ever needed back, is giving each worker
+  // its own database rather than raising this number and reopening #241.
+  workers: 1,
   retries: process.env.CI ? 1 : 0,
   reporter: [['list']],
   use: {
@@ -14,6 +35,17 @@ export default defineConfig({
     // that was plainly visible and enabled.
     reducedMotion: 'reduce'
   },
+  // Playwright's default is 5s, and nothing overrode it. That is generous on an
+  // idle laptop and tight on this runner: a full CI pass takes around 21.5
+  // minutes on a single machine that also builds, migrates and runs three other
+  // suites, and the #239 failure reported exactly `Timeout: 5000ms`.
+  //
+  // Costs nothing on a green run. This bounds how long a *failing* assertion
+  // waits before giving up, not how long a passing one takes — a locator that
+  // resolves in 200ms still resolves in 200ms.
+  expect: {
+    timeout: 10000
+  },
   webServer: {
     command: 'npm run dev',
     url: 'http://localhost:5173',

frontend/tests/e2e/admin-disable-customer.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect, createItem, uniqueSuffix } from './fixtures';
+import { test, expect, createItem, uniqueSuffix, findOrFail } from './fixtures';
 
 test.describe('Disabling a customer account', () => {
   test('an admin can disable an account and the customer is told at sign-in', async ({
@@ -34,7 +34,11 @@ test.describe('Disabling a customer account', () => {
     await expect(accountModal.emailText(customer.email)).toBeVisible();
 
     const customers = await (await request.get('/api/admin/customers')).json();
-    const id = customers.find((c: { email: string }) => c.email === customer.email).id;
+    const id = findOrFail(
+      customers,
+      (c: { email: string }) => c.email === customer.email,
+      `the customer ${customer.email}`
+    ).id;
     await request.post(`/api/admin/customers/${id}/disable`);
 
     // The cookie is unchanged, so this proves the server rejects it rather
@@ -53,7 +57,11 @@ test.describe('Disabling a customer account', () => {
     header
   }) => {
     const customers = await (await request.get('/api/admin/customers')).json();
-    const id = customers.find((c: { email: string }) => c.email === customer.email).id;
+    const id = findOrFail(
+      customers,
+      (c: { email: string }) => c.email === customer.email,
+      `the customer ${customer.email}`
+    ).id;
     await request.post(`/api/admin/customers/${id}/disable`);
 
     await admin.open('Customers');

frontend/tests/e2e/admin-inline-category.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect, uniqueSuffix, createCategory } from './fixtures';
+import { test, expect, uniqueSuffix, createCategory, findOrFail } from './fixtures';
 
 test.describe('Inline category creation from the item form', () => {
   test('creates a category without leaving the item form and assigns it', async ({
@@ -32,8 +32,13 @@ test.describe('Inline category creation from the item form', () => {
     await expect(page.getByText('Item added')).toBeVisible();
 
     const items = await (await page.request.get('/api/admin/items')).json();
-    const saved = items.find((item: { name: string }) => item.name === itemName);
+    // findOrFail already fails with the row count when the item is missing,
-    expect(saved).toBeTruthy();
+    // which the toBeTruthy assertion this replaces could not report.
+    const saved = findOrFail(
+      items,
+      (item: { name: string }) => item.name === itemName,
+      `the item named ${itemName}`
+    );
     expect(saved.category_name).toBe(categoryName);
   });
 

frontend/tests/e2e/admin-save-failures.spec.ts

@@ -59,23 +59,11 @@ test.describe('Admin save failures', () => {
     await expect(adminInventory.row(name)).toBeVisible();
   });
 
-  // SKIPPED, temporarily, to get main green while #241 is outstanding. See #245
+  // Un-skipped now that #241 has landed (was skipped under #245). This is the
-  // before deleting this comment or the skip.
+  // only end-to-end check that adding an item actually reaches the database
-  //
+  // rather than just firing a toast — the happy path of the core admin action —
-  // It fails in CI and passes locally, and which test fails moves around: a
+  // so it is worth having back.
-  // local parallel run of the whole suite on the same commit failed four
+  test('saves an item successfully when the server accepts it', async ({
-  // *different* specs and not this one. That is #241 — fullyParallel against a
-  // single shared database — so fixing this test on its own would be guessing at
-  // a symptom that will simply reappear somewhere else.
-  //
-  // Not the image work from #226: addItem fills a name and a price and saves,
-  // attaching nothing, so the re-encoding path is never entered here.
-  //
-  // What this stops covering is not trivial. It is the only end-to-end check
-  // that adding an item actually reaches the database rather than just firing a
-  // toast — the happy path of the core admin action. Un-skip it as soon as #241
-  // lands; if it still fails then, it is a real defect and worth chasing.
-  test.skip('saves an item successfully when the server accepts it', async ({
     page,
     admin,
     adminInventory

frontend/tests/e2e/admin-taxonomy.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect, uniqueSuffix } from './fixtures';
+import { test, expect, uniqueSuffix, findOrFail } from './fixtures';
 
 // The e2e database is shared and never reset, so every fixture name carries a
 // unique suffix and assertions are scoped to the nodes this run created. The
@@ -41,8 +41,11 @@ test.describe('Admin taxonomy', () => {
     // The table paginates and the shared database holds many tags, so the new
     // row is confirmed through the API rather than hunted for across pages.
     const tags = await (await page.request.get('/api/admin/tags')).json();
-    const created = tags.find((tag: { name: string }) => tag.name === `vintage-${RUN}`);
+    const created = findOrFail(
-    expect(created).toBeTruthy();
+      tags,
+      (tag: { name: string }) => tag.name === `vintage-${RUN}`,
+      `the tag vintage-${RUN}`
+    );
     expect(created.color).toBeTruthy();
   });
 

frontend/tests/e2e/email-templates.spec.ts

@@ -1,4 +1,4 @@
-import { test, expect, uniqueSuffix } from './fixtures';
+import { test, expect, uniqueSuffix, findOrFail } from './fixtures';
 
 // Each test leaves the templates as it found them, because they are stored in
 // admin_settings and would otherwise change the copy a later test reads.
@@ -51,7 +51,11 @@ test.describe('Editing the customer emails', () => {
 
     // Persisted, not merely accepted by the form.
     const stored = await (await page.request.get('/api/admin/email-templates')).json();
-    const reset = stored.find((t: { key: string }) => t.key === 'passwordReset');
+    const reset = findOrFail(
+      stored,
+      (t: { key: string }) => t.key === 'passwordReset',
+      'the passwordReset template'
+    );
     expect(reset.subject).toBe(subject);
   });
 
@@ -69,7 +73,11 @@ test.describe('Editing the customer emails', () => {
 
     // And nothing was stored.
     const stored = await (await page.request.get('/api/admin/email-templates')).json();
-    const reset = stored.find((t: { key: string }) => t.key === 'passwordReset');
+    const reset = findOrFail(
+      stored,
+      (t: { key: string }) => t.key === 'passwordReset',
+      'the passwordReset template'
+    );
     expect(reset.body).toBeNull();
   });
 
@@ -88,7 +96,11 @@ test.describe('Editing the customer emails', () => {
     await expect(page.getByText('Password reset restored to the default')).toBeVisible();
 
     const stored = await (await page.request.get('/api/admin/email-templates')).json();
-    const reset = stored.find((t: { key: string }) => t.key === 'passwordReset');
+    const reset = findOrFail(
+      stored,
+      (t: { key: string }) => t.key === 'passwordReset',
+      'the passwordReset template'
+    );
     expect(reset.subject).toBeNull();
     expect(reset.body).toBeNull();
   });
@@ -113,7 +125,11 @@ test.describe('Previewing the customer emails', () => {
     await expect(adminEmails.preview('Password reset').getByText(wording)).toBeVisible();
 
     const stored = await (await page.request.get('/api/admin/email-templates')).json();
-    expect(stored.find((t: { key: string }) => t.key === 'passwordReset').body).toBeNull();
+    expect(findOrFail(
+      stored,
+      (t: { key: string }) => t.key === 'passwordReset',
+      'the passwordReset template'
+    ).body).toBeNull();
   });
 
   test('substitutes sample values rather than showing raw placeholders', async ({ page, admin, adminEmails }) => {

frontend/tests/e2e/favorites-filter.spec.ts

@@ -8,7 +8,8 @@ import {
   uniqueEmail,
   PASSWORD,
   StorefrontPage,
-  FavoritePrompt
+  FavoritePrompt,
+  findOrFail
 } from './fixtures';
 
 // The storefront runs against a shared database that is never reset, so every
@@ -178,7 +179,11 @@ test.describe('Filtering the storefront by favorites', () => {
 
     const api = await createAdminContext(playwright);
     const items = await (await api.get('/api/items')).json();
-    const sells = items.find((item: { name: string }) => item.name === SELLS);
+    const sells = findOrFail(
+      items,
+      (item: { name: string }) => item.name === SELLS,
+      `the item named ${SELLS}`
+    );
     await sellItem(api, sells.id);
     await api.dispose();