496 lines
33 KiB
Markdown
496 lines
33 KiB
Markdown
|
|
---
|
|||
|
|
convoy: adopt-playwright-smoke
|
|||
|
|
brief_number: 1
|
|||
|
|
depends_on: []
|
|||
|
|
files:
|
|||
|
|
- package.json
|
|||
|
|
- package-lock.json
|
|||
|
|
- playwright.config.js
|
|||
|
|
- tests/visual/homepage.spec.ts
|
|||
|
|
- .gitignore
|
|||
|
|
---
|
|||
|
|
|
|||
|
|
# Brief 1: Install `@playwright/test`, ship `playwright.config.js`, add the homepage visual spec — so the post-PR-17 `Preview smoke` and `Screenshot diff` workflows reach `npx playwright test` and execute against the Vercel preview
|
|||
|
|
|
|||
|
|
## Goal (1 sentence)
|
|||
|
|
|
|||
|
|
Wire `@playwright/test@^1.60.0` into devDeps, create `playwright.config.js` with two projects (`smoke` + `visual`) and a CI-gated `x-vercel-protection-bypass` header per `playwright.config` Decision 2, add a single `tests/visual/homepage.spec.ts` baseline-bearing screenshot test per Decision 3, add three `npm` scripts per Decision 6, and ignore Playwright's local-run artifacts in `.gitignore` — without renaming or editing the existing `tests/smoke/app.smoke.spec.ts`, without touching `eslint.config.mjs`, and without committing any baseline images (operator seeds those in a follow-up Linux-Docker run per Decision 4).
|
|||
|
|
|
|||
|
|
## Files in scope (do not edit anything else)
|
|||
|
|
|
|||
|
|
- `package.json` — modified.
|
|||
|
|
- `package-lock.json` — modified (regenerated by `npm install`; commit in the SAME commit as `package.json` to keep `npm ci` happy in CI per Risk R7).
|
|||
|
|
- `playwright.config.js` — new (root level).
|
|||
|
|
- `tests/visual/homepage.spec.ts` — new.
|
|||
|
|
- `.gitignore` — modified.
|
|||
|
|
|
|||
|
|
**Files explicitly out of scope** (do not touch even if it seems related):
|
|||
|
|
|
|||
|
|
- `tests/smoke/app.smoke.spec.ts` — exists, stays as-is per Decision 1. NOT renamed to `.js`. NOT edited.
|
|||
|
|
- `eslint.config.mjs` — empirically verified to parse the existing `.ts` spec cleanly (Decision 5 + Boot-the-brief finding #2 below). NOT touched.
|
|||
|
|
- `vitest.config.js`, `test/setup.js`, any file under `test/` (vitest's home) — different runner; out of scope.
|
|||
|
|
- `.github/workflows/preview-smoke.yml`, `.github/workflows/visual-diff.yml`, `.github/workflows/ci.yml` — all workflow YAML is owned by other convoys (PR #17 / PR #16 / `fix-lint-baseline`); zero touches here.
|
|||
|
|
- `AGENTS.md` § 7 (the seed-on-Linux workflow + the local-run command table) — that's the doc-writer pass at convoy close, NOT this brief.
|
|||
|
|
- `tests/visual/__screenshots__/` — do NOT create or commit baselines in this convoy per Decision 4. Operator runs `npm run test:visual:update` in a Linux Docker env (`mcr.microsoft.com/playwright:v1.60.0-noble`) and commits in a follow-up `seed-visual-baselines-on-linux` PR.
|
|||
|
|
- `pages/api/health.js` — already exists and already returns 200 anonymously (verified by reading the 7-line source). Do NOT modify or move.
|
|||
|
|
- `next.config.js`, `tailwind.config.js`, `postcss.config.js` — unrelated; zero touches.
|
|||
|
|
|
|||
|
|
## Conventions to follow
|
|||
|
|
|
|||
|
|
### Decisions from the convoy file (cite when implementing)
|
|||
|
|
|
|||
|
|
- **Decision 1 (`.convoys/adopt-playwright-smoke.md` § Decisions, post-IA round):** keep `tests/smoke/app.smoke.spec.ts` as `.ts`. New visual spec also `.ts` (`tests/visual/homepage.spec.ts`).
|
|||
|
|
- **Decision 2:** in `playwright.config.js`, `throw` at config load when `process.env.CI === 'true'` AND `VERCEL_AUTOMATION_BYPASS_SECRET` is missing/empty. In non-CI, `console.warn` once and continue with `extraHTTPHeaders` undefined. Error message names the env var, the rotation command, and points at `AGENTS.md § 7`.
|
|||
|
|
- **Decision 3:** two projects — `smoke` (`testMatch: 'smoke/**/*.spec.@(ts|js)'`) and `visual` (`testMatch: 'visual/**/*.spec.@(ts|js)'`). Both share the same `use:` block (no per-project `use:` overrides).
|
|||
|
|
- **Decision 4:** do NOT commit baselines. First CI run of `Screenshot diff` will fail at the test step; the existing `continue-on-error: true` swallows the failure and the comment-on-PR step posts "Visual Diff — view run" with empty artifacts. That's the documented end state of this brief.
|
|||
|
|
- **Decision 5:** no `eslint.config.mjs` change. Verified empirically that `npx eslint tests/smoke/app.smoke.spec.ts` exits 0 against the current config; the new `tests/visual/homepage.spec.ts` is structurally identical and will also pass.
|
|||
|
|
- **Decision 6:** add three scripts — `test:smoke`, `test:visual`, `test:visual:update`. No auto-boot of `next dev`.
|
|||
|
|
|
|||
|
|
### Repo conventions (cite + match)
|
|||
|
|
|
|||
|
|
- **ESM module style.** `package.json` has `"type": "module"`. `vitest.config.js` and `next.config.js` are both ESM (`import { defineConfig } from '...'; export default defineConfig({...})`). `playwright.config.js` MUST match this shape:
|
|||
|
|
```js
|
|||
|
|
import { defineConfig } from '@playwright/test';
|
|||
|
|
export default defineConfig({ ... });
|
|||
|
|
```
|
|||
|
|
- **No-go zones (`.cursor/rules/no-go-zones.mdc`).** None of the files in scope are listed. `playwright.config.js` does not exist yet. `tests/visual/` does not exist yet. `package.json` / `package-lock.json` / `.gitignore` are all editable per established convoy precedent (`bump-next-js`, `fix-auth-bypass`).
|
|||
|
|
- **Secret-handling discipline (AGENTS.md § 7):**
|
|||
|
|
- NEVER `console.log` / `echo` / write to a file any string containing `process.env.VERCEL_AUTOMATION_BYPASS_SECRET`.
|
|||
|
|
- The `console.warn` branch (Decision 2 dev path) must say "VERCEL_AUTOMATION_BYPASS_SECRET unset" — NOT print the value.
|
|||
|
|
- The fail-loud branch (Decision 2 CI path) `throw`s a string error message; the message names the env var by name but does NOT echo any value.
|
|||
|
|
- **Style match.** Two close precedents for ESM config files: `vitest.config.js` (heavy inline comments explaining each non-default choice) and `next.config.js` (terse). `playwright.config.js` should follow `vitest.config.js`'s commented style because the convoy's AC #8 requires it ("`playwright.config.js` is well-commented (every non-obvious choice has a one-line explanation), so the next agent doesn't need to re-derive context from the convoy file").
|
|||
|
|
|
|||
|
|
## Acceptance criteria
|
|||
|
|
|
|||
|
|
### `package.json`
|
|||
|
|
|
|||
|
|
- [ ] Add `"@playwright/test": "^1.60.0"` to `devDependencies` (alphabetical position — between `@neondatabase/serverless` is in `dependencies`, so in `devDependencies` it lands between `@testing-library/react` and `autoprefixer`; verify alphabetization is preserved).
|
|||
|
|
|
|||
|
|
Pin rationale: `^1.60.0` accepts patch updates (1.60.x), matches the workflow's `npx playwright install --with-deps chromium` (which downloads the bundled browser matching the installed package version), and 1.60.0 is 13 days old at time of writing — fresh but not bleeding-edge. If `npm install` resolves to a newer 1.60.x patch, that's expected.
|
|||
|
|
|
|||
|
|
- [ ] Add three scripts to the `scripts` block, between `test:run` and the closing brace:
|
|||
|
|
|
|||
|
|
```json
|
|||
|
|
"test:smoke": "playwright test --project=smoke",
|
|||
|
|
"test:visual": "playwright test --project=visual",
|
|||
|
|
"test:visual:update": "playwright test --project=visual --update-snapshots"
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
Notes:
|
|||
|
|
- Use bare `playwright test`, NOT `npx playwright test`. With the dep installed locally, npm scripts resolve `playwright` from `node_modules/.bin` automatically. Matches the existing `test` / `test:run` shape (which uses bare `vitest`, not `npx vitest`).
|
|||
|
|
- `test:visual:update` is the exact command the visual workflow's PR-comment text instructs operators to run — keep the verbatim string match so a future operator can copy-paste from the comment.
|
|||
|
|
- Do NOT add `test:e2e`, `test:smoke:local`, or any auto-boot variant. Decision 6 explicitly rejected those.
|
|||
|
|
|
|||
|
|
- [ ] No other changes to `package.json`. `dependencies`, `name`, `version`, `private`, `type` all stay byte-identical.
|
|||
|
|
|
|||
|
|
### `package-lock.json`
|
|||
|
|
|
|||
|
|
- [ ] Regenerated by running `npm install` after the `package.json` edit. Commit the resulting lockfile in the SAME commit as the `package.json` change (Risk R7 in the convoy file). The diff will be large (Playwright pulls many transitive deps) but is mechanical; do NOT hand-edit it.
|
|||
|
|
|
|||
|
|
- [ ] Verify lockfile sync by running `npm ci` locally after commit: it should succeed with exit 0. If it errors with "Missing: ... from lock file" or "Invalid: lock file's ... does not satisfy package.json", the regeneration is incomplete — re-run `npm install` and re-commit.
|
|||
|
|
|
|||
|
|
### `playwright.config.js` (new file at repo root)
|
|||
|
|
|
|||
|
|
- [ ] Verbatim shape (commented for AC #8; the implementer is free to tighten wording but every non-obvious choice MUST have a one-line explanation):
|
|||
|
|
|
|||
|
|
```js
|
|||
|
|
// Playwright config for the post-PR-17 `Preview smoke` and
|
|||
|
|
// `Screenshot diff` workflows. ESM per the repo's
|
|||
|
|
// `"type": "module"` setting in package.json. Companion docs:
|
|||
|
|
// `.convoys/adopt-playwright-smoke.md` (Decisions 1-6),
|
|||
|
|
// AGENTS.md § 7 (Vercel preview bypass conventions).
|
|||
|
|
|
|||
|
|
import { defineConfig } from '@playwright/test';
|
|||
|
|
|
|||
|
|
const BASE_URL = process.env.BASE_URL ?? 'http://localhost:3000';
|
|||
|
|
const BYPASS_SECRET = process.env.VERCEL_AUTOMATION_BYPASS_SECRET;
|
|||
|
|
|
|||
|
|
// `CI === 'true'` is the canonical GitHub Actions signal (set by
|
|||
|
|
// the runner). Playwright config has no Next.js context, so
|
|||
|
|
// `NODE_ENV` is not reliable here. Mirrors the rate-limit.js
|
|||
|
|
// fail-closed pattern documented in AGENTS.md Gotcha #12.
|
|||
|
|
const IS_CI = process.env.CI === 'true';
|
|||
|
|
|
|||
|
|
if (IS_CI && !BYPASS_SECRET) {
|
|||
|
|
// Fail loud in CI per Decision 2. The workflow's `env:` block
|
|||
|
|
// (preview-smoke.yml line 101, visual-diff.yml line 85) maps
|
|||
|
|
// `secrets.VERCEL_AUTOMATION_BYPASS_SECRET` into the process
|
|||
|
|
// env; if it's empty here, the secret is unseeded or the
|
|||
|
|
// workflow YAML drift broke the mapping.
|
|||
|
|
throw new Error(
|
|||
|
|
'VERCEL_AUTOMATION_BYPASS_SECRET is required in CI to reach ' +
|
|||
|
|
'Vercel-Protection-protected preview deployments. ' +
|
|||
|
|
'Reseed via: gh secret set VERCEL_AUTOMATION_BYPASS_SECRET --body "<value>". ' +
|
|||
|
|
'See AGENTS.md § 7 for the full plumbing context.'
|
|||
|
|
);
|
|||
|
|
}
|
|||
|
|
|
|||
|
|
if (!BYPASS_SECRET && !IS_CI) {
|
|||
|
|
// Dev fallback per Decision 2 — warn once at config load,
|
|||
|
|
// proceed without the header. Local runs target localhost
|
|||
|
|
// (no preview protection) or a non-protected URL.
|
|||
|
|
// eslint-disable-next-line no-console -- intentional one-shot warning at config load
|
|||
|
|
console.warn(
|
|||
|
|
'[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ' +
|
|||
|
|
'running without the Vercel bypass header. Targets a non-protected ' +
|
|||
|
|
'URL (e.g. http://localhost:3000). Hitting a protected preview without ' +
|
|||
|
|
'this header will return Vercel\'s SSO challenge page.'
|
|||
|
|
);
|
|||
|
|
}
|
|||
|
|
|
|||
|
|
export default defineConfig({
|
|||
|
|
// Both `tests/smoke/` and `tests/visual/` live under `tests/`.
|
|||
|
|
// Project-level `testMatch` (below) partitions them so the
|
|||
|
|
// two workflows (`--project=smoke` and `--project=visual`)
|
|||
|
|
// each see only the specs they should run.
|
|||
|
|
testDir: './tests',
|
|||
|
|
|
|||
|
|
// Smoke + visual specs are independent; parallelism within a
|
|||
|
|
// single spec adds no value here and would complicate the
|
|||
|
|
// per-test screenshot baseline lifecycle.
|
|||
|
|
fullyParallel: false,
|
|||
|
|
workers: IS_CI ? 1 : undefined,
|
|||
|
|
|
|||
|
|
// One retry in CI handles transient Vercel preview flakes
|
|||
|
|
// (cold-start, DNS propagation). Local: zero retries — fail
|
|||
|
|
// fast so the dev sees the issue immediately.
|
|||
|
|
retries: IS_CI ? 1 : 0,
|
|||
|
|
|
|||
|
|
// 30s per test is plenty for the 3 smoke checks + 1 visual
|
|||
|
|
// screenshot. The convoy file's success metric is < 5min
|
|||
|
|
// total workflow runtime; per-test 30s is well inside that.
|
|||
|
|
timeout: 30_000,
|
|||
|
|
expect: { timeout: 10_000 },
|
|||
|
|
|
|||
|
|
// List reporter in dev for human readability; add HTML in CI
|
|||
|
|
// so the `Upload Playwright report on failure` step
|
|||
|
|
// (preview-smoke.yml line 104) has a populated `playwright-report/`
|
|||
|
|
// to upload. `open: 'never'` keeps the HTML from auto-launching
|
|||
|
|
// a browser tab in headless CI.
|
|||
|
|
reporter: IS_CI ? [['list'], ['html', { open: 'never' }]] : 'list',
|
|||
|
|
|
|||
|
|
// Visual baselines live at `tests/visual/__screenshots__/<arg>{ext}`.
|
|||
|
|
// Workflow `visual-diff.yml` line 94 uploads this exact path as the
|
|||
|
|
// artifact — keep them aligned. {arg} is the snapshot name from
|
|||
|
|
// `toHaveScreenshot('home.png')` without the extension; {ext} is
|
|||
|
|
// the extension with the leading dot.
|
|||
|
|
//
|
|||
|
|
// Cross-platform note (Risk R3): this template drops Playwright's
|
|||
|
|
// default `-<browser>-<platform>` suffix. That means a Mac dev
|
|||
|
|
// running `npm run test:visual:update` overwrites the Linux-CI
|
|||
|
|
// baseline. Per Decision 4, we don't commit baselines this convoy;
|
|||
|
|
// operator seeds via `mcr.microsoft.com/playwright:v1.60.0-noble`
|
|||
|
|
// Docker on Linux. Multi-platform support is the
|
|||
|
|
// `seed-visual-baselines-on-linux` follow-up convoy's job.
|
|||
|
|
snapshotPathTemplate: 'tests/visual/__screenshots__/{arg}{ext}',
|
|||
|
|
|
|||
|
|
use: {
|
|||
|
|
baseURL: BASE_URL,
|
|||
|
|
// Headers apply to BOTH browser `page.goto(...)` calls AND the
|
|||
|
|
// test-level `request` fixture's APIRequestContext (verified
|
|||
|
|
// against Playwright docs: `testOptions.extraHTTPHeaders` is
|
|||
|
|
// shared between browser context and APIRequestContext
|
|||
|
|
// construction). This is why `tests/smoke/app.smoke.spec.ts`'s
|
|||
|
|
// third test (`request.get('/api/health')`) reaches the
|
|||
|
|
// protected preview without re-injecting the header in the
|
|||
|
|
// spec body. If a future hotfix shows the header NOT
|
|||
|
|
// propagating to APIRequestContext, see Risk R1 in the
|
|||
|
|
// convoy file's Architecture section.
|
|||
|
|
extraHTTPHeaders: BYPASS_SECRET
|
|||
|
|
? { 'x-vercel-protection-bypass': BYPASS_SECRET }
|
|||
|
|
: undefined,
|
|||
|
|
// Trace OFF this convoy. Enabling it would land the bypass
|
|||
|
|
// header in the HAR payload (Risk R6); a future polish convoy
|
|||
|
|
// owns the trace-on + HAR-sanitization decision.
|
|||
|
|
trace: 'off',
|
|||
|
|
screenshot: 'off',
|
|||
|
|
video: 'off',
|
|||
|
|
},
|
|||
|
|
|
|||
|
|
projects: [
|
|||
|
|
{
|
|||
|
|
// `Preview smoke` workflow invokes `--project=smoke` (per
|
|||
|
|
// preview-smoke.yml line 98). The testMatch keeps the
|
|||
|
|
// visual specs out of this project.
|
|||
|
|
name: 'smoke',
|
|||
|
|
testMatch: 'smoke/**/*.spec.@(ts|js)',
|
|||
|
|
},
|
|||
|
|
{
|
|||
|
|
// `Screenshot diff` workflow invokes `--project=visual`
|
|||
|
|
// (per visual-diff.yml line 82). The testMatch keeps the
|
|||
|
|
// smoke specs out of this project (so a `--project=visual`
|
|||
|
|
// run doesn't redundantly execute the smoke tests).
|
|||
|
|
name: 'visual',
|
|||
|
|
testMatch: 'visual/**/*.spec.@(ts|js)',
|
|||
|
|
},
|
|||
|
|
],
|
|||
|
|
});
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
Two things the implementer can vary without breaking AC:
|
|||
|
|
|
|||
|
|
1. Comment wording — the substantive choices (predicate, error message contents, path template) must stay; the explanatory prose can be tightened.
|
|||
|
|
2. Property ordering inside `defineConfig({...})` — Playwright doesn't care; alphabetical or grouped-by-concern are both fine.
|
|||
|
|
|
|||
|
|
Things the implementer MUST NOT change:
|
|||
|
|
|
|||
|
|
- The `IS_CI` predicate (`=== 'true'`, not `=== 'true' || === '1'`).
|
|||
|
|
- The `throw` vs `console.warn` branch logic.
|
|||
|
|
- The `snapshotPathTemplate` value (the workflow's artifact upload path depends on it).
|
|||
|
|
- The two project names (`smoke`, `visual`) — workflow YAML invokes them verbatim.
|
|||
|
|
- The two `testMatch` patterns — partitioning is the whole reason for two projects.
|
|||
|
|
- The `trace: 'off'` setting (Risk R6).
|
|||
|
|
- `BYPASS_SECRET ? { ... } : undefined` ternary — passing `extraHTTPHeaders: { 'x-vercel-protection-bypass': undefined }` would still send the header with the literal string `'undefined'`, which 401s loudly. The conditional is load-bearing.
|
|||
|
|
|
|||
|
|
### `tests/visual/homepage.spec.ts` (new file)
|
|||
|
|
|
|||
|
|
- [ ] Verbatim shape:
|
|||
|
|
|
|||
|
|
```ts
|
|||
|
|
import { test, expect } from '@playwright/test';
|
|||
|
|
|
|||
|
|
/**
|
|||
|
|
* Visual baseline for the public homepage.
|
|||
|
|
*
|
|||
|
|
* FIRST RUN (no committed baseline yet):
|
|||
|
|
* The Screenshot diff workflow runs `playwright test --project=visual
|
|||
|
|
* --update-snapshots=none` (per .github/workflows/visual-diff.yml). With
|
|||
|
|
* no baseline file at `tests/visual/__screenshots__/home.png` AND the
|
|||
|
|
* `none` flag, this test FAILS — and that's the documented end state of
|
|||
|
|
* the `adopt-playwright-smoke` convoy (Decision 4 in
|
|||
|
|
* `.convoys/adopt-playwright-smoke.md`). The workflow's
|
|||
|
|
* `continue-on-error: true` swallows the failure and the comment-on-PR
|
|||
|
|
* step posts "Visual Diff — view run" with empty artifacts.
|
|||
|
|
*
|
|||
|
|
* SEEDING THE BASELINE (post-merge follow-up):
|
|||
|
|
* Run `npm run test:visual:update` in a Linux environment so the
|
|||
|
|
* generated PNG matches what CI will produce. The cleanest path is the
|
|||
|
|
* Playwright Docker image:
|
|||
|
|
*
|
|||
|
|
* docker run --rm -v "$PWD":/work -w /work \
|
|||
|
|
* mcr.microsoft.com/playwright:v1.60.0-noble \
|
|||
|
|
* sh -c "npm ci && BASE_URL=https://<preview>.vercel.app \
|
|||
|
|
* VERCEL_AUTOMATION_BYPASS_SECRET=<value> \
|
|||
|
|
* npm run test:visual:update"
|
|||
|
|
*
|
|||
|
|
* Then commit `tests/visual/__screenshots__/home.png`. This is tracked
|
|||
|
|
* as the `seed-visual-baselines-on-linux` follow-up convoy.
|
|||
|
|
*/
|
|||
|
|
const BASE = process.env.BASE_URL ?? 'http://localhost:3000';
|
|||
|
|
|
|||
|
|
test.describe('visual: public homepage', () => {
|
|||
|
|
test('home renders consistently against baseline', async ({ page }) => {
|
|||
|
|
await page.goto(BASE);
|
|||
|
|
await expect(page).toHaveScreenshot('home.png');
|
|||
|
|
});
|
|||
|
|
});
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
Notes:
|
|||
|
|
|
|||
|
|
- `BASE` constant mirrors the existing `tests/smoke/app.smoke.spec.ts` shape (line 11). Don't read `process.env.BASE_URL` inside the test body.
|
|||
|
|
- Snapshot name is the literal string `'home.png'`. The `snapshotPathTemplate` from `playwright.config.js` resolves this to `tests/visual/__screenshots__/home.png`.
|
|||
|
|
- `test.describe` block name `'visual: public homepage'` matches the smoke spec's naming pattern (`'smoke: app boots and core pages render'`).
|
|||
|
|
- Default viewport (1280×720) is fine for a first-pass baseline; do NOT set `viewport:` overrides this convoy.
|
|||
|
|
- Do NOT add `await page.waitForLoadState('networkidle')` — networkidle is unreliable on a JS-heavy Next.js app and causes false flakes. `page.goto` already waits for `load` by default; that's enough for a homepage baseline.
|
|||
|
|
- Do NOT add `{ fullPage: true }` to `toHaveScreenshot`. Default (viewport-only) keeps the baseline file small (~50KB) and easier to review on PRs.
|
|||
|
|
|
|||
|
|
### `.gitignore`
|
|||
|
|
|
|||
|
|
- [ ] Add the following block AFTER the existing `.code-review-graph/` line (current line 39):
|
|||
|
|
|
|||
|
|
```gitignore
|
|||
|
|
|
|||
|
|
# Playwright test runner artifacts (generated on every local run;
|
|||
|
|
# never committed). Baselines under `tests/visual/__screenshots__/`
|
|||
|
|
# are EXPLICITLY NOT ignored — they must be committed when they exist.
|
|||
|
|
/playwright-report/
|
|||
|
|
/test-results/
|
|||
|
|
/.playwright/
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
Three rules:
|
|||
|
|
|
|||
|
|
1. `/playwright-report/` — the HTML reporter's output (config `reporter` block).
|
|||
|
|
2. `/test-results/` — Playwright's default `--output` directory for trace/screenshot/video on failure.
|
|||
|
|
3. `/.playwright/` — Playwright's local browser cache and other internal state.
|
|||
|
|
|
|||
|
|
Do NOT add `tests/visual/__screenshots__/` to `.gitignore`. Baselines are committed artifacts (per Decision 4 they don't exist yet, but when they do, they MUST be tracked). This is a boot-the-brief finding — see Finding 3 below.
|
|||
|
|
|
|||
|
|
### Cross-file checks
|
|||
|
|
|
|||
|
|
- [ ] **No secret leaks.** Grep before commit:
|
|||
|
|
```bash
|
|||
|
|
rg -i 'console\.log.*VERCEL_AUTOMATION_BYPASS_SECRET' playwright.config.js tests/
|
|||
|
|
```
|
|||
|
|
Expected: zero matches. The two intentional references (the `if (IS_CI && !BYPASS_SECRET)` `throw` and the `if (!BYPASS_SECRET && !IS_CI)` `console.warn`) name the env var but do NOT echo its value.
|
|||
|
|
- [ ] **No baseline files committed.** Grep before commit:
|
|||
|
|
```bash
|
|||
|
|
ls tests/visual/__screenshots__/ 2>/dev/null && echo "FAIL: directory exists with files; do NOT commit" || echo "OK: no baselines"
|
|||
|
|
```
|
|||
|
|
Expected: `OK: no baselines`. If you ran `npm run test:visual:update` locally to verify the spec resolves, delete the generated PNG before commit.
|
|||
|
|
- [ ] **`npm run test:run` exit 0** (vitest baseline unchanged):
|
|||
|
|
```bash
|
|||
|
|
npm run test:run
|
|||
|
|
```
|
|||
|
|
Expected: `Tests 21 passed (21)` per the existing vitest suite (16 auth + 5 Layout). If any vitest test fails, investigate before merge — this brief should be a pure additive change with no vitest impact.
|
|||
|
|
- [ ] **`npm run lint` exit code unchanged** (Decision 5):
|
|||
|
|
```bash
|
|||
|
|
npm run lint 2>&1 | tail -3
|
|||
|
|
```
|
|||
|
|
Expected: `✖ 128 problems (81 errors, 47 warnings)` — the established baseline. If it grows by 1-2 new problems from the new visual spec, investigate (Risk R8); the architect's empirical check on the existing `.ts` smoke spec showed `exit 0` so the new spec should be silent too.
|
|||
|
|
- [ ] **`npm run build` exit 0** — Playwright is a devDep and should not affect the Next.js build graph. Run as a smoke check:
|
|||
|
|
```bash
|
|||
|
|
npm run build 2>&1 | tail -5
|
|||
|
|
```
|
|||
|
|
Expected: build success (Turbopack compile ~1-2s, 23 static pages + 47 API routes per the post-`bump-next-js` baseline).
|
|||
|
|
- [ ] **Diff hygiene.** `git diff main..HEAD --stat` should show only the 5 files listed in the brief frontmatter. No whitespace-only changes elsewhere.
|
|||
|
|
|
|||
|
|
### Acceptance criterion #1 — end-state behavior
|
|||
|
|
|
|||
|
|
After this brief lands on the convoy branch and a Vercel preview deployment is published for the PR:
|
|||
|
|
|
|||
|
|
- [ ] **`Preview smoke` workflow:** wait-action succeeds in ≤90s (already proven by PR #17 at 194ms); `npm ci` succeeds; `npx playwright install --with-deps chromium` succeeds (~30-60s); `Run smoke tests` step REACHES `npx playwright test --project=smoke`; **3 smoke tests execute against the Vercel preview**. Pass/fail outcome:
|
|||
|
|
- **Most likely PASS**: home renders (existing pages don't 5xx), `/login` renders (existing page), `/api/health` returns 2xx (verified handler). If any of these fail, the failure is a real signal — investigate before declaring the brief incomplete.
|
|||
|
|
- Total workflow runtime: < 5 minutes (convoy success metric).
|
|||
|
|
- [ ] **`Screenshot diff` workflow:** wait-action succeeds; `npx playwright install` succeeds; `Capture screenshots (PR)` step REACHES `npx playwright test --project=visual --update-snapshots=none`. **The single visual test FAILS** because no baseline exists. `continue-on-error: true` swallows the failure. `Upload screenshots + diffs` step uploads `tests/visual/__screenshots__/` (empty) + `test-results/` (contains the failure detail). `Comment on PR with diff link` step posts "## Visual Diff" comment with the run URL. **This is the documented end state of this convoy** per Decision 4 — operator follows up with the seed-on-Linux PR.
|
|||
|
|
- [ ] **The bypass secret does NOT appear in any line of either workflow's run log.** Same verification pattern as the `fix-vercel-deployment-protection-in-ci` brief (download logs, grep for first 8 chars of the secret value, expect zero hits). GitHub Actions auto-masks registered secrets; this brief's contribution (Decision 2's `throw` and `console.warn` branches) does not interpolate the secret value into any string.
|
|||
|
|
|
|||
|
|
## Manual verification (in addition to CI on push)
|
|||
|
|
|
|||
|
|
Run these in order. Paste relevant output (with secrets redacted) into the PR description.
|
|||
|
|
|
|||
|
|
- [ ] **Local install + lockfile parity.**
|
|||
|
|
```bash
|
|||
|
|
npm install
|
|||
|
|
npm ci # second run, verify lockfile is in sync
|
|||
|
|
```
|
|||
|
|
Expected: both succeed exit 0. `npm ci` is the critical one — if it errors, the lockfile is out of sync and the implementer needs to re-run `npm install` and re-commit.
|
|||
|
|
|
|||
|
|
- [ ] **Config loads cleanly.**
|
|||
|
|
```bash
|
|||
|
|
npx playwright --version
|
|||
|
|
npx playwright test --list --project=smoke
|
|||
|
|
npx playwright test --list --project=visual
|
|||
|
|
```
|
|||
|
|
Expected:
|
|||
|
|
- `Version 1.60.x` (or later 1.60 patch).
|
|||
|
|
- `--list --project=smoke` enumerates 3 tests from `tests/smoke/app.smoke.spec.ts`.
|
|||
|
|
- `--list --project=visual` enumerates 1 test from `tests/visual/homepage.spec.ts`.
|
|||
|
|
If `--list` shows specs in the wrong project, the `testMatch` patterns are wrong — fix before commit.
|
|||
|
|
|
|||
|
|
- [ ] **Config Decision-2 fail-loud branch.** Simulate CI without the secret:
|
|||
|
|
```bash
|
|||
|
|
unset VERCEL_AUTOMATION_BYPASS_SECRET
|
|||
|
|
CI=true npx playwright test --list --project=smoke
|
|||
|
|
```
|
|||
|
|
Expected: exit code 1, error includes "VERCEL_AUTOMATION_BYPASS_SECRET is required in CI" and the `gh secret set` rotation command. If it does NOT error, the Decision 2 logic is wrong.
|
|||
|
|
|
|||
|
|
- [ ] **Config Decision-2 warn-and-continue branch.** Simulate dev without the secret:
|
|||
|
|
```bash
|
|||
|
|
unset VERCEL_AUTOMATION_BYPASS_SECRET
|
|||
|
|
unset CI
|
|||
|
|
npx playwright test --list --project=smoke
|
|||
|
|
```
|
|||
|
|
Expected: stderr includes the `[playwright.config] VERCEL_AUTOMATION_BYPASS_SECRET unset — ...` warning, command proceeds and lists tests with exit 0.
|
|||
|
|
|
|||
|
|
- [ ] **Smoke spec runs against localhost.** In one terminal:
|
|||
|
|
```bash
|
|||
|
|
npm run dev
|
|||
|
|
```
|
|||
|
|
In another:
|
|||
|
|
```bash
|
|||
|
|
BASE_URL=http://localhost:3000 npm run test:smoke
|
|||
|
|
```
|
|||
|
|
Expected: 3 tests pass against the local Next.js dev server. If any fail locally, the spec body has a real issue OR a local config drift — investigate before pushing.
|
|||
|
|
|
|||
|
|
- [ ] **Visual spec fail-on-missing-baseline behavior.** Against localhost:
|
|||
|
|
```bash
|
|||
|
|
BASE_URL=http://localhost:3000 npx playwright test --project=visual --update-snapshots=none
|
|||
|
|
```
|
|||
|
|
Expected: 1 test FAILS with "A snapshot doesn't exist at tests/visual/__screenshots__/home.png". This is the desired Decision-4 behavior. If you accidentally generate a baseline locally with `--update-snapshots`, delete it before commit:
|
|||
|
|
```bash
|
|||
|
|
rm -rf tests/visual/__screenshots__/
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
- [ ] **`.gitignore` actually ignores the right things.** After a local Playwright run (which creates `playwright-report/` and `test-results/`):
|
|||
|
|
```bash
|
|||
|
|
git status --short
|
|||
|
|
```
|
|||
|
|
Expected: no `playwright-report/` or `test-results/` entries. If they appear, the `.gitignore` entries are wrong (missing leading `/` or wrong directory name).
|
|||
|
|
|
|||
|
|
- [ ] **No bypass-secret leak in spec output.** After running `npm run test:smoke` with the bypass var set:
|
|||
|
|
```bash
|
|||
|
|
VERCEL_AUTOMATION_BYPASS_SECRET=fake-secret-value BASE_URL=http://localhost:3000 \
|
|||
|
|
npm run test:smoke 2>&1 | grep -i 'fake-secret-value' && echo "FAIL: secret leaked" || echo "OK: secret not in output"
|
|||
|
|
```
|
|||
|
|
Expected: `OK: secret not in output`. (`fake-secret-value` is just a probe string for the grep; the real secret never enters this command.)
|
|||
|
|
|
|||
|
|
- [ ] **Push and observe the first workflow run.** From `convoy/adopt-playwright-smoke`:
|
|||
|
|
```bash
|
|||
|
|
git push -u origin HEAD
|
|||
|
|
```
|
|||
|
|
Then watch both workflows. Expect the end-state described in Acceptance criterion #1 above: smoke passes (or fails on a real assertion), visual fails on missing baseline + posts the comment.
|
|||
|
|
|
|||
|
|
## Boot-the-brief findings (preempted by the architect; do not re-investigate)
|
|||
|
|
|
|||
|
|
### Finding 1 — `@playwright/test@^1.60.0` resolves cleanly against the current dep tree
|
|||
|
|
|
|||
|
|
`npm view @playwright/test version` → `1.60.0` (released 2026-05-11). No peer-dep conflicts with the existing `react@^18.3.1` / `next@^16.2.6` / `eslint@^9.39.4` graph (Playwright has no React or Next peers). The bundled Chromium binary downloaded by `npx playwright install --with-deps chromium` (already in both workflow steps — see `preview-smoke.yml` line 95 and `visual-diff.yml` line 79) matches the installed package version, so no version drift between the JS API and the browser binary.
|
|||
|
|
|
|||
|
|
### Finding 2 — ESLint already parses `.ts` test files cleanly
|
|||
|
|
|
|||
|
|
Verified 2026-05-24 on the convoy branch HEAD (`c8f1541`):
|
|||
|
|
|
|||
|
|
```bash
|
|||
|
|
$ npx eslint tests/smoke/app.smoke.spec.ts
|
|||
|
|
$ echo $?
|
|||
|
|
0
|
|||
|
|
```
|
|||
|
|
|
|||
|
|
Zero output, exit 0. The `eslint-config-next/core-web-vitals` chain bundled by `eslint-config-next@16` carries `typescript-eslint` as a hard dep (one of the reasons `typescript@^5.9.3` is in `devDependencies` per AGENTS.md Gotcha #9). The new `tests/visual/homepage.spec.ts` is structurally identical to the existing smoke spec (same imports, same patterns) so will also lint clean. **No `eslint.config.mjs` change is needed** (Decision 5).
|
|||
|
|
|
|||
|
|
### Finding 3 — `tests/visual/__screenshots__/` MUST NOT be in `.gitignore`
|
|||
|
|
|
|||
|
|
Default Playwright snapshot behavior: baselines are committed source-of-truth (visual regression depends on having a known-good reference). The convoy file's success metric requires the visual workflow's `Upload screenshots + diffs` step (visual-diff.yml line 88-96) to find files at `tests/visual/__screenshots__/` once baselines exist. Adding the path to `.gitignore` would silently break that step in any future PR that touches `pages/**` after baselines are committed in the follow-up convoy.
|
|||
|
|
|
|||
|
|
### Finding 4 — `extraHTTPHeaders` applies to the test-level `request` fixture
|
|||
|
|
|
|||
|
|
Per [Playwright fixtures docs](https://playwright.dev/docs/api/class-fixtures#fixtures-request), the test-level `request` fixture is "Isolated APIRequestContext instance for each test." Per [testOptions docs](https://playwright.dev/docs/api/class-testoptions), `extraHTTPHeaders` is "An object containing additional HTTP headers to be sent with **every request**." The testOptions surface is shared between browser context and APIRequestContext construction — the `request` fixture inherits the header. **This is why `tests/smoke/app.smoke.spec.ts`'s third test (`request.get('/api/health')`) reaches the protected preview without re-injecting the header in the spec body.** See Risk R1 in the convoy file's Architecture section for the hotfix path if this propagation breaks empirically.
|
|||
|
|
|
|||
|
|
### Finding 5 — `pages/api/health.js` returns 200 anonymously
|
|||
|
|
|
|||
|
|
The 7-line handler does NOT call `getUserFromRequest`, does NOT call `checkAuthRateLimit`, and does NOT gate on `req.method`. The Vercel preview's edge-protection layer terminates above the function: a request with the bypass header reaches the function as if anonymous, and the function returns `{ status: 'ok', ... }` with HTTP 200. The third smoke test (`request.get('/api/health')`) will pass cleanly against a Vercel preview, given the `extraHTTPHeaders` plumb (Finding 4).
|
|||
|
|
|
|||
|
|
### Finding 6 — Repo `"type": "module"` requires ESM-shape `playwright.config.js`
|
|||
|
|
|
|||
|
|
`package.json` line 5 declares `"type": "module"`. `vitest.config.js` and `next.config.js` both follow the ESM `import {...} from '...'; export default ...` shape. CommonJS-style `module.exports = { ... }` in `playwright.config.js` would throw at load time with `ReferenceError: module is not defined in ES module scope`. The spec'd `playwright.config.js` matches the ESM shape — do NOT regress to CommonJS.
|
|||
|
|
|
|||
|
|
### Finding 7 — `snapshotPathTemplate` token reference
|
|||
|
|
|
|||
|
|
Per [Playwright `testProject.snapshotPathTemplate` docs](https://playwright.dev/docs/api/class-testproject#test-project-snapshot-path-template), supported tokens are: `{arg}`, `{ext}`, `{projectName}`, `{snapshotDir}`, `{testDir}`, `{testFileDir}`, `{testFileName}`, `{testFilePath}`. **There is no `{platform}` or `{browserName}` token** — the default platform-suffix-in-filename behavior is built into the default template, and any custom template loses it. This is the structural reason for Risk R3 (cross-platform mismatch) and feeds Decision 4 (defer baseline commit to a Linux-Docker follow-up convoy).
|
|||
|
|
|
|||
|
|
### Finding 8 — `--update-snapshots=none` semantics
|
|||
|
|
|
|||
|
|
Per [Playwright CLI docs](https://playwright.dev/docs/test-cli): "Possible values are 'all', 'changed', 'missing', and 'none'. Running tests without the flag defaults to 'missing'; running tests with the flag but without a value defaults to 'changed'." Mode `none` (what `visual-diff.yml` line 82 uses) NEVER creates snapshots; missing snapshots fail the test. The workflow's `continue-on-error: true` + comment-on-PR step is the documented safety net. **First CI run of this brief on a touching-`pages/**` PR will produce a "Visual Diff — view run" comment with empty artifacts; that's the desired Decision-4 end state.**
|
|||
|
|
|
|||
|
|
## Out of scope (do not do these)
|
|||
|
|
|
|||
|
|
- [ ] Do not rename `tests/smoke/app.smoke.spec.ts` to `.js` (Decision 1).
|
|||
|
|
- [ ] Do not commit any baseline image to `tests/visual/__screenshots__/` (Decision 4).
|
|||
|
|
- [ ] Do not edit `eslint.config.mjs` (Decision 5 + Finding 2).
|
|||
|
|
- [ ] Do not edit any `.github/workflows/*.yml` file (those are owned by other convoys; the brief's job is to MAKE the YAML's `npx playwright test` invocation work, not to modify the YAML).
|
|||
|
|
- [ ] Do not edit `AGENTS.md` (doc-writer pass at convoy close owns § 7 updates and the seed-on-Linux instructions).
|
|||
|
|
- [ ] Do not add a `test:smoke:local` or `test:e2e` wrapper script that boots `next dev` automatically (Decision 6).
|
|||
|
|
- [ ] Do not add a `vitest` watch/setup change to support Playwright — the two runners stay independent (Test plan section of the convoy file).
|
|||
|
|
- [ ] Do not enable `trace: 'on'` or `trace: 'retain-on-failure'` in `playwright.config.js`'s `use:` block (Risk R6).
|
|||
|
|
- [ ] Do not add a `webServer:` block to `playwright.config.js` (auto-starts Next.js — explicitly rejected by Decision 6).
|
|||
|
|
- [ ] Do not pin `@playwright/test` to an exact version (`1.60.0` without the `^`) — patch upgrades are desired (Risk R9 mitigation path).
|
|||
|
|
- [ ] Do not add a `tsconfig.json` to the repo just because the new spec is `.ts`. Lint already accepts it; tsc-noEmit is not run in CI; the JS-only repo policy (AGENTS.md Gotcha #9) explicitly defers TypeScript adoption to a separate convoy.
|
|||
|
|
- [ ] Do not bump `node-version: '20'` in any workflow (out of scope; Vercel default is 20).
|
|||
|
|
- [ ] Do not run `npm audit fix` as part of this brief. If `npm install` surfaces audit warnings, note them in the PR description but do NOT take action — audit churn is a separate concern.
|
|||
|
|
|
|||
|
|
## Rationale (≤3 sentences)
|
|||
|
|
|
|||
|
|
The post-PR-17 workflows already invoke `npx playwright test --project={smoke,visual}` and already export `BASE_URL` + `VERCEL_AUTOMATION_BYPASS_SECRET` to the test step's `env:` — this brief is the small bridge that makes those invocations actually find a config, a dep, and a runnable visual spec. Bundling the visual spec with the config in one brief is the right call because they're semantically coupled (the visual spec depends on the `visual` project being declared in the config, which depends on `@playwright/test` being installed); splitting would force interim states that don't compile or run. Decision 4 (no baselines now) and Decision 6 (no auto-boot wrapper) explicitly keep the surface area small so the convoy ships in one PR; baseline curation and DX wrappers are queued as follow-up convoys (`seed-visual-baselines-on-linux`, possibly `adopt-test-smoke-local`) that operators can take or leave based on actual usage friction.
|