Work on removing flakiness from E2E test suite, renumbering to remove emails category

Author: webstackdev

_TODO.md

@@ -56,3 +56,33 @@ Right now we're using string literals to define HTML email templates for site ma
 Vercel AI Gateway, maybe could use for a chatbot:
 
 https://vercel.com/kevin-browns-projects-dd474f73/astro-webstackbuilders-com/ai-gateway
+
+1. Firefox/WebKit locator.setChecked(...) “did not change its state”
+
+With your markup, the checkbox is sr-only and the visual switch is a separate element (peer-based). In Firefox/WebKit, a forced click on a clipped/visually-hidden checkbox can “click” without producing the native toggle behavior Playwright expects (or the component’s JS is not wired to the checkbox click in a way that actually flips the DOM checked state).
+
+Most deterministic alternatives (conceptually):
+
+- Don’t uncheck via the hidden input. Use the “Deny All” control (it exists as #consent-deny-all), then click “Allow All”, then assert all are checked. This tests the same behavior (“Allow All enables every category”) without the fragile intermediate toggle interaction.
+
+- If you truly need to uncheck just one category, do it by setting state via JS (checked = false + dispatch input/change) because that bypasses the click/geometry issues and targets the state machine the component is likely listening to.
+
+2. mobile-safari consent banner: waitForPageLoad() still times out (15s)
+
+Important observation: in consentBanner.spec.ts, you already wait on a much more direct readiness signal right after waitForPageLoad():
+
+consent-banner.isInitialized === true
+So the likely story here is: waitForPageLoad() is unnecessary for these tests, and when it flakes it blocks the entire suite.
+
+3. mobile-safari store persistence beforeEach timeouts + Edge footer page.goto('/') timeouts
+
+These are pure navigation-level failures (timeouts in page.goto or in a beforeEach that presumably can’t get through its startup navigation). The usual causes are:
+
+a global readiness wait that never resolves (which can cascade into beforeEach timing out)
+
+- Two questions so we pick the right fixes
+
+For the consent “Allow All enables every category” test: are you okay with changing the test to use #consent-deny-all instead of unchecking a single toggle (still verifies Allow All enables everything, but avoids the hidden-switch interaction entirely)?
+
+For the navigation timeouts (mobile-safari + Edge): were these runs executed while npm run dev was definitely running and stable, or is it possible the server wasn’t up / got interrupted?
+

playwright.config.ts

@@ -35,7 +35,7 @@ const buildWebServerEnv = () => {
   return env
 }
 
-const workersHighParallel = process.env['GITHUB_ACTIONS'] ? 1 : '75%'
+const workersHighParallel = process.env['GITHUB_ACTIONS'] ? 1 : 2
 const testMatchHighParallel = '**/*.spec.ts'
 const testIgnoreHighParallel = [
   '03-forms/**/*.spec.ts',
@@ -66,7 +66,7 @@ export default defineConfig({
   forbidOnly: !!process.env['GITHUB_ACTIONS'],
   /** Retry on CI only */
   retries: process.env['GITHUB_ACTIONS'] ? 2 : 0,
-  /** Opt out of parallel tests on CI. */
+  /** Opt out of high parallelism in CI-mode runs. */
   workers: workersHighParallel,
   /** Only run @ready tests in CI, all tests locally */
   ...(process.env['GITHUB_ACTIONS'] ? { grep: /@ready/ } : {}),

test/e2e/db/helpers.ts

@@ -230,8 +230,24 @@ export const deleteDsarRequestById = async (id: string) => {
 
 export const deleteConsentRecordsBySubjectId = async (dataSubjectId: string) => {
   const libsql = getLibsqlClient()
-  await libsql.execute({
-    sql: `DELETE FROM consentEvents WHERE dataSubjectId = ?`,
-    args: [dataSubjectId],
-  })
+
+  const maxAttempts = 5
+  for (let attempt = 1; attempt <= maxAttempts; attempt++) {
+    try {
+      await libsql.execute({
+        sql: `DELETE FROM consentEvents WHERE dataSubjectId = ?`,
+        args: [dataSubjectId],
+      })
+      return
+    } catch (error) {
+      const message = error instanceof Error ? error.message : String(error)
+      const isBusy = message.includes('SQLITE_BUSY') || message.toLowerCase().includes('database is locked')
+      if (!isBusy || attempt === maxAttempts) {
+        throw error
+      }
+
+      // Yield briefly to let another worker release its write lock.
+      await new Promise<void>((resolve) => setTimeout(resolve, 50 * attempt))
+    }
+  }
 }

test/e2e/helpers/pageObjectModels/BasePage.ts

@@ -130,6 +130,27 @@ export class BasePage extends BuiltInsPage {
         window.__astroPageLoadCounter = 0
       }
 
+      // `astro:page-load` is primarily a View Transitions signal.
+      // Some browsers/environments may not reliably dispatch it on a fresh load.
+      // We also bump the counter on full document loads so `waitForPageLoad()` works
+      // for both navigation modes.
+      let fullLoadCountedForDocument = false
+      const bumpFullLoadCounter = () => {
+        if (fullLoadCountedForDocument) return
+        fullLoadCountedForDocument = true
+        window.__astroPageLoadCounter = (window.__astroPageLoadCounter ?? 0) + 1
+      }
+
+      if (typeof window !== 'undefined') {
+        window.addEventListener('load', bumpFullLoadCounter, { passive: true })
+        // Covers BFCache restores where `load` may not fire.
+        window.addEventListener('pageshow', bumpFullLoadCounter, { passive: true })
+      }
+
+      if (typeof document !== 'undefined' && document.readyState === 'complete') {
+        bumpFullLoadCounter()
+      }
+
       if (!window.__astroPageLoadListenerAttached) {
         const registerAstroPageLoadListener = () => {
           if (window.__astroPageLoadListenerAttached) return
@@ -305,7 +326,7 @@ export class BasePage extends BuiltInsPage {
    */
   async waitForPageLoad(options?: { requireNext?: boolean; timeout?: number }): Promise<void> {
     const requireNext = options?.requireNext ?? false
-    const timeout = options?.timeout ?? wait.defaultWait
+    const timeout = options?.timeout ?? wait.navigation
     const currentCount = await this._page.evaluate(() => window.__astroPageLoadCounter ?? 0)
 
     if (!requireNext && currentCount > this.lastAstroPageLoadCount) {
@@ -435,6 +456,14 @@ export class BasePage extends BuiltInsPage {
     await expect(this._page.locator('h1').first()).toBeVisible()
   }
 
+  /**
+   * Verify homepage hero section is present and visible
+   */
+  async expectHeroSection(): Promise<void> {
+    await expect(this._page.locator('section[aria-labelledby="home-hero-title"]')).toBeVisible()
+    await expect(this._page.locator('#home-hero-title')).toBeVisible()
+  }
+
   /**
    * Check if page has specific heading
    */

test/e2e/helpers/pageObjectModels/BuiltInsPage.ts

@@ -40,7 +40,7 @@ export class BuiltInsPage {
    * via `afterGoto` (without re-implementing retry logic everywhere).
    */
   async goto(path: string, options?: BuiltInsGotoOptions): Promise<null | Response> {
-    const requestedTimeout = options?.timeout ?? wait.defaultWait
+    const requestedTimeout = options?.timeout ?? wait.navigation
     const waitUntil = options?.waitUntil ?? 'domcontentloaded'
 
     const navigate = async (timeout: number) => {
@@ -58,7 +58,7 @@ export class BuiltInsPage {
       const message = error instanceof Error ? error.message : String(error)
       if (message.includes('ERR_ABORTED')) {
         response = await navigate(requestedTimeout)
-      } else if (!options?.timeout && message.includes('Timeout')) {
+      } else if (!options?.timeout && message.includes('Timeout') && requestedTimeout < wait.navigation) {
         // Allow a single retry with a longer timeout to absorb slow prerender navigations
         response = await navigate(wait.navigation)
       } else {

test/e2e/helpers/pageObjectModels/NewsletterPage.ts

@@ -4,6 +4,7 @@
  */
 import { type Page, expect } from '@playwright/test'
 import { BasePage } from '@test/e2e/helpers'
+import { wait } from '@test/e2e/helpers/waitTimeouts'
 
 export class NewsletterPage extends BasePage {
   private readonly subscribeActionEndpoint = '/_actions/newsletter/subscribe'
@@ -33,7 +34,7 @@ export class NewsletterPage extends BasePage {
    */
   async navigateToNewsletterForm(): Promise<void> {
     await this.goto(this.fixturePath)
-    await this.waitForLoadState('networkidle') // Ensure all scripts are loaded
+    await this.page.waitForSelector(this.formSelector, { state: 'visible', timeout: wait.navigation })
     await this.expectNewsletterForm()
   }
 
@@ -133,6 +134,18 @@ export class NewsletterPage extends BasePage {
     await expect(this.page.locator(this.messageSelector)).toContainText(text)
   }
 
+  /**
+   * Assert the newsletter form reached a successful confirmation state.
+   * Copy can vary; this checks semantics and avoids coupling tests to exact wording.
+   */
+  async expectSuccessConfirmation(): Promise<void> {
+    const message = this.page.locator(this.messageSelector)
+
+    await expect(message).toBeVisible({ timeout: wait.defaultWait })
+    await expect(message).not.toContainText(/sending confirmation email/i, { timeout: wait.polling })
+    await expect(message).toContainText(/check your email|confirmation email|confirm your subscription/i)
+  }
+
   /**
    * Verify loading spinner is visible
    */
@@ -150,7 +163,7 @@ export class NewsletterPage extends BasePage {
   /**
    * Wait for the spinner to enter the loading state at least once
    */
-  async waitForSpinnerLoadingState(timeout = 2000): Promise<void> {
+  async waitForSpinnerLoadingState(timeout = wait.quickAssert): Promise<void> {
     await this.page.waitForFunction(
       selector => {
         const spinner = document.querySelector(selector)

test/e2e/specs/03-forms/newsletter-subscription.spec.ts

@@ -21,8 +21,8 @@ test.describe('Newsletter Subscription Form', () => {
     await newsletterPage.checkGdprConsent()
     await newsletterPage.submitForm()
 
-    // Should show confirmation message (using partial match to handle variations)
-    await newsletterPage.expectMessageContains('check your email')
+    // Should show confirmation message (wording varies)
+    await newsletterPage.expectSuccessConfirmation()
   })
 
   test('@ready form rejects invalid email format', async ({ page: playwrightPage }) => {
@@ -119,8 +119,7 @@ test.describe('Newsletter Subscription Form', () => {
     await newsletterPage.checkGdprConsent()
     await newsletterPage.submitForm()
 
-    // Wait for success message (updated to match actual API response)
-    await newsletterPage.expectMessageContains('Please check your email to confirm your subscription')
+    await newsletterPage.expectSuccessConfirmation()
 
     // Verify form is cleared
     await newsletterPage.expectFormReset()
@@ -167,7 +166,7 @@ test.describe('Newsletter Subscription Form', () => {
     await newsletterPage.checkGdprConsent()
     await newsletterPage.submitForm()
 
-    await newsletterPage.expectMessageContains('check your email')
+    await newsletterPage.expectSuccessConfirmation()
   })
 
   test('@ready API error preserves form state and surfaces message', async ({ page: playwrightPage }) => {

test/e2e/specs/04-components/breadcrumbs.spec.ts

@@ -9,6 +9,7 @@ import {
   expect,
   test,
 } from '@test/e2e/helpers'
+import { wait } from '@test/e2e/helpers/waitTimeouts'
 
 test.describe('Breadcrumbs Component', () => {
   test('@ready breadcrumbs display on article pages', async ({ page: playwrightPage }) => {
@@ -48,8 +49,25 @@ test.describe('Breadcrumbs Component', () => {
     const page = await BreadCrumbPage.init(playwrightPage)
     await page.openFirstArticleDetail()
 
-    await page.click('nav[aria-label="Breadcrumbs"] a')
-    await page.waitForLoadState('networkidle')
+    const supportsViewTransitions = await page.evaluate(() => {
+      if (typeof document === 'undefined') {
+        return false
+      }
+      return typeof document.startViewTransition === 'function'
+    })
+
+    if (supportsViewTransitions) {
+      const waitForLoad = page.waitForPageLoad({ requireNext: true, timeout: wait.navigation })
+      await page.click('nav[aria-label="Breadcrumbs"] a')
+      await waitForLoad
+    } else {
+      const navigationPromise = page.page.waitForURL((url: URL) => url.pathname === '/', {
+        waitUntil: 'domcontentloaded',
+        timeout: wait.navigation,
+      })
+      await page.click('nav[aria-label="Breadcrumbs"] a')
+      await navigationPromise
+    }
 
     await page.expectUrlContains('localhost:4321/')
   })

test/e2e/specs/04-components/consentBanner.spec.ts

@@ -88,16 +88,16 @@ test.describe('Consent Banner', () => {
    */
   test.beforeEach(async ({ page: playwrightPage, context }) => {
     const page = await BasePage.init(playwrightPage)
-    // Clear all cookies and storage before navigation
+    // Clear cookies first so our initial origin visit is clean
     await context.clearCookies()
 
-    // Navigate to page without auto-dismissing the consent banner
+    // Establish origin so we can clear localStorage/sessionStorage deterministically
     await page.goto('/', { skipCookieDismiss: true, timeout: wait.navigation })
     await resetConsentState(page)
+    await context.clearCookies()
 
-    // Reload so the modal logic re-runs with a clean browser state across all engines
-    await page.reload({ waitUntil: 'domcontentloaded' })
-    await page.waitForLoadState('networkidle')
+    // Fresh navigation so consent logic runs with a fully reset state
+    await page.goto('/', { skipCookieDismiss: true, timeout: wait.navigation })
     await page.waitForPageLoad()
 
     // Wait for consent banner custom element to be initialized
@@ -148,7 +148,6 @@ test.describe('Consent Banner', () => {
     await acceptAllCookies(page)
 
     await page.reload({ waitUntil: 'domcontentloaded' })
-    await page.waitForLoadState('networkidle')
     await removeViteErrorOverlay(page)
 
     await expect(cookieBanner).toBeHidden()
@@ -161,7 +160,6 @@ test.describe('Consent Banner', () => {
     await acceptAllCookies(page)
 
     await page.goto('/about', { timeout: wait.navigation })
-    await page.waitForLoadState('networkidle')
     await removeViteErrorOverlay(page)
 
     await expect(cookieBanner).toBeHidden()

test/e2e/specs/04-components/consentPreferences.spec.ts

@@ -17,6 +17,24 @@ const CONSENT_PAGE_PATH = '/consent'
 
 const toggleLabel = (checkboxId: string): string => `[data-consent-toggle="${checkboxId}"]`
 
+async function toggleCheckboxViaKeyboard(page: BasePage, checkboxId: string): Promise<void> {
+  const selector = `#${checkboxId}`
+  const checkbox = page.locator(selector)
+
+  await checkbox.scrollIntoViewIfNeeded()
+  await checkbox.focus()
+
+  await expect
+    .poll(async () => {
+      return await page.evaluate((id: string) => {
+        return document.activeElement?.id === id
+      }, checkboxId)
+    })
+    .toBe(true)
+
+  await page.keyboard.press('Space')
+}
+
 const decodeAstroActionJson = (raw: unknown): unknown => {
   if (!Array.isArray(raw) || raw.length === 0) {
     return raw
@@ -103,7 +121,6 @@ test.describe('Consent Preferences Component', () => {
 
     await context.clearCookies()
     await page.goto(CONSENT_PAGE_PATH, { timeout: wait.navigation })
-    await playwrightPage.waitForLoadState('networkidle')
     await removeViteErrorOverlay(page)
     await waitForConsentPreferences(page)
   })
@@ -212,9 +229,9 @@ test.describe('Consent Preferences Component', () => {
 
     await page.locator(ALLOW_ALL_BUTTON).click()
 
-    await page.locator(toggleLabel('analytics-cookies')).click()
-    await page.locator(toggleLabel('functional-cookies')).click()
-    await page.locator(toggleLabel('marketing-cookies')).click()
+    await toggleCheckboxViaKeyboard(page, 'analytics-cookies')
+    await toggleCheckboxViaKeyboard(page, 'functional-cookies')
+    await toggleCheckboxViaKeyboard(page, 'marketing-cookies')
 
     await expect(analyticsCheckbox).not.toBeChecked()
     await expect(functionalCheckbox).not.toBeChecked()
@@ -235,7 +252,10 @@ test.describe('Consent Preferences Component', () => {
     const marketingCheckbox = page.locator('#marketing-cookies')
 
     await page.locator(ALLOW_ALL_BUTTON).click()
-    await page.locator(toggleLabel('functional-cookies')).click()
+
+    // Toggle via keyboard to mirror accessible interaction and avoid browser differences
+    // around clicking sr-only inputs.
+    await toggleCheckboxViaKeyboard(page, 'functional-cookies')
     await expect(functionalCheckbox).not.toBeChecked()
 
     await page.locator(ALLOW_ALL_BUTTON).click()
@@ -245,4 +265,24 @@ test.describe('Consent Preferences Component', () => {
     await expect(marketingCheckbox).toBeChecked()
 
   })
+
+  test('@ready label click toggles functional switch (chromium only)', async ({ page: playwrightPage }, testInfo) => {
+    if (testInfo.project.name !== 'chromium') {
+      test.skip()
+    }
+
+    const page = await BasePage.init(playwrightPage)
+    await waitForConsentPreferences(page)
+
+    const functionalCheckbox = page.locator('#functional-cookies')
+
+    await page.locator(ALLOW_ALL_BUTTON).click()
+    await expect(functionalCheckbox).toBeChecked()
+
+    await page.locator(toggleLabel('functional-cookies')).click()
+    await expect(functionalCheckbox).not.toBeChecked()
+
+    await page.locator(toggleLabel('functional-cookies')).click()
+    await expect(functionalCheckbox).toBeChecked()
+  })
 })