diff --git a/.github/workflows/tests.yml b/.github/workflows/tests.yml index b9372f1..a72c527 100644 --- a/.github/workflows/tests.yml +++ b/.github/workflows/tests.yml @@ -9,6 +9,14 @@ concurrency: group: tests-${{ github.ref }} cancel-in-progress: true +# `$env/static/public` is resolved at build time, so every variable imported from it must be +# present for `vite build` and `svelte-check` to succeed on a clean checkout. These are the +# local-stack defaults already published in .env.local.example - never real credentials. +env: + PUBLIC_USE_LOCAL_POKEMON_SPRITE_FOLDER: 'false' + PUBLIC_SUPABASE_URL: http://127.0.0.1:54321 + PUBLIC_SUPABASE_ANON_KEY: eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZS1kZW1vIiwicm9sZSI6ImFub24iLCJleHAiOjE5ODM4MTI5OTZ9.CRXP1A7WOeoJeXxjNni43kdQwgnWNReilDMblYTn_I0 + jobs: quality: runs-on: ubuntu-latest @@ -22,7 +30,6 @@ jobs: - run: npm run check - run: npm run lint - run: npm run test:fast - - run: npm run test:coverage - uses: actions/upload-artifact@v4 if: always() with: @@ -39,8 +46,8 @@ jobs: node-version: 20 cache: npm - run: npm ci + # `supabase start` applies migrations and seeds; no separate reset is needed. - run: npx supabase start - - run: npx supabase db reset - run: npm run test:integration - if: always() run: npx supabase stop @@ -69,9 +76,10 @@ jobs: with: path: ~/.cache/ms-playwright key: playwright-chromium-${{ runner.os }}-${{ hashFiles('package-lock.json') }} + restore-keys: | + playwright-chromium-${{ runner.os }}- - run: npx playwright install --with-deps chromium - run: npx supabase start - - run: npx supabase db reset - run: npm run test:bdd - uses: actions/upload-artifact@v4 if: failure() diff --git a/README.md b/README.md index e1cfe3a..487e1bd 100644 --- a/README.md +++ b/README.md @@ -42,15 +42,21 @@ The test suite is split by responsibility so a failure points to the correct lay definitions and browser fixtures live beside it under `tests/bdd`. - `tests/build` verifies generated service-worker and manifest artifacts after each supported build. -Run the offline suites while developing: +Run the offline suites while developing. `test:fast` includes the coverage run, so there is no need +to run both: ```bash npm run test:fast -npm run test:coverage ``` -Database and BDD tests require Docker and the local Supabase stack. The wrappers read local keys from -`supabase status`; no credentials are written to disk or committed: +Coverage is measured across all of `src/lib` (excluding type-only models and the browser-only +store/action modules) with global thresholds set to the current baseline, so new untested code lowers +the number instead of being invisible to the gate. Ratchet the thresholds in `vitest.config.mts` up as +coverage grows, never down. + +Database and BDD tests require Docker and the local Supabase stack. The wrappers read the local keys +from `supabase status` at run time, so no keys are hard-coded in the test files; the local stack's +well-known demo keys do appear in `.env.local.example`, and no real credentials are committed: ```bash npm run supabase:start @@ -67,9 +73,24 @@ Gherkin describes outcomes in domain language. Keep selectors, API calls, test-u provider mocks in step definitions or support fixtures. `@product-review` marks a rule that should be reviewed with product stakeholders, but does not skip it. Missing or ambiguous steps fail generation. -Google Drive and Dropbox scenarios use a local provider server and private endpoint overrides. They do -not contact real provider accounts. Chromium is the only configured browser project. Playwright traces -and screenshots are retained on failure under `test-results`. +Google Drive and Dropbox scenarios use a local provider server (`scripts/mock-provider-server.mjs`) +and never contact real provider accounts. The endpoint overrides that point at it are refused unless +`ALLOW_PROVIDER_ENDPOINT_OVERRIDES=true` **and** the override is a loopback URL - these endpoints +receive the OAuth client secret and refresh token, so they must not be redirectable in a deployed +environment. `npm run test:bdd` sets the flag; nothing else should. + +The mock's recorded requests, refresh counter and fail-uploads switch are reset before every scenario +by an auto fixture in `tests/bdd/fixtures.ts`. That reset is also why the suite runs with a single +worker: the mock is one shared process, so parallel scenarios would reset each other's state. A global +teardown deletes the users each run creates, so repeated local runs do not need a database reset. + +Chromium is the only configured browser project. Playwright traces and screenshots are retained on +failure under `test-results`. + +Known gap: the password-reset scenarios use an ordinary signed-in session rather than a recovery link, +because following a real recovery link currently bounces to `/signin` - the browser client in +`src/routes/+layout.ts` has no cookie `set`/`remove` method, so it cannot persist the session it parses +out of the URL. `createRecoveryLink` in `tests/bdd/support/app.ts` is ready for when that is fixed. The current National Dex maximum is deliberately asserted as 1025. When adding a new generation, update that expectation together with Pokémon data, the corresponding game/dex files, database seed, diff --git a/package.json b/package.json index 4121062..de48747 100644 --- a/package.json +++ b/package.json @@ -23,8 +23,8 @@ "tailwind": "npx tailwindcss -i ./static/input.css -o ./static/output.css", "test:unit": "vitest run tests/unit", "test:data": "vitest run tests/data", - "test:fast": "npm run test:unit && npm run test:data", "test:coverage": "vitest run tests/unit --coverage", + "test:fast": "npm run test:coverage && npm run test:data", "test:integration": "node scripts/run-with-local-supabase.mjs vitest run --config vitest.integration.config.mts", "test:bdd:generate": "bddgen", "test:bdd:inner": "npm run build-inject-manifest && npm run test:bdd:generate && playwright test --project=chromium", @@ -34,7 +34,7 @@ "test:build:inject-static": "npm run build-inject-manifest && vitest run --config vitest.build.config.mts", "test:build:inject-node": "npm run build-inject-manifest-node && NODE_ADAPTER=true vitest run --config vitest.build.config.mts", "test:build": "npm run test:build:generate-static && npm run test:build:generate-node && npm run test:build:inject-static && npm run test:build:inject-node", - "test:ci": "npm run test:fast && npm run test:coverage && npm run test:integration && npm run test:build && npm run test:bdd", + "test:ci": "npm run test:fast && npm run test:integration && npm run test:build && npm run test:bdd", "test": "npm run test:ci", "supabase:start": "supabase start", "supabase:stop": "supabase stop", diff --git a/playwright.config.ts b/playwright.config.ts index 23499eb..b9c112f 100644 --- a/playwright.config.ts +++ b/playwright.config.ts @@ -25,6 +25,7 @@ import { nodeAdapter } from './adapter.mjs'; */ export default defineConfig({ testDir, + globalTeardown: './tests/bdd/globalTeardown.ts', /* Folder for test artifacts such as screenshots, videos, traces, etc. */ outputDir: 'test-results/', timeout: 90 * 1000, diff --git a/scripts/mock-provider-server.mjs b/scripts/mock-provider-server.mjs index b1784c8..c7e406c 100644 --- a/scripts/mock-provider-server.mjs +++ b/scripts/mock-provider-server.mjs @@ -14,7 +14,12 @@ const server = createServer(async (request, response) => { const url = new URL(request.url ?? '/', `http://127.0.0.1:${port}`); let body = ''; for await (const chunk of request) body += chunk; - state.requests.push({ method: request.method, path: url.pathname, query: url.search, body }); + + // Control-plane calls (including Playwright's webServer readiness polling of /__mock/state) + // must not show up as provider traffic the assertions then reason about. + if (!url.pathname.startsWith('/__mock/')) { + state.requests.push({ method: request.method, path: url.pathname, query: url.search, body }); + } if (url.pathname === '/__mock/state') return send(response, 200, state); if (url.pathname === '/__mock/reset') { diff --git a/scripts/run-with-local-supabase.mjs b/scripts/run-with-local-supabase.mjs index ec95195..5791e1c 100644 --- a/scripts/run-with-local-supabase.mjs +++ b/scripts/run-with-local-supabase.mjs @@ -8,25 +8,44 @@ if (!command) { process.exit(2); } +const SETUP_HINT = 'Run "npm run supabase:start" followed by "npm run supabase:reset".'; +// npx is a shell script on Windows, where spawn needs a shell to find it. +const useShell = process.platform === 'win32'; + +function fail(message, detail) { + console.error(message); + if (detail) console.error(String(detail).trim()); + process.exit(1); +} + const status = spawnSync('npx', ['supabase', 'status', '--output', 'json'], { encoding: 'utf8', - stdio: ['ignore', 'pipe', 'pipe'] + stdio: ['ignore', 'pipe', 'pipe'], + shell: useShell }); +if (status.error) { + fail( + 'Unable to run "npx supabase status" - is the supabase CLI installed?', + status.error.message + ); +} + if (status.status !== 0) { - console.error('Local Supabase is required but is not running.'); - console.error('Run "npm run supabase:start" followed by "npm run supabase:reset".'); - if (status.stderr.trim()) console.error(status.stderr.trim()); - process.exit(status.status ?? 1); + const stderr = status.stderr ?? ''; + // Distinguish a stopped stack from a genuinely broken CLI invocation, so the hint is only + // printed when it is actually the advice the reader needs. + if (/not running|supabase start/i.test(stderr)) { + fail(`Local Supabase is required but is not running.\n${SETUP_HINT}`, stderr); + } + fail(`"supabase status" failed with exit code ${status.status}.`, stderr); } let values; try { values = JSON.parse(status.stdout); } catch (error) { - console.error('Unable to parse "supabase status --output json".'); - console.error(error); - process.exit(1); + fail('Unable to parse "supabase status --output json".', error); } const apiUrl = values.API_URL ?? values.api_url ?? 'http://127.0.0.1:54321'; @@ -34,12 +53,30 @@ const anonKey = values.ANON_KEY ?? values.PUBLISHABLE_KEY ?? values.anon_key; const serviceRoleKey = values.SERVICE_ROLE_KEY ?? values.SECRET_KEY ?? values.service_role_key; if (!anonKey || !serviceRoleKey) { - console.error('Supabase status did not return an anonymous and service-role key.'); - process.exit(1); + fail(`Supabase status did not return an anonymous and service-role key.\n${SETUP_HINT}`); +} + +// A running-but-unseeded database is the most common broken state, and it surfaces downstream as +// a confusing assertion failure. Check it here instead. +const probe = await fetch(`${apiUrl}/rest/v1/pokedex_entries?select=id&limit=1`, { + headers: { apikey: anonKey, Authorization: `Bearer ${anonKey}` } +}).catch((error) => { + fail(`Unable to reach the local Supabase REST API at ${apiUrl}.\n${SETUP_HINT}`, error); +}); + +if (!probe.ok) { + fail( + `The local Supabase database has no readable pokedex_entries (HTTP ${probe.status}).\n${SETUP_HINT}`, + await probe.text() + ); +} +if (((await probe.json()) ?? []).length === 0) { + fail(`The local Supabase database is empty - migrations or seeds have not run.\n${SETUP_HINT}`); } const child = spawnSync(command, args, { stdio: 'inherit', + shell: useShell, env: { ...process.env, PUBLIC_SUPABASE_URL: process.env.PUBLIC_SUPABASE_URL ?? apiUrl, @@ -51,4 +88,6 @@ const child = spawnSync(command, args, { } }); -process.exit(child.status ?? 1); +if (child.error) fail(`Unable to run "${command}".`, child.error.message); +// A signalled child reports status === null; exiting 0 there would hide the failure. +process.exit(child.signal ? 1 : child.status ?? 1); diff --git a/supabase/config.toml b/supabase/config.toml index 8b00a0f..2fb0088 100644 --- a/supabase/config.toml +++ b/supabase/config.toml @@ -119,7 +119,8 @@ enabled = true # in emails. site_url = "http://localhost:5173" # A list of *exact* URLs that auth providers are permitted to redirect to post authentication. -additional_redirect_urls = ["http://localhost:5173"] +# 4173 is the preview server the BDD suite runs against; recovery links redirect there. +additional_redirect_urls = ["http://localhost:5173", "http://localhost:4173"] # How long tokens are valid for, in seconds. Defaults to 3600 (1 hour), maximum 604,800 (1 week). jwt_expiry = 3600 # If disabled, the refresh token will never expire. @@ -149,7 +150,8 @@ anonymous_users = 30 # Number of sessions that can be refreshed in a 5 minute interval per IP address. token_refresh = 150 # Number of sign up and sign-in requests that can be made in a 5 minute interval per IP address (excludes anonymous users). -sign_in_sign_ups = 30 +# Raised for the BDD suite: every scenario provisions and signs in its own user from one IP. +sign_in_sign_ups = 300 # Number of OTP / Magic link verifications that can be made in a 5 minute interval per IP address. token_verifications = 30 # Number of Web3 logins that can be made in a 5 minute interval per IP address. diff --git a/tests/bdd/features/account-access.feature b/tests/bdd/features/account-access.feature index 1b84abd..b721810 100644 --- a/tests/bdd/features/account-access.feature +++ b/tests/bdd/features/account-access.feature @@ -36,12 +36,12 @@ Feature: Account access @product-review Scenario: Reject mismatched replacement passwords - Given I am on the password reset page with a recovery session + Given I am signed in on the password reset page When I enter two different replacement passwords Then I am told that the passwords do not match Scenario: Complete a password reset - Given I am on the password reset page with a recovery session + Given I am signed in on the password reset page When I enter a valid replacement password Then I am told that my password was updated diff --git a/tests/bdd/features/backup-export.feature b/tests/bdd/features/backup-export.feature index a4488d3..cb3eb5a 100644 --- a/tests/bdd/features/backup-export.feature +++ b/tests/bdd/features/backup-export.feature @@ -28,7 +28,7 @@ Feature: Backup and export And I have a Living Dex named "Quoted, Dex" When I save a catch note containing a comma and quote Then the mocked provider receives a valid escaped CSV - And the catch update remains saved + And the note survives a reload Scenario: Refresh an expired provider token Given Dropbox is connected with an expired token @@ -38,6 +38,6 @@ Feature: Backup and export Scenario: Record provider failure without losing progress Given Google Drive is connected to a failing mocked provider When I update collection progress - Then the catch update remains saved + Then the catch remains marked caught And the provider failure is shown in backup settings diff --git a/tests/bdd/features/pwa-offline.feature b/tests/bdd/features/pwa-offline.feature index 4ffe63a..01777d8 100644 --- a/tests/bdd/features/pwa-offline.feature +++ b/tests/bdd/features/pwa-offline.feature @@ -6,13 +6,18 @@ Feature: Offline-friendly application Scenario: Register the service worker When I open the built application Then a service worker controls the page - And the application cache is present + And the application shell is precached - Scenario: Navigate while offline + Scenario: Reload the home page while offline Given I have opened the built application online - When I go offline and revisit the home page with a trailing slash + When I go offline and reload the home page Then the application remains available + Scenario: Navigate to another route while offline + Given I have opened the built application online + When I go offline and navigate to the sign-in page + Then the sign-in form is available offline + Scenario: Restore network access Given I have opened the built application online When I go offline and then return online diff --git a/tests/bdd/fixtures.ts b/tests/bdd/fixtures.ts index 06c25ea..3f3aa66 100644 --- a/tests/bdd/fixtures.ts +++ b/tests/bdd/fixtures.ts @@ -10,11 +10,30 @@ export type ScenarioState = { entries: Array<{ pokemon: string; form: string | null; num: number; id: string }>; lastResponseStatus: number | null; lastMessage: string | null; + caughtEntryLabel: string | null; }; -type Fixtures = { state: ScenarioState }; +type Fixtures = { state: ScenarioState; providerMock: void }; + +const MOCK_URL = process.env.MOCK_PROVIDER_URL ?? 'http://127.0.0.1:4199'; export const test = base.extend({ + /** + * The mock provider keeps recorded requests, the refresh counter and the fail-uploads switch + * in one process-wide object. Without a reset per scenario, assertions are satisfied by + * whatever ran before them - and the failing-upload scenario would poison every later one. + */ + providerMock: [ + // eslint-disable-next-line no-empty-pattern + async ({}, use) => { + const response = await fetch(`${MOCK_URL}/__mock/reset`); + if (!response.ok) { + throw new Error(`Unable to reset the mock provider at ${MOCK_URL}: ${response.status}`); + } + await use(); + }, + { auto: true } + ], // Playwright fixture callbacks require the dependency object even when this fixture has none. // eslint-disable-next-line no-empty-pattern state: async ({}, use, testInfo) => { @@ -31,7 +50,8 @@ export const test = base.extend({ pokedexName: null, entries: [], lastResponseStatus: null, - lastMessage: null + lastMessage: null, + caughtEntryLabel: null }); } }); diff --git a/tests/bdd/globalTeardown.ts b/tests/bdd/globalTeardown.ts new file mode 100644 index 0000000..917e0ca --- /dev/null +++ b/tests/bdd/globalTeardown.ts @@ -0,0 +1,48 @@ +/** + * Deletes the users each BDD run provisions, so repeated local runs do not need a full + * `supabase db reset` to stay clean. Pokédexes, catch records and integrations follow via the + * schema's cascades. Only the suite's own synthetic addresses are touched. + */ +const SUPABASE_URL = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321'; +const OWNED_EMAIL = /^(bdd|other|integration)-.*@example\.test$/; + +type AdminUser = { id: string; email?: string }; + +async function adminRequest(path: string, init: RequestInit = {}) { + const key = process.env.E2E_SERVICE_ROLE_KEY; + if (!key) return null; + return fetch(`${SUPABASE_URL}${path}`, { + ...init, + headers: { + apikey: key, + Authorization: `Bearer ${key}`, + 'Content-Type': 'application/json', + ...(init.headers ?? {}) + } + }); +} + +export default async function globalTeardown() { + const listed = await adminRequest('/auth/v1/admin/users?per_page=1000'); + if (!listed) { + console.warn('Skipping BDD teardown: E2E_SERVICE_ROLE_KEY is not set.'); + return; + } + if (!listed.ok) { + console.warn(`Skipping BDD teardown: unable to list users (${listed.status}).`); + return; + } + + const { users = [] } = (await listed.json()) as { users?: AdminUser[] }; + const disposable = users.filter((user) => user.email && OWNED_EMAIL.test(user.email)); + + let failures = 0; + for (const user of disposable) { + const deleted = await adminRequest(`/auth/v1/admin/users/${user.id}`, { method: 'DELETE' }); + if (!deleted?.ok) failures++; + } + + console.log( + `BDD teardown removed ${disposable.length - failures} of ${disposable.length} test users.` + ); +} diff --git a/tests/bdd/steps/account.steps.ts b/tests/bdd/steps/account.steps.ts index a03d8bd..148f330 100644 --- a/tests/bdd/steps/account.steps.ts +++ b/tests/bdd/steps/account.steps.ts @@ -28,11 +28,19 @@ Given('I am signed in', async ({ page, state }) => { await expect(page).toHaveURL(/\/my-pokedexes$/); }); -Given('I am on the password reset page with a recovery session', async ({ page, state }) => { +/** + * Deliberately an ordinary signed-in session, not a recovery one: following a real recovery + * action link currently bounces to /signin, because the browser client in src/routes/+layout.ts + * has no cookie `set`/`remove` method and so cannot persist the session it parses out of the + * URL. Until that is fixed, these scenarios cover the form, not the emailed-link flow - hence + * the step name. `createRecoveryLink` in ../support/app.ts is ready for when it is. + */ +Given('I am signed in on the password reset page', async ({ page, state }) => { await createConfirmedUser(state); await signIn(page, state); await expect(page).toHaveURL(/\/my-pokedexes$/); await page.goto('/reset-password'); + await expect(page.getByLabel('New Password')).toBeVisible(); }); When('I register with valid account details', async ({ page, state }) => { diff --git a/tests/bdd/steps/backup.steps.ts b/tests/bdd/steps/backup.steps.ts index fd399ab..d7b2eaf 100644 --- a/tests/bdd/steps/backup.steps.ts +++ b/tests/bdd/steps/backup.steps.ts @@ -72,7 +72,6 @@ When('I visit backup settings', async ({ page }) => { }); When('I connect the mocked {string} provider', async ({ page }, provider: string) => { - await fetch(`${MOCK_URL}/__mock/reset`); await page.goto('/backup-settings'); const card = page.locator('.card, .border').filter({ hasText: provider }).last(); await card.getByRole('button', { name: 'Connect' }).click(); @@ -125,29 +124,54 @@ Then('the backup connection is rejected', async ({ state }) => { expect(state.lastResponseStatus).toBe(400); }); -Then('the mocked provider receives a valid escaped CSV', async () => { +async function mockState() { const response = await fetch(`${MOCK_URL}/__mock/state`); - const mock = (await response.json()) as { requests: Array<{ path: string; body: string }> }; - const upload = mock.requests.find((request) => request.path.includes('upload')); - expect(upload?.body).toContain('"A comma, and a ""quote"""'); + if (!response.ok) throw new Error(`Mock provider is unavailable: ${response.status}`); + return (await response.json()) as { + refreshes: number; + requests: Array<{ method: string; path: string; body: string }>; + }; +} + +Then('the mocked provider receives a valid escaped CSV', async () => { + const { requests } = await mockState(); + const upload = requests.find((request) => request.path.includes('upload')); + expect(upload, 'no upload reached the mock provider').toBeDefined(); + expect(upload!.body).toContain('"A comma, and a ""quote"""'); }); -Then('the catch update remains saved', async ({ page, state }) => { +Then('the note survives a reload', async ({ page, state }) => { await page.goto(`/pokedex/${state.pokedexId}`); await expect(firstPokemon(page)).toBeVisible(); await openFirstPokemon(page); - const dialog = page.getByRole('dialog'); - const caught = dialog.getByText('Caught:', { exact: true }).locator('..').getByRole('checkbox'); - const notes = dialog.getByLabel('Notes:'); - expect((await caught.isChecked()) || (await notes.inputValue()).includes('comma')).toBe(true); + await expect(page.getByRole('dialog').getByLabel('Notes:')).toHaveValue('A comma, and a "quote"'); +}); + +Then('the catch remains marked caught', async ({ page, state }) => { + await page.goto(`/pokedex/${state.pokedexId}`); + await expect(firstPokemon(page)).toBeVisible(); + await openFirstPokemon(page); + const caught = page + .getByRole('dialog') + .getByText('Caught:', { exact: true }) + .locator('..') + .getByRole('checkbox'); + await expect(caught).toBeChecked(); }); Then('the token is refreshed before the mocked upload', async ({ state }) => { expect(state.lastResponseStatus).toBe(200); - const response = await fetch(`${MOCK_URL}/__mock/state`); - const mock = (await response.json()) as { refreshes: number; requests: Array<{ path: string }> }; - expect(mock.refreshes).toBeGreaterThan(0); - expect(mock.requests.some((request) => request.path.includes('upload'))).toBe(true); + const { refreshes, requests } = await mockState(); + // Exactly one refresh, and it has to come before the upload it was needed for - a global + // ">= 1" would be satisfied by any earlier scenario's traffic. + expect(refreshes).toBe(1); + const refreshIndex = requests.findIndex( + (request) => + request.path.endsWith('/token') && request.body.includes('grant_type=refresh_token') + ); + const uploadIndex = requests.findIndex((request) => request.path.includes('upload')); + expect(refreshIndex).toBeGreaterThanOrEqual(0); + expect(uploadIndex).toBeGreaterThan(refreshIndex); }); Then('the provider failure is shown in backup settings', async ({ page }) => { diff --git a/tests/bdd/steps/pokedex-lifecycle.steps.ts b/tests/bdd/steps/pokedex-lifecycle.steps.ts index aeb90e1..7b8efa8 100644 --- a/tests/bdd/steps/pokedex-lifecycle.steps.ts +++ b/tests/bdd/steps/pokedex-lifecycle.steps.ts @@ -1,6 +1,6 @@ import { createBdd } from 'playwright-bdd'; import { test, expect } from '../fixtures'; -import { createDexThroughUi } from '../support/app'; +import { createDexThroughUi, deleteAllPokedexes } from '../support/app'; const { Given, When, Then } = createBdd(test); @@ -11,11 +11,10 @@ async function dexCard(page: Parameters[0], name: str return card; } -Given('I have no Pokédexes', async ({ page }) => { - await page.goto('/my-pokedexes'); - await expect( - page.locator('.card').filter({ has: page.getByRole('button', { name: 'View' }) }) - ).toHaveCount(0); +// Establishes the precondition rather than asserting it - a fresh user happens to be empty, +// which would make an assertion here pass without testing anything. +Given('I have no Pokédexes', async ({ state }) => { + await deleteAllPokedexes(state); }); Given('I have a Living Dex named {string}', async ({ page, state }, name: string) => { diff --git a/tests/bdd/steps/progress.steps.ts b/tests/bdd/steps/progress.steps.ts index 793f628..603a61a 100644 --- a/tests/bdd/steps/progress.steps.ts +++ b/tests/bdd/steps/progress.steps.ts @@ -50,17 +50,27 @@ When('I add the note {string} to the first Pokémon', async ({ page }, note: str await page.getByRole('dialog').getByLabel('Notes:').blur(); }); +function boxContainer(page: Parameters[0], box: number) { + return page + .getByRole('heading', { name: `Box ${box}`, exact: true }) + .locator('..') + .locator('..'); +} + When('I mark box {int} as caught', async ({ page }, box: number) => { - const heading = page.getByRole('heading', { name: `Box ${box}`, exact: true }); - const container = heading.locator('..').locator('..'); + const container = boxContainer(page, box); await container.getByRole('button', { name: 'Open bulk actions menu' }).click(); await container.getByRole('button', { name: 'Mark box as Caught' }).click(); }); -When('I filter to Pokémon that are not caught', async ({ page }) => { +When('I filter to Pokémon that are not caught', async ({ page, state }) => { if ((await page.getByRole('dialog').count()) > 0) { await page.getByRole('dialog').getByRole('button', { name: 'Close', exact: true }).click(); } + // Remember which entry was caught, so the assertion can name it rather than trusting + // whichever entry happens to be first after filtering. + state.caughtEntryLabel = await firstPokemon(page).getAttribute('aria-label'); + expect(state.caughtEntryLabel).toMatch(/Status: Caught/); await page.getByText('Not caught', { exact: true }).locator('..').getByRole('checkbox').check(); }); @@ -94,18 +104,31 @@ Then('its HOME state and note persist after reloading', async ({ page }) => { ); }); -Then('box {int} contains {int} caught Pokémon', async ({ page }, _box: number, count: number) => { +Then('box {int} contains {int} caught Pokémon', async ({ page }, box: number, count: number) => { await settleAndReload(page); - const firstBoxEntries = page - .getByRole('button', { name: /^View details for / }) - .filter({ hasNot: page.locator('[disabled]') }); - for (let index = 0; index < count; index++) { - await expect(firstBoxEntries.nth(index)).toHaveAttribute('aria-label', /Status: Caught/); - } + const entries = boxContainer(page, box).getByRole('button', { name: /^View details for / }); + await expect(entries).toHaveCount(count); + const labels = await entries.evaluateAll((elements) => + elements.map((element) => element.getAttribute('aria-label') ?? '') + ); + expect(labels.filter((label) => /Status: Caught/.test(label))).toHaveLength(count); }); -Then('the caught Pokémon is filtered out', async ({ page }) => { - await expect(firstPokemon(page)).toHaveAttribute('aria-disabled', 'true'); +Then('the caught Pokémon is filtered out', async ({ page, state }) => { + const label = state.caughtEntryLabel; + if (!label) throw new Error('No caught entry was recorded before filtering'); + // That specific entry is excluded... + await expect(page.getByRole('button', { name: label, exact: true })).toHaveAttribute( + 'aria-disabled', + 'true' + ); + // ...and the filter did not simply exclude everything. + await expect( + page + .getByRole('button', { name: /^View details for / }) + .and(page.locator('[aria-disabled="false"]')) + .first() + ).toBeVisible(); }); Then('the {string} box layout remains selected', async ({ page }, layout: string) => { diff --git a/tests/bdd/steps/pwa.steps.ts b/tests/bdd/steps/pwa.steps.ts index 9d9fbda..7df48a2 100644 --- a/tests/bdd/steps/pwa.steps.ts +++ b/tests/bdd/steps/pwa.steps.ts @@ -17,6 +17,17 @@ async function waitForServiceWorker(page: Page) { }); } +async function cacheContents(page: Page) { + return page.evaluate(async () => { + const contents: Record = {}; + for (const name of await caches.keys()) { + const cache = await caches.open(name); + contents[name] = (await cache.keys()).map((request) => request.url); + } + return contents; + }); +} + When('I open the built application', async ({ page }) => { await page.goto('/'); await waitForServiceWorker(page); @@ -29,9 +40,16 @@ Given('I have opened the built application online', async ({ page }) => { await page.reload(); }); -When('I go offline and revisit the home page with a trailing slash', async ({ page }) => { +When('I go offline and reload the home page', async ({ page }) => { await page.context().setOffline(true); - await page.goto('/', { waitUntil: 'domcontentloaded' }); + await page.reload({ waitUntil: 'domcontentloaded' }); +}); + +When('I go offline and navigate to the sign-in page', async ({ page }) => { + await page.context().setOffline(true); + // Client-side navigation, which only works if the route's chunks were precached. + await page.getByRole('link', { name: 'Sign In' }).click(); + await expect(page).toHaveURL(/\/signin$/); }); When('I go offline and then return online', async ({ page }) => { @@ -45,11 +63,45 @@ Then('a service worker controls the page', async ({ page }) => { expect(await waitForServiceWorker(page)).toMatch(/\/(?:sw|prompt-sw)\.js$/); }); -Then('the application cache is present', async ({ page }) => { - const cacheNames = await page.evaluate(() => caches.keys()); - expect(cacheNames.length).toBeGreaterThan(0); +/** + * Replaces the deleted client-test/sw.spec.ts assertions. The precise entries differ by build + * strategy, so this checks the contract that matters: one workbox precache, the app shell and + * web manifest are in it, and `_app/immutable` assets are cached WITHOUT a revision query - + * that last one is the `dontCacheBustURLsMatching` behaviour, and it silently regresses. + */ +Then('the application shell is precached', async ({ page }) => { + const contents = await cacheContents(page); + const names = Object.keys(contents).filter((name) => name.startsWith('workbox-precache')); + expect(names).toHaveLength(1); + + const origin = new URL(page.url()).origin; + const urls = contents[names[0]].map((url) => url.slice(`${origin}/`.length)); + + expect(urls, 'app shell is not precached').toContain(''); + expect( + urls.some((url) => url.startsWith('manifest.webmanifest?__WB_REVISION__=')), + 'revisioned manifest.webmanifest is not precached' + ).toBe(true); + expect( + urls.some((url) => url.startsWith('_app/version.json?__WB_REVISION__=')), + 'revisioned _app/version.json is not precached' + ).toBe(true); + + const immutable = urls.filter((url) => url.startsWith('_app/immutable/')); + expect(immutable.some((url) => url.endsWith('.css'))).toBe(true); + expect(immutable.some((url) => url.endsWith('.js'))).toBe(true); + expect( + immutable.filter((url) => url.includes('__WB_REVISION__')), + 'immutable assets must not be cache-busted' + ).toEqual([]); }); Then('the application remains available', async ({ page }) => { await expect(page.getByRole('heading', { name: /Start Your Pokédex Journey/ })).toBeVisible(); }); + +Then('the sign-in form is available offline', async ({ page }) => { + await expect(page.getByLabel('Email')).toBeVisible(); + await expect(page.getByLabel('Password')).toBeVisible(); + await expect(page.getByRole('button', { name: 'Sign In' })).toBeVisible(); +}); diff --git a/tests/bdd/support/app.ts b/tests/bdd/support/app.ts index b197c20..18987c7 100644 --- a/tests/bdd/support/app.ts +++ b/tests/bdd/support/app.ts @@ -33,6 +33,44 @@ export async function createConfirmedUser(state: ScenarioState): Promise { state.userId = body.id; } +export async function deleteAllPokedexes(state: ScenarioState): Promise { + const key = requireServiceRoleKey(); + if (!state.userId) throw new Error('A confirmed user is required before clearing Pokédexes'); + const response = await fetch( + `${SUPABASE_URL}/rest/v1/pokedexes?userId=eq.${encodeURIComponent(state.userId)}`, + { method: 'DELETE', headers: { apikey: key, Authorization: `Bearer ${key}` } } + ); + if (!response.ok) throw new Error(`Unable to clear Pokédexes: ${await response.text()}`); +} + +/** + * Asks Supabase for a real recovery action link - the same token-bearing URL the emailed link + * carries - so the reset scenarios exercise token verification rather than an ordinary session. + * `redirectTo` must be listed in `auth.additional_redirect_urls` in supabase/config.toml. + */ +export async function createRecoveryLink( + state: ScenarioState, + redirectTo: string +): Promise { + const key = requireServiceRoleKey(); + const response = await fetch(`${SUPABASE_URL}/auth/v1/admin/generate_link`, { + method: 'POST', + headers: { + apikey: key, + Authorization: `Bearer ${key}`, + 'Content-Type': 'application/json' + }, + body: JSON.stringify({ type: 'recovery', email: state.email, redirect_to: redirectTo }) + }); + if (!response.ok) + throw new Error( + `Unable to generate a recovery link: ${response.status} ${await response.text()}` + ); + const body = (await response.json()) as { action_link?: string }; + if (!body.action_link) throw new Error('Supabase returned no recovery action link'); + return body.action_link; +} + export async function signIn(page: Page, state: ScenarioState, password = state.password) { await page.goto('/signin'); await page.getByLabel('Email').fill(state.email); diff --git a/tests/integration/defaultForm.integration.test.ts b/tests/integration/defaultForm.integration.test.ts index 6c1033c..daf3494 100644 --- a/tests/integration/defaultForm.integration.test.ts +++ b/tests/integration/defaultForm.integration.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, beforeAll } from 'vitest'; import { createClient, type SupabaseClient } from '@supabase/supabase-js'; -import CombinedDataRepository from '../../src/lib/repositories/CombinedDataRepository'; +import CombinedDataRepository from '$lib/repositories/CombinedDataRepository'; import { readRepoCsv } from '../support/csv'; /** @@ -9,14 +9,22 @@ import { readRepoCsv } from '../support/csv'; * fix branch makes the same assertions pass without test-only schema knowledge. */ const SUPABASE_URL = process.env.TEST_SUPABASE_URL ?? 'http://127.0.0.1:54321'; -const SUPABASE_KEY = - process.env.TEST_SUPABASE_ANON_KEY ?? - 'eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJzdXBhYmFzZS1kZW1vIiwicm9sZSI6ImFub24iLCJleHAiOjE5ODM4MTI5OTZ9.CRXP1A7WOeoJeXxjNni43kdQwgnWNReilDMblYTn_I0'; +const SETUP_HINT = + 'Run integration tests through "npm run test:integration", which reads the local keys from "supabase status".'; + +// No baked-in key: a fallback would silently point a misconfigured run at the wrong stack +// instead of failing with the instruction above. +function requireAnonKey(): string { + const key = process.env.TEST_SUPABASE_ANON_KEY; + if (!key) throw new Error(`Integration tests require TEST_SUPABASE_ANON_KEY. ${SETUP_HINT}`); + return key; +} async function requireSupabase() { + const key = requireAnonKey(); try { const res = await fetch(`${SUPABASE_URL}/rest/v1/pokedex_entries?select=id&limit=1`, { - headers: { apikey: SUPABASE_KEY, Authorization: `Bearer ${SUPABASE_KEY}` }, + headers: { apikey: key, Authorization: `Bearer ${key}` }, signal: AbortSignal.timeout(2000) }); if (!res.ok) throw new Error(`HTTP ${res.status}: ${await res.text()}`); @@ -43,7 +51,7 @@ describe('pokedex behavior regressions', () => { beforeAll(async () => { await requireSupabase(); - supabase = createClient(SUPABASE_URL, SUPABASE_KEY); + supabase = createClient(SUPABASE_URL, requireAnonKey()); const all: Row[] = []; for (let from = 0; ; from += 1000) { const { data, error } = await supabase @@ -118,9 +126,7 @@ describe('pokedex behavior regressions', () => { }); it('returns every expected entry despite the PostgREST row cap', async () => { - const { calculateExpectedEntries } = await import( - '../../src/lib/services/PokedexMappingService' - ); + const { calculateExpectedEntries } = await import('$lib/services/PokedexMappingService'); const baseDex = { id: 'test', name: 'test', diff --git a/tests/support/csv.ts b/tests/support/csv.ts index 8eab419..acd4031 100644 --- a/tests/support/csv.ts +++ b/tests/support/csv.ts @@ -56,6 +56,13 @@ export function readRepoCsv(relativePath: string): CsvRow[] { return parseCsv(readFileSync(resolve(process.cwd(), relativePath), 'utf8')); } +/** + * Typographic and straight apostrophes are the same identity here (Farfetch'd, Sirfetch'd), so + * the comparison absorbs the difference rather than forcing every data file to agree on one + * code point. + */ export function normalizedIdentity(...parts: Array): string { - return parts.map((part) => (part ?? '').trim().toLocaleLowerCase('en-GB')).join('|'); + return parts + .map((part) => (part ?? '').trim().replace(/[‘’ʼ]/g, "'").toLocaleLowerCase('en-GB')) + .join('|'); } diff --git a/tests/unit/auth.test.ts b/tests/unit/auth.test.ts index 085b39f..e39fb9d 100644 --- a/tests/unit/auth.test.ts +++ b/tests/unit/auth.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it, vi } from 'vitest'; -import { getOptionalUserId, requireAuth } from '../../src/lib/utils/auth'; +import { getOptionalUserId, requireAuth } from '$lib/utils/auth'; function eventReturning(value: unknown) { return { locals: { safeGetSession: vi.fn(async () => value) } } as never; diff --git a/tests/unit/boxPlacement.test.ts b/tests/unit/boxPlacement.test.ts index c026c42..709c3db 100644 --- a/tests/unit/boxPlacement.test.ts +++ b/tests/unit/boxPlacement.test.ts @@ -1,5 +1,5 @@ import { describe, expect, it } from 'vitest'; -import { calculateBoxNumbers, calculateBoxPlacement } from '../../src/lib/utils/boxPlacement'; +import { calculateBoxNumbers, calculateBoxPlacement } from '$lib/utils/boxPlacement'; describe('box placement', () => { it.each([ diff --git a/tests/unit/catchRecordWriteQueue.test.ts b/tests/unit/catchRecordWriteQueue.test.ts index c9fabaa..c2944f9 100644 --- a/tests/unit/catchRecordWriteQueue.test.ts +++ b/tests/unit/catchRecordWriteQueue.test.ts @@ -1,6 +1,6 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { createCatchRecordWriteQueue } from '../../src/lib/utils/catchRecordWriteQueue'; -import type { CatchRecord } from '../../src/lib/models/CatchRecord'; +import { createCatchRecordWriteQueue } from '$lib/utils/catchRecordWriteQueue'; +import type { CatchRecord } from '$lib/models/CatchRecord'; function mkRecord(overrides: Partial = {}): CatchRecord { return { diff --git a/tests/unit/combinedDataRepository.test.ts b/tests/unit/combinedDataRepository.test.ts index c0d1a4f..a2760bf 100644 --- a/tests/unit/combinedDataRepository.test.ts +++ b/tests/unit/combinedDataRepository.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect } from 'vitest'; -import CombinedDataRepository from '../../src/lib/repositories/CombinedDataRepository'; +import CombinedDataRepository from '$lib/repositories/CombinedDataRepository'; type Call = { method: string; args: unknown[] }; type TableQuery = { table: string; calls: Call[] }; diff --git a/tests/unit/oauthState.test.ts b/tests/unit/oauthState.test.ts index e722d02..93b9a72 100644 --- a/tests/unit/oauthState.test.ts +++ b/tests/unit/oauthState.test.ts @@ -5,7 +5,7 @@ import { readOAuthStateCookie, setOAuthStateCookie, type OAuthStatePayload -} from '../../src/lib/utils/oauthState'; +} from '$lib/utils/oauthState'; function eventWithCookie(raw?: string) { return { diff --git a/tests/unit/regionalDexMapping.test.ts b/tests/unit/regionalDexMapping.test.ts index be27c75..9496b7f 100644 --- a/tests/unit/regionalDexMapping.test.ts +++ b/tests/unit/regionalDexMapping.test.ts @@ -4,7 +4,7 @@ import { getRegionalDexFieldName, getRegionalDexKey, hasRegionalDex -} from '../../src/lib/utils/regionalDexMapping'; +} from '$lib/utils/regionalDexMapping'; describe('regional dex mapping', () => { it.each([ diff --git a/vitest.config.mts b/vitest.config.mts index 9171383..c3b915f 100644 --- a/vitest.config.mts +++ b/vitest.config.mts @@ -15,19 +15,17 @@ export default defineConfig({ provider: 'v8', reporter: ['text', 'json-summary', 'html'], reportsDirectory: 'coverage', - include: [ - 'src/lib/utils/boxPlacement.ts', - 'src/lib/utils/catchRecordWriteQueue.ts', - 'src/lib/utils/oauthState.ts', - 'src/lib/utils/regionalDexMapping.ts', - 'src/lib/services/PokedexExportFormatting.ts' - ], + // The whole library surface is measured, so anything new and untested drags the + // numbers down instead of being invisible to the gate. Excluded here: type-only + // models, and the store/action modules that only run in a browser. + include: ['src/lib/**/*.ts'], + exclude: ['src/lib/models/**', 'src/lib/stores/**', 'src/lib/actions/**'], + // Set to the measured baseline. Ratchet these up as coverage grows; never down. thresholds: { - perFile: true, - statements: 90, - functions: 90, - lines: 90, - branches: 80 + statements: 33, + functions: 73, + lines: 33, + branches: 79 } } }