From d5c97fab5c2c82d37922abb8734438ec8c075f02 Mon Sep 17 00:00:00 2001 From: tiennm99 Date: Sun, 6 Sep 2026 19:51:16 +0700 Subject: [PATCH] test(e2e): cover the URL contract, and compile game routes before the race Adds coverage for this branch's behaviour: the homoglyph 404 through a real percent-encoded request, href assertions on both not-found exits (a visible link to nowhere is still a dead end), and the two halves of InlineScript's contract -- no console error where React renders the script on the client, and an executable type in the served markup everywhere else. That second one is asserted on raw HTML, with no browser, because in a browser the regression is invisible: ThemeToggle re-applies the theme on mount, so ends up correct whether or not the script ran -- just a flash later. Mutating the helper to always emit text/plain previously failed nothing. global-setup fixes a real flake. next dev compiles a route on first request, and 8 workers demanded the same cold compile at once, so every test navigating to a /game URL failed together at the 60s timeout -- reproducible by touching any source file. One serial warm-up pays the compile once: 28 passed in 14s. The fetch is bounded and its failure logged, because a skipped warm-up otherwise looks exactly like the flake it prevents. eslint ignores Playwright's artifact dirs, which are gitignored but were still walked, crashing a lint run concurrent with a test run. --- eslint.config.mjs | 11 ++++++- playwright.config.mjs | 3 ++ tests/e2e/global-setup.js | 38 ++++++++++++++++++++++ tests/e2e/routing.spec.js | 66 +++++++++++++++++++++++++++++++++++++-- 4 files changed, 114 insertions(+), 4 deletions(-) create mode 100644 tests/e2e/global-setup.js diff --git a/eslint.config.mjs b/eslint.config.mjs index 51d0094..69f7122 100644 --- a/eslint.config.mjs +++ b/eslint.config.mjs @@ -5,7 +5,16 @@ import globals from "globals"; // FlatCompat bridge (@eslint/eslintrc) is gone. const eslintConfig = [ { - ignores: [".next/**", ".next-check/**", "src/data/**"], + // Playwright's artifact dirs are gitignored but eslint walks them anyway, + // and a lint run concurrent with a test run crashes on the files the + // reporter is still writing (ENOENT scandir 'test-results'). + ignores: [ + ".next/**", + ".next-check/**", + "src/data/**", + "test-results/**", + "playwright-report/**", + ], }, ...coreWebVitals, { diff --git a/playwright.config.mjs b/playwright.config.mjs index 44c436d..22c0819 100644 --- a/playwright.config.mjs +++ b/playwright.config.mjs @@ -10,6 +10,9 @@ import { defineConfig, devices } from '@playwright/test'; export default defineConfig({ testDir: 'tests/e2e', testMatch: '**/*.spec.js', + // Compiles the game routes once, before the workers race for them. See the + // file for why the suite fails in a block of 8 without it. + globalSetup: './tests/e2e/global-setup.js', fullyParallel: true, // The suite is deterministic (no live services); a failure is a real one. retries: 0, diff --git a/tests/e2e/global-setup.js b/tests/e2e/global-setup.js new file mode 100644 index 0000000..48b2a9b --- /dev/null +++ b/tests/e2e/global-setup.js @@ -0,0 +1,38 @@ +// Warms the dev server's heavy routes before any worker runs. +// +// `next dev` compiles a route on its first request. The game routes are the +// expensive ones, and 8 parallel workers otherwise all demand that same cold +// compile at once, each waiting behind it -- every test that navigates to a +// /game URL then fails together at the 60s timeout. Reproducible on demand: +// touch a source file (invalidating the compile cache) and run the suite. +// +// One serial pass over the three route shapes pays the compile once. A warm +// server, or a production one, makes these near-instant no-ops. +export default async function globalSetup(config) { + const { baseURL } = config.projects[0].use; + + // One per compiled route: the prerendered region page, the dynamic legacy + // redirect, and the not-found path through the [region] segment. + const routes = ['/', '/game/tphcm', '/game?region=TPHCM', '/game/notaregion']; + + for (const route of routes) { + try { + // Bounded: a wedged compile would otherwise hang here forever, and + // Playwright's globalTimeout is 0, so the run would stall with no output. + await fetch(new URL(route, baseURL), { + redirect: 'follow', + signal: AbortSignal.timeout(60_000), + }); + } catch (error) { + // A warm-up is an optimisation, never a gate: if the server is not + // answering, the specs' own waits and retries still apply, and their + // failure messages say far more than one thrown here would. Only a + // network-level failure lands here -- fetch resolves on any HTTP status, + // so a 404 or a 500 is a successful warm-up, not a miss. + // + // Said out loud, because a skipped warm-up looks exactly like the flake + // it exists to prevent: 8 game-route tests timing out together. + console.warn(`[global-setup] warm-up of ${route} failed: ${error.message}`); + } + } +} diff --git a/tests/e2e/routing.spec.js b/tests/e2e/routing.spec.js index 8f227ec..acb051f 100644 --- a/tests/e2e/routing.spec.js +++ b/tests/e2e/routing.spec.js @@ -105,6 +105,15 @@ test('404s an unknown region', async ({ page }) => { expect(response.status()).toBe(404); }); +test('404s a spelling that only uppercases into a region', async ({ page }) => { + // U+0131 dotless i: 'hn-badınh'.toUpperCase() is 'HN-BADINH', a real code. + // Accepting it would serve a real round at a URL that is its own ISR entry + // and its own analytics row -- the split the region path exists to avoid. + // Asserted through a real request because the encoding is half the point. + const response = await page.goto('/game/hn-bad%C4%B1nh'); + expect(response.status()).toBe(404); +}); + test('serves a real region with no imagery instead of 404ing it', async ({ page }) => { // Deliberately NOT a 404. The API answers an uncovered region with a written // message ("... has no street view coverage yet") and the page renders it; @@ -112,17 +121,57 @@ test('serves a real region with no imagery instead of 404ing it', async ({ page // here, so this asserts only the routing half -- that the page exists and // serves. The message itself is the API's contract, covered in // tests/region-request.test.js. - const { isRegion, isPlayable } = await import('../../src/lib/regions.js'); + const { isRegion, isPlayable, regionSlug } = await import('../../src/lib/regions.js'); const code = 'TPHCM-CUCHI'; expect(isRegion(code), `${code} should still be a region`).toBe(true); expect(isPlayable(code), `${code} gained coverage; pick another fixture`).toBe(false); - const { regionSlug } = await import('../../src/lib/regions.js'); const response = await page.goto(`/game/${regionSlug(code)}`); expect(response.status()).toBe(200); expect(landedAt(page)).toBe(`/game/${regionSlug(code)}`); }); +test('the pre-paint theme script is served executable', async ({ request }) => { + // The other half of InlineScript's contract. Asserted on the raw HTML, with + // no browser, because in a browser this is invisible: ThemeToggle re-applies + // the theme on mount (ThemeToggle.js:33-39), so ends up with the right + // class whether or not the script ran -- just a flash later. An executable + // type in the served markup is the only thing that proves it runs before + // paint, so that is what this pins. + for (const path of ['/', '/game/tphcm']) { + const html = await (await request.get(path)).text(); + const headEnd = html.indexOf(''); + // Asserted, not assumed: indexOf returning -1 would slice to nothing and + // quietly turn the checks below into assertions about an empty string. + expect(headEnd, `${path} served no `).toBeGreaterThan(0); + const head = html.slice(0, headEnd); + expect(head, `${path} lost its inline theme script`).toContain('classList.toggle'); + expect(head, `${path} serves the theme script inert -- it would flash`) + .toMatch(/