From 74936e6d6a142aa7dcdaf5ce319f08579040955d Mon Sep 17 00:00:00 2001 From: Thom Lamb Date: Tue, 1 Sep 2026 09:14:20 -0500 Subject: [PATCH] fix(e2e): match the modal OK button exactly so a random suffix cannot collide (#253) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Playwright matches an accessible name case-insensitively and as a substring unless exact is passed, so `getByRole('button', { name: 'OK' })` matched any button whose name merely contained "ok". Caught on a full serial run of main: strict mode violation: getByRole('button', { name: 'OK' }) resolved to 2 elements: 1) aka getByRole('button', { name: 'Freed rmtiq9okg22k9e' }) 2) aka getByRole('button', { name: 'OK', exact: true }) A leftover "Freed …" toast was still on screen and the random base36 suffix happened to contain "ok". Roughly one suffix in a few hundred does, which is the profile of a test that fails occasionally and reproduces for nobody. This is not the contention #241 addressed. It reproduced with workers: 1, serially, on a fresh database — it needs only a stale toast and an unlucky suffix. It is at least part of what #245 skipped a real test to work around. Also makes the taxonomy modal's OK and the sold-filter 'All' radio exact, the only other targets short enough to appear inside a random suffix. The dozen or so remaining short names are left alone deliberately: they would need a collision in deliberately-chosen test data rather than a random one, and substring matching is load-bearing in some of them. Verified with two consecutive full e2e passes, 155 of 155 both times. Closes #253 Co-Authored-By: Claude Opus 5 --- frontend/tests/e2e/pages/AdminInventory.ts | 7 ++++++- frontend/tests/e2e/pages/AdminTaxonomy.ts | 3 ++- frontend/tests/e2e/sold-filter.spec.ts | 3 ++- 3 files changed, 10 insertions(+), 3 deletions(-) diff --git a/frontend/tests/e2e/pages/AdminInventory.ts b/frontend/tests/e2e/pages/AdminInventory.ts index 6325a83..0410b80 100644 --- a/frontend/tests/e2e/pages/AdminInventory.ts +++ b/frontend/tests/e2e/pages/AdminInventory.ts @@ -27,7 +27,12 @@ export class AdminInventory { this.price = page.getByLabel('Price (USD)'); this.category = page.getByLabel('Category', { exact: true }); this.saveButton = page.getByRole('button', { name: 'Save', exact: true }); - this.confirmButton = page.getByRole('button', { name: 'OK' }); + // exact, because Playwright matches an accessible name case-insensitively + // as a *substring* by default. Item names carry a random base36 suffix, and + // one containing "ok" — plus any leftover toast still on screen, such as + // "Freed rmtiq9okg22k9e" — makes this resolve to two buttons and fail as a + // strict mode violation. See #253. + this.confirmButton = page.getByRole('button', { name: 'OK', exact: true }); this.formDialog = page.getByRole('dialog'); this.tagPicker = page.getByText('Pick existing tags'); } diff --git a/frontend/tests/e2e/pages/AdminTaxonomy.ts b/frontend/tests/e2e/pages/AdminTaxonomy.ts index d729ac4..5f60a78 100644 --- a/frontend/tests/e2e/pages/AdminTaxonomy.ts +++ b/frontend/tests/e2e/pages/AdminTaxonomy.ts @@ -21,7 +21,8 @@ export class AdminTaxonomy { this.addCategoryButton = page.getByRole('button', { name: 'Add Category' }); this.addTagButton = page.getByRole('button', { name: 'Add Tag' }); this.name = page.getByLabel('Name'); - this.confirmButton = page.getByRole('button', { name: 'OK' }); + // Substring matching would collide with random suffixes. See #253. + this.confirmButton = page.getByRole('button', { name: 'OK', exact: true }); this.tagAddedNotice = page.getByText('Tag added'); } diff --git a/frontend/tests/e2e/sold-filter.spec.ts b/frontend/tests/e2e/sold-filter.spec.ts index 16b1a88..c6959a3 100644 --- a/frontend/tests/e2e/sold-filter.spec.ts +++ b/frontend/tests/e2e/sold-filter.spec.ts @@ -59,7 +59,8 @@ test.describe('Filtering the storefront by availability', () => { await page.reload(); await expect(storefront.gridCell(NAMES.sold)).toBeVisible(); - await expect(page.getByRole('radio', { name: 'All' })).toBeChecked(); + // exact: 'All' is short enough to appear inside other labels. See #253. + await expect(page.getByRole('radio', { name: 'All', exact: true })).toBeChecked(); }); // Not sold is the default, so it is held as "no preference" rather than as an -- 2.54.0