diff --git a/frontend/tests/e2e/resend-verification.spec.ts b/frontend/tests/e2e/resend-verification.spec.ts index 6cba5d0..50e291f 100644 --- a/frontend/tests/e2e/resend-verification.spec.ts +++ b/frontend/tests/e2e/resend-verification.spec.ts @@ -22,24 +22,70 @@ test.describe('Resending your verification email', () => { // The message the customer gets on the fourth attempt is the point of the // limiter's copy: it says the mail probably did send and where to look, // rather than only that a limit exists. + // + // ## Why this asserts on responses rather than on what is on screen (#257) + // + // This test failed roughly one full-suite run in three, never in isolation, + // with a strict-mode violation: `getByText(/already sent several/)` resolved + // to three refusal toasts where one was expected. Two investigations could + // not establish why, and the second corrected the arithmetic the first + // relied on — antd toasts auto-dismiss, so **the number visible at the moment + // of an assertion is a lower bound on how many refusals happened, not a + // count**. Three visible refusals is equally consistent with four where the + // first had already faded. + // + // That makes toast-counting the wrong instrument twice over: it is + // timing-dependent, and `toBeVisible()` on a multi-match locator fails in + // strict mode even when the behaviour was correct. The responses are the + // behaviour; the toasts are a lossy rendering of it. + // + // So the sequence of statuses is recorded and asserted directly. If this + // fails again it now says which failure it is, which is exactly what #257 + // could not determine: `[429, 429, 429, 429]` means the customer's bucket + // already held hits before the test clicked anything, while more than four + // entries means the UI sent more requests than there were clicks. Either + // answer identifies the mechanism from a single failing run, rather than + // needing a temporary probe re-added and the suite run until it fails again. test('says something useful once the allowance runs out', async ({ page, customer, accountModal }) => { + const isResend = (url: string) => + new URL(url).pathname === '/api/customers/resend-verification'; + + // Recorded from the first navigation onward, deliberately: a request the + // test did not make is one of the explanations still open, and a recorder + // installed after the clicks could not see it. + const statuses: number[] = []; + page.on('response', (res) => { + if (isResend(res.url())) statuses.push(res.status()); + }); + await accountModal.open(); const resend = accountModal.resendVerificationButton; - for (let i = 0; i < 3; i++) { - await resend.click(); - await expect(resend).toBeEnabled(); + // Each click waits for its own response before the next. The previous + // `await expect(resend).toBeEnabled()` looked like pacing but was a no-op — + // the button is never disabled, so it returned immediately and left four + // requests racing. Waiting on the response removes concurrency as a + // variable, so a failure here cannot be explained by ordering. + for (let i = 0; i < 4; i++) { + await Promise.all([ + page.waitForResponse((res) => isResend(res.url())), + resend.click() + ]); } - await resend.click(); + // The allowance is three an hour, so three sends and one refusal. The + // message names the customer because a bucket carrying hits from somewhere + // else is the open question, and the address is what identifies whose. + expect(statuses, `resend responses for ${customer.email}`).toEqual([204, 204, 204, 429]); - // Matched on the phrase unique to the refusal. "spam folder" appears in the - // success message too, and the three stacked success toasts from the loop - // above are still on screen, so the looser match finds them instead. - await expect(page.getByText(/already sent several/)).toBeVisible(); + // The copy itself still matters — it is what this test is named for. Scoped + // with `.first()` because strict mode treats several identical toasts as + // ambiguous, and how many are still on screen is a timing detail this + // assertion should not depend on. + await expect(page.getByText(/already sent several/).first()).toBeVisible(); }); });