diff --git a/.claude/skills/frontend-htmx/SKILL.md b/.claude/skills/frontend-htmx/SKILL.md index 8c8103fb..624e9bce 100644 --- a/.claude/skills/frontend-htmx/SKILL.md +++ b/.claude/skills/frontend-htmx/SKILL.md @@ -62,9 +62,11 @@ Server can control behavior via htmx response headers (`HX-Reswap`, `HX-Redirect ## Checking a frontend change -The browser suite (`e2e/browser`, from W6 on; `docs/testing.md` has the "Browser tests" section) runs the -real app with freshly built assets in Chromium. It fails on console errors, uncaught exceptions and CSP -violations on every page, so an inline script or a style without the nonce shows up without a dedicated test. +The browser suite (`e2e/browser`; `docs/testing.md` has the "Browser tests" section with a worked example) +runs the real app with freshly built assets in Chromium. It fails on console errors, uncaught exceptions, +CSP violations and failed or 404/5xx requests on every page, so an inline script or a style without the +nonce shows up without a dedicated test. A test that provokes an error on purpose exempts it with +`browser.Allow`. - `make test-ui RUN=''` while iterating; `HEADED=1 SLOWMO=250` to watch it run. The target runs `yarn build` first. Run the whole suite once before committing. diff --git a/.claude/skills/test-failure/SKILL.md b/.claude/skills/test-failure/SKILL.md index b1a3412d..5c0fc8db 100644 --- a/.claude/skills/test-failure/SKILL.md +++ b/.claude/skills/test-failure/SKILL.md @@ -14,9 +14,10 @@ Goal: find the cause while reading as little as possible. Stop at the first step 3. **One test, verbose:** `go test ./pkg/x/ -run '^TestName$' -count=1 -v 2>&1 | tail -n 40`. Add `/subtest_name` to the `-run` pattern for table tests. 4. **Search the log, don't print it:** `grep -n -A5 'TestName' `. Never `cat` a log. - **Browser tests** (`e2e/browser`, `make test-ui`): the report line names a screenshot and a trace. Look - at the screenshot first (the Read tool shows images), then the guard message: console error, CSP - violation or failed request. Open the trace only if both leave it unclear. Re-run one test with + **Browser tests** (`e2e/browser`, `make test-ui`; see "Browser tests" in `docs/testing.md`): the + report line names a screenshot and a trace in `.ui-artifacts/`. Look at the screenshot first (the Read + tool shows images), then the guard message (`browser: console.error: …`, a CSP violation, a failed + request or a 404/5xx response). Open the trace only if both leave it unclear. Re-run one test with `make test-ui RUN='^TestName$'`. Stale assets are a common cause, and `make test-ui` rebuilds them. A locator timeout usually means the markup changed, not that the app is slow; don't raise timeouts. An `architecture` test failure (`pkg/arch`, from RS on) names the package and the forbidden import: diff --git a/.claude/skills/wave-run/SKILL.md b/.claude/skills/wave-run/SKILL.md index faca2f0f..bd3bcbab 100644 --- a/.claude/skills/wave-run/SKILL.md +++ b/.claude/skills/wave-run/SKILL.md @@ -53,6 +53,10 @@ subagents, whose context is thrown away. the plan says so (contract-defining work), or after a task has failed twice. - Tasks that own disjoint files go out in **one message** with several `Agent` calls, `subagent_type: "general-purpose"`. Never `fork`: a fork drags the coordinator's context along. +- Parallel tasks that write files in **one package** (W3's `e2e`, W6's `e2e/browser`) each iterate under + their own build tag (`//go:build browser && b3`, run with `-tags browser,b3`) and switch to the shared + tag before reporting, so one agent's half-written file doesn't break the others' compile. Shared build + steps such as `yarn build` run once in the coordinator before dispatch, never in each agent. - A broad question ("where is X used across the handlers?") goes to an `Explore` subagent, which returns the answer rather than the files. The coordinator's own lookups use the LSP tool. - **Subagents run no git commands** and don't touch `go.mod` (only W0 and W4.S2 do, each as a single task). @@ -88,14 +92,15 @@ Variations by wave: - **W6 (browser):** step 1 is `make test-ui RUN=`, then again with `COUNT=3` (a flaky test is sent back, not accepted). The mutation check breaks a Stimulus - controller or an htmx attribute the task covers. + controller or an htmx attribute the task covers. At the end of the wave, empty each controller's + `connect()` in turn and run the suite: a controller that survives is untested, whatever the reports say. - **R-waves and RS (refactors):** replace step 1 with `make test-q PKG=` plus `git diff --stat -- e2e/`, which must be empty. Also check that the task shrank the `pkg/arch` allowlist and didn't grow it. The mutation check becomes: break the service method the task extracted and watch an E2E or service test fail. Run `make test-ui` once per commit, next to `make check-q`. Not part of the budget: reading every test file. Read a test only when the mutation check fails to fail. -Run `make check-q` once before each commit; commit per task. +Run `make check-q` and `make lint` (CI lints; `check-q` does not) once before each commit; commit per task. For each bug a subagent reports: check `gh issue list --label bug` and the "Known bugs" list in `docs/plan/wb.md`, file an issue if it is new (security bugs go to the owner, not a public issue), and put diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 940777f2..876e385a 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -105,6 +105,56 @@ jobs: commit_message: "chore: apply go fix" file_pattern: "*.go" + browser: + name: Browser + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v7 + + - name: Set up Go + uses: actions/setup-go@v7 + with: + go-version-file: go.mod + cache: true + + - name: Set up Node.js + uses: actions/setup-node@v7 + with: + node-version: "24" + cache: "yarn" + cache-dependency-path: cmd/web/yarn.lock + + - name: Install system dependencies + run: sudo apt-get update && sudo apt-get install -y libvips-dev + + - name: Install frontend dependencies + run: yarn install --frozen-lockfile + working-directory: cmd/web + + - name: Cache Playwright driver and Chromium + uses: actions/cache@v6 + with: + path: | + ~/.cache/ms-playwright + ~/.cache/ms-playwright-go + key: playwright-${{ runner.os }}-${{ hashFiles('go.sum') }} + + - name: Install Chromium + run: make ui-deps UI_DEPS_FLAGS=--with-deps + + - name: Browser tests + run: make test-ui + env: + QRUN_MAX: "400" + + - name: Upload traces and screenshots + if: failure() + uses: actions/upload-artifact@v7 + with: + name: ui-artifacts + path: .ui-artifacts/ + if-no-files-found: ignore + frontend: name: Frontend runs-on: ubuntu-latest diff --git a/.gitignore b/.gitignore index a0bb7d28..742050eb 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ .cover/ coverage.out +.ui-artifacts/ diff --git a/.golangci.yml b/.golangci.yml index 9bc459cc..563b75af 100644 --- a/.golangci.yml +++ b/.golangci.yml @@ -1,6 +1,8 @@ version: "2" run: tests: true + build-tags: + - browser linters: enable: - misspell diff --git a/Makefile b/Makefile index 56d957a6..864c30b3 100644 --- a/Makefile +++ b/Makefile @@ -1,6 +1,8 @@ -.PHONY: shell tunnel lint test test-short cover build check fix check-q test-q vet-q cover-q model +.PHONY: shell tunnel lint test test-short cover build check fix check-q test-q vet-q cover-q model ui-deps test-ui ui-trace PKG ?= ./... +TAGS ?= +tags_flag = $(if $(TAGS),-tags $(TAGS)) shell: flyctl postgres connect -a pcomdb @@ -46,20 +48,42 @@ fix: # Quiet variants for agents: one line on success, a trimmed report on failure # (full output goes to a log file). They run the same steps as `make check`, # which stays the verbose CI form. -# Narrow with PKG, for example `make test-q PKG=./pkg/links/...`. +# Narrow with PKG, for example `make test-q PKG=./pkg/links/...`, and pass +# build tags with TAGS, for example `make vet-q PKG=./e2e/browser/... TAGS=browser`. check-q: @tools/qrun.sh build go build -o /dev/null ./... @tools/qrun.sh vet go vet ./... @tools/qrun.sh test go test ./... test-q: - @tools/qrun.sh test go test $(PKG) + @tools/qrun.sh test go test $(tags_flag) $(PKG) vet-q: - @tools/qrun.sh vet go vet $(PKG) + @tools/qrun.sh vet go vet $(tags_flag) $(PKG) cover-q: - @QRUN_SHOW_OK=1 tools/qrun.sh cover go test -cover $(PKG) + @QRUN_SHOW_OK=1 tools/qrun.sh cover go test $(tags_flag) -cover $(PKG) + +# Browser tests (e2e/browser, build tag `browser`). `make ui-deps` installs the +# Playwright driver and Chromium once (UI_DEPS_FLAGS=--with-deps also installs +# the system libraries, on Linux). `make test-ui` builds the frontend and runs +# the suite quietly: RUN= narrows it, COUNT= repeats it, and +# HEADED=1 SLOWMO=250 shows the browser (headless otherwise). A failed test logs the path of its +# trace; open it with `make ui-trace F=`. +PLAYWRIGHT = go run github.com/mxschmitt/playwright-go/cmd/playwright +RUN ?= +COUNT ?= 1 +UI_DEPS_FLAGS ?= + +ui-deps: + $(PLAYWRIGHT) install $(UI_DEPS_FLAGS) chromium + +test-ui: + @tools/qrun.sh ui-build yarn --cwd cmd/web build + @HEADED=$(HEADED) SLOWMO=$(SLOWMO) tools/qrun.sh test-ui go test -tags browser -count=$(COUNT) $(if $(RUN),-run '$(RUN)') ./e2e/browser/... + +ui-trace: + $(PLAYWRIGHT) show-trace $(F) # Shape of a generated model without reading pkg/model/core: # `make model` lists the models, `make model T=User` prints one. diff --git a/README.md b/README.md index 2a085742..cc642594 100644 --- a/README.md +++ b/README.md @@ -62,6 +62,28 @@ make lint # golangci-lint Docker must be running: tests that touch the database start a Postgres container via `pkg/testutil/testdb`. +### Browser tests + +User flows (forms, htmx swaps, Stimulus controllers) are tested in a real +Chromium driven by [playwright-go](https://github.com/mxschmitt/playwright-go). +They live in `e2e/browser` behind the build tag `browser`, so `make test` and +`make check` don't run them. + +``` +make ui-deps # once: installs the Playwright driver and Chromium +make test-ui # builds the frontend, then runs the suite +make test-ui RUN=TestSmoke COUNT=3 # narrow it, repeat it +make test-ui HEADED=1 SLOWMO=250 RUN=TestSmoke_LoginAndBoostedNavigation +make ui-trace F=.ui-artifacts/.trace.zip # inspect a failure +``` + +Chromium runs **headless** by default, so no window opens and a passing run +prints only `ok: test-ui`. Add `HEADED=1` to see the browser and `SLOWMO=` +to slow each step down. Narrow the run with `RUN` when watching, because the +tests run in parallel and each one opens its own window. A failed test saves a +screenshot and a trace under `.ui-artifacts/` and prints their paths. +`docs/testing.md` explains how the suite works and how to write a test. + ### psql access ``` diff --git a/docs/archive/history.md b/docs/archive/history.md index 6f959360..74131c0a 100644 --- a/docs/archive/history.md +++ b/docs/archive/history.md @@ -49,3 +49,56 @@ results, and the subagents at 43k–167k (the factories were the most expensive: each. model-shape was used through `make model` (5 calls). LSP was loaded once in total and test-failure never, because nothing failed. This is the first recorded wave, so there is no earlier cost line to compare with. + +## W6 — Browser tests (2026-09-26, branch `test/w6-browser`) + +**Built.** + +- B0 (coordinator): `e2e/browser` behind the `browser` build tag, playwright-go, + `e2e.WithRealAssets()`, logged-in pages from the session cookie, guards on page + errors, `console.error`, CSP violations and failed same-origin requests, a trace + and screenshot on failure, `make ui-deps`/`test-ui`/`ui-trace`, and a `Browser` + CI job. Smoke tests for login, boosted navigation, an action button and the + error toast. +- B1–B7: one file per area (navigation, writing, comments, actions, settings, + layout sweep, accounts). The suite runs in about 35 seconds, and passes + `-count=3`. +- Every Stimulus controller except `selfsubmit` (used by no template) and + `collapse` (only under the mobile-menu test skipped on #140) fails a test when + its `connect()` is emptied. Removing `json-enc`, `head-support`, the + `htmx:responseError` handler or the `htmx:sendError` handler each fails a test. +- Every mutating browser route is used successfully, except `signup` (skipped on + #139) and `signup_waiting_list`, which is switched off in code (Q6). The API + routes are W3's. +- Bugs filed: #139 (pages rendered from a bare map have no CSP nonce: `/signup`, + `/confirm_signup`, `/articles`, `/confirm_waiting_list`), #140 (opening the + mobile menu violates `style-src-attr`), #141 (a submit right after the post + form re-renders itself can go out natively and get a 403). + +**Wrong.** + +- The plan said comments update "without a full reload". The comment form + reloads the page on purpose (it keeps the scroll position), so a subagent + pinned the intended behavior as a bug. The test now asserts the reload + behavior, and no issue was filed. +- The share link has no copy button, so the planned "copy the share link" + step had nothing to click. The clipboard controller is tested through the + API key instead. +- The haiku layout sweep was vacuous (`window.scrollWidth` is undefined, so + the overflow check could never fail) and allowed CSP violations. It was + redone at sonnet. + +**Left out.** `/confirm_signup` in the layout sweep (it needs a user with a +known confirmation seed, and #139 blocks the page anyway). The `Browser` +check becomes required on `master` right after this PR merges (not before, or +open PRs without the job would wait forever); that is the owner's step. + +**Cost.** 2 coordinator sessions (B0, then B1–B7) and 8 subagents (7 sonnet, +1 haiku), 945 turns in total. The coordinators peaked at 133k and 140k +context with 92k and 108k of tool results; subagents at 79k–262k, with the +settings and writing tasks the most expensive (150–167 turns, 234k–271k of +results). 91 wasteful calls, almost all `cat` of whole files. Skills: +frontend-htmx by 4 agents, wave-run twice, wave-close once; model-shape and +test-failure never, and LSP barely (2k of results). Compared with W0, the +wave cost about three times the turns for twice the subagents: browser +tasks iterate far more than unit tests do. diff --git a/docs/implementation-plan.md b/docs/implementation-plan.md index 5385a991..c89216f3 100644 --- a/docs/implementation-plan.md +++ b/docs/implementation-plan.md @@ -51,7 +51,7 @@ Related documents: | W3 | End-to-end HTTP tests: server rules | W0 | not started | `test/w3-e2e` | | W4 | Local stack (Postgres, tommy for mail and S3), dev tooling container, app in compose, seed | W0 | not started | `test/w4-local-stack` | | W5 | Coverage ratchet | W1–W4 | not started | `test/w5-ratchet` | -| W6 | Browser tests (playwright-go): user flows | W0 | not started | `test/w6-browser` | +| W6 | Browser tests (playwright-go): user flows | W0 | done | `test/w6-browser` | | WB | Bug-fix wave (#108–#117, #119–#122) | W1–W3 | not started | `fix/wb-survey-bugs` | | R1 | Router decomposition (move handlers) | W3, W6, WB | planned | `refactor/r1-router` | | RS | Repositories and services, thin handlers | R1 | planned | `refactor/rs-layers` | diff --git a/docs/lessons.md b/docs/lessons.md index a193beb5..6815e7c1 100644 --- a/docs/lessons.md +++ b/docs/lessons.md @@ -15,3 +15,20 @@ Generalizable lessons from running the waves. Wave-specific notes go in - **Subagents cat files.** Even with AGENTS.md's reading rules, W0's subagents printed whole files 33 times and loaded LSP once. The task preamble now says it explicitly. +- **Parallel subagents in one Go package.** One half-written file breaks the + compile for every agent in the package. Give each agent its own build tag + while it iterates (`//go:build browser && b3`, run with `-tags browser,b3`) + and have it switch to the shared tag before reporting. Shared build steps + (`yarn build`) run once in the coordinator, not in every agent, or they race + on the output directory. +- **A cheap tier needs a self-checking task.** A layout sweep asserts things + that pass whether or not the check works (`undefined > n` is false). Such a + task can't catch its own mistakes, so it is not cheap. Ask for a proof that + each check can fail. +- **Verify bug reports against intent.** A subagent told the wrong expected + behavior reports the real one as a bug. Before filing, check the code and + its history (`git log -S`) for a deliberate choice. +- **Mutation-sweep the controllers.** Emptying each Stimulus controller's + `connect()` in turn and running the suite found the one controller whose + tests only ever took the happy path (confirm: every test accepted the + dialog). The agents' own "covers" lists overstated coverage. diff --git a/docs/plan/r2.md b/docs/plan/r2.md index 573a57a8..df9e2349 100644 --- a/docs/plan/r2.md +++ b/docs/plan/r2.md @@ -49,8 +49,16 @@ Replace the switch with explicit settings, such as `--secure-cookies`, - The E2E harness starts a tommy test container (the same one that serves S3, below) and adds `app.Mails(t, filter)`, which polls `GET /api/v1/events?plugin=mail` until - the expected mail arrives. Tests that asserted on the `outgoing_emails` - queue gain an assertion on delivered mail. + the expected mail arrives. **E2E and browser tests switch from the + `outgoing_emails` queue to delivered mail**: they assert on what tommy + received (recipient, subject, body), and flows that follow a link from an + email (signup confirmation in `e2e/browser/accounts_test.go`) take it from + the delivered message. The queue is an implementation detail that R4 may + replace with a job queue, so after R2 no test under `e2e/` reads it + (`git grep ListOutgoingEmails e2e/` is empty). Today that is + `e2e/browser/accounts_test.go`, `comments_test.go`, `settings_test.go`, + plus any W3 test added by then. `factory.ListOutgoingEmails` stays for + package tests of `dbsender` itself. - Unit and package tests keep using `fakesender`. **Object storage everywhere; local file storage is dropped.** diff --git a/docs/plan/w6.md b/docs/plan/w6.md deleted file mode 100644 index a304a9f6..00000000 --- a/docs/plan/w6.md +++ /dev/null @@ -1,148 +0,0 @@ -## W6 — Browser tests - -**Goal:** a browser suite that drives the real app with real assets and is -**the specification of everything a user does in a page**. Every flow that -needs the page's JavaScript is tested here, not over plain HTTP: forms, -action buttons, htmx swaps and redirects, Stimulus controllers, -confirmations, toasts, dark mode, and no console or CSP errors. Later frontend -work (templates, controllers, SCSS, and upgrades of htmx, Stimulus or any -other frontend dependency) is checked with one command, and an upgrade that -breaks a page fails a test. - -W3 pins the server rules that don't depend on the frontend (visibility, -guards, API, RSS). The successful use of every mutating route is W6's; the -coordinators check that W3 and W6 together cover every route. - -W6 depends only on W0 (the E2E harness and the factories). It runs in parallel -with W1–W4 and is the main safety net for R1 and RS; run B0 first after W0. It is a test wave: -the W0–W6 ground rules in `docs/testing.md` apply. **Templates, JS and SCSS -don't change in W6.** Locators use roles, labels, text and the existing CSS -classes. Adding `data-testid` attributes is a change for later frontend work, -not this wave. - -### Tool: playwright-go - -[playwright-go](https://github.com/playwright-community/playwright-go) rather -than the Node runner, because the tests have to reuse what W0 built: the -harness starts the real binary against a fresh test database, and fixtures -come from `pkg/testutil/factory`, the same way every other test describes the -world. A Node runner would need a second fixture mechanism. playwright-go -bundles the Playwright driver, so traces, `codegen` and `show-trace` remain -available for iterating -(`go run github.com/playwright-community/playwright-go/cmd/playwright …`). - -Visual snapshot (pixel) tests are left out. Font rendering differs between -macOS and Linux, so baselines made on one fail on the other. Revisit once -browsers run inside the tools container, where baselines would be produced -in one place. - -### B0 Harness, CI and a smoke test (strong, coordinator) - -This task defines the contract for B1–B6. - -- The package is `e2e/browser`, behind the build tag `browser`, so - `make test` and `make check` need no browser. `go.mod` gains - `playwright-go`. That is the only `go.mod` change W6 may make, and only in - B0. -- **Assets:** `e2e.Start(t, e2e.WithRealAssets())` serves the real - `cmd/web/dist` instead of the stub manifest. `make test-ui` runs `yarn build` - first, so the suite always tests the current frontend source. -- **Browser:** one Chromium per package (`TestMain`); a fresh browser context - per test, so tests are isolated and may run `t.Parallel()`. - `browser.Page(t, app, browser.As(user))` returns a page that is already - logged in. It reuses the session cookie from `App.Client(t).LoginAs`, - rather than filling in the login form in every test. -- **Guards on every page:** a test fails on an uncaught page error, a - `console.error`, a CSP violation (`securitypolicyviolation`), or a failed - request to the app's own origin. These catch most frontend regressions - without a specific assertion. -- **Assertions** use Playwright's auto-waiting locator assertions - (`playwright.NewPlaywrightAssertions()`), never sleeps. Database state is - checked through factory readers, as in W3. -- **On failure:** the test saves a trace and a full-page screenshot and prints - both paths in one line. Open the trace with `make ui-trace F=`. CI - uploads the directory as an artifact. -- **Make targets:** `make ui-deps` installs Chromium, once. `make test-ui` runs - the suite headless (`RUN=` narrows it, `COUNT=` repeats it). `HEADED=1 SLOWMO=250` watches - it run locally. The quiet targets gain a `TAGS=` pass-through, so that - `make vet-q PKG=./e2e/browser/... TAGS=browser` compiles the suite. -- **CI:** a `browser` job that runs `yarn install --frozen-lockfile`, - `make ui-deps` (with a cached browser directory) and `make test-ui`. It is - required for merging from W6 on. -- **Smoke test:** log in through the real form, land on `/feed`, and - navigate to `/controls` through a boosted link. Assert that the title - changed and the page didn't fully reload: a marker set on `window` before - the click must survive the navigation. -- Add a "Browser tests" section to `docs/testing.md` with one worked example. - Update the `frontend-htmx` and `test-failure` skills to point at it. - -### Reliability rules (part of B0's contract) - -A browser suite is only useful if a red run means a real regression. - -- **Assert outcomes, not mechanisms.** Tests assert what the user sees - (text, roles, visibility, URL, title) and the resulting database state. - They don't wait for htmx events, read `hx-*` attributes, inspect request - headers or call `window.htmx`. That keeps them valid across htmx versions - and across replacing htmx altogether. -- **Wait by assertion only.** Playwright's auto-waiting locator assertions, - never sleeps and never `WaitForTimeout`. A "nothing happened" check asserts - on a state that the action would have changed (for example, the row count - after the swap), not on elapsed time. -- **Isolation.** A fresh database (`e2e.Start`) and browser context per test, - no shared fixtures, so tests may use `t.Parallel()` and run in any order. -- **No retries.** CI doesn't re-run failed browser tests. A flaky test is a - bug: it is fixed or skipped with an issue number, never retried into green. - B0's smoke test must pass `-count=10` in a row, and every scenario task's - tests `-count=3`, before the task is accepted. -- **Library mutation checks.** Besides a broken controller, B0 proves the - suite catches frontend-library breakage. With each of these changes to - `cmd/web/client/js/index.js` the suite must fail, and then it is reverted: - the `json-enc` extension removed (action buttons stop working), the - `head-support` import removed (the title no longer changes on boosted - navigation), and the `htmx:responseError` handler removed (no error - toast). A test that stays green through these doesn't cover htmx. -- **Dependency updates run the suite.** The `browser` CI job runs on every - PR, including Dependabot's npm updates, and is required for merging. - -### htmx and frontend inventory - -The coordinator greps the frontend and checks that each item below is -exercised by at least one test whose assertion is user-visible, the same way -it checks the Stimulus controllers: - -- every `hx-*` attribute in use (`hx-boost`, `hx-post`, `hx-swap` in each - mode used, `hx-target`, `hx-trigger`, `hx-disabled-elt`, `hx-encoding`, - `hx-headers`) and every extension (`head-support`, `json-enc`); -- the `htmx.config` choices in `index.js` (script tags not executed, CSP - nonces for inline scripts and styles), checked through the CSP guard; -- the error handlers (`htmx:responseError`, `htmx:sendError`): a server error - and a dropped connection (`page.Route` aborting the request) each show the - error toast; -- every `HX-*` response header the server sends (`HX-Redirect`, - `HX-Refresh`, `HX-Retarget`, `HX-Trigger` and so on), through its effect: - the new URL, the reloaded page, the error rendered in place, the toast. - -### Scenarios (parallel after B0) - -Each task owns one file, `e2e/browser/_test.go`. Tests are written -against the seed-like worlds the factories build, not against `cmd/seed` -itself. - -| Task | Covers | Tier | -|---|---|---| -| B1 | Navigation and chrome: boosted navigation between every top-level page (title and head merge, back and forward), flashes and toasts appear and auto-dismiss, the collapse, toggle and spoiler controllers, dark mode under `prefers-color-scheme: dark` emulation (the `data-bs-theme` and a computed background from `_dark-mode.scss`), the mobile menu at a 390px viewport, and the error toast on a server error and on a dropped connection (`page.Route`) | mid | -| B2 | Writing: `/write` (also with `?prompt=`) and the markdown editor controller (type, toolbar buttons, preview if present), autosave of a draft, publish, make a published post a draft again, delete through the editor, the rendered post (headings, code highlighting, gallery, lite-youtube embed element, lazy images), edit an existing post, delete a draft via the confirm dialog (`page.OnDialog`) | mid | -| B3 | Comments: comment on a post, reply inline via the comment-form controller, the thread updates without a full reload, nesting and collapse, the author and participant notifications queued (factory readers), and a user who may not comment doesn't get the form | mid | -| B4 | Actions via the generic action controller, clicked through the `/controls` buttons, including `skipReload` swaps and `hx-swap="delete"`: the three-user connection story (A and B connected, B and C connected, A requests C, B signs, C accepts, and A then sees C's direct-only post), and each other connection action once (whitelist and remove from whitelist, create and drop a connection, revoke and dismiss a mediation request, reject a connection); create a share and copy its link (grant clipboard permission and read it back), open it anonymously, delete it and see it gone; ask for a post (prompt) and dismiss a prompt | mid | -| B5 | Settings and forms: validation errors rendered in place (htmx retarget), save general settings, change password and log in again, user styles applied on the profile page, generate an API key, send an invite (email queued), add a feed (pointing at an `httptest.Server` the test owns), see its items, dismiss one and unsubscribe, export settings and import them back, upload an image through the file input and see it render (`naturalWidth > 0`, served from `/user-media`) | mid | -| B6 | Layout sweep: every page the W4.S5 crawl visits, logged in and anonymous, at 1280px and 390px: no horizontal overflow (`scrollWidth <= innerWidth`), no guard violations, and every image loaded | cheap | -| B7 | Accounts: log in through the form (bad credentials show the error in place; a signed `return_url` lands on that page), log out, sign up while registration is open, accept an invitation (the new account is logged in and connected to the inviter), and follow a confirmation link from the queued email | mid | - -**Done when:** the suite passes locally and in CI in under five minutes, and -passes `-count=3` in CI without a failure; each Stimulus controller under -`cmd/web/client/js/controllers/` and each item of the htmx inventory is -exercised by at least one test (the coordinator checks both against a grep); -every mutating route is used successfully by a test (with W3, every route is -covered); and one deliberately broken controller plus the three library -mutation checks each fail a test. diff --git a/docs/testing.md b/docs/testing.md index eda5919c..535c123e 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -111,13 +111,97 @@ Every package that uses it needs `func TestMain(m *testing.M) { e2e.Main(m) }`. `PostForm`, `PostJSON`, `LoginAs(email, password)` and `Do(req)` for anything else; each returns a `*Response` with `RequireStatus(code)`, `Doc()` (goquery) and `Location()`; `Header` and `Body` are plain fields. A feed a test creates must point at an `httptest.Server` the test owns, never a real remote URL. Mail is asserted through the outgoing queue for now: -`factory.ListOutgoingEmails(ctx, app.DB, core.OutgoingEmailWhere.EmailType.EQ(...))`. +`factory.ListOutgoingEmails(ctx, app.DB, core.OutgoingEmailWhere.EmailType.EQ(...))`. This is temporary: +the app can't deliver to tommy until R2 makes the Mailjet base URL configurable, and R2 then moves every +E2E and browser mail assertion to delivered mail in tommy (`app.Mails`). + +## Browser tests: e2e/browser + +Every user flow that needs the page's JavaScript is tested here, in headless Chromium through +[playwright-go](https://github.com/mxschmitt/playwright-go): forms, action buttons, htmx swaps and +redirects, Stimulus controllers, confirmations, toasts, dark mode. The package is behind the build tag +`browser`, so `make test` and `make check` need no browser. Run it with `make test-ui`, which builds the +frontend first (`RUN=` narrows it, `COUNT=` repeats it, `HEADED=1 SLOWMO=250` shows the browser); +install Chromium once with `make ui-deps`. Compile it with `make vet-q PKG=./e2e/browser/... TAGS=browser`. + +What a run does: + +1. `make test-ui` runs `yarn build`, so the suite tests the current frontend source, then + `go test -tags browser ./e2e/browser/...` through the quiet runner. A passing run prints one line. +2. `TestMain` starts the Playwright driver bundled with playwright-go and launches **one Chromium for + the package**, headless unless `HEADED` is set (`SLOWMO=` delays every action). It uses the + browsers `make ui-deps` installed in Playwright's cache (`~/Library/Caches/ms-playwright` on macOS). +3. Each test starts its own app (the real web binary against a fresh test database) and gets a fresh + browser context in that Chromium: separate cookies and storage, so tests run in parallel. +4. On failure the screenshot and trace paths are printed. `make ui-trace F=` opens Playwright's trace + viewer, a timeline of every action with a DOM snapshot, console and network log at each step. + +To watch a test, run one at a time: `make test-ui HEADED=1 SLOWMO=250 RUN=`. Without `RUN`, +every parallel test opens its own window. + +The harness API: + +- `e2e.Start(t, e2e.WithRealAssets())` serves the real `cmd/web/dist`. Every test starts its own app. +- `browser.NewUser(t, app, opts...)` is `factory.User` with the password `browser.Password`, and + `browser.Page(t, app, browser.As(user))` returns a page in a fresh browser context, already logged in + (it reuses an HTTP login's session cookie). The base URL is the app's, so `page.Goto("/feed")`. +- **Guards:** the page fails the test on an uncaught error, a `console.error`, a CSP violation, a failed + request to the app or an app response of 404 or 5xx. A test that causes an error on purpose exempts it + with `browser.Allow(regexp)`, matched against the guard message. `browser.Configure(fn)` changes the + context options (viewport, `ColorScheme`). +- **On failure** a full-page screenshot and a trace are saved to `.ui-artifacts/` (or `$UI_ARTIFACTS`) and + both paths are logged in one line; open the trace with `make ui-trace F=`. CI uploads the directory. + +Reliability rules, because a red run must mean a real regression: + +1. **Assert outcomes, not mechanisms:** what the user sees (text, roles, visibility, URL, title) and the + database state through the factory readers. Never wait for htmx events, read `hx-*` attributes, inspect + request headers or call `window.htmx`, so the tests survive an htmx upgrade and catch one that breaks a page. +2. **Wait by assertion only:** `browser.Expect` (Playwright's auto-waiting assertions), never a sleep or + `WaitForTimeout`. A "nothing happened" check asserts on a state the action would have changed. +3. **Locate by role, label and text**, then by existing CSS classes. Test waves don't add `data-testid`. +4. **Isolation:** no shared fixtures; every test may call `t.Parallel()`. +5. **No retries:** a flaky test is fixed, or skipped with an issue number. A new test must pass + `make test-ui RUN= COUNT=3` before it is accepted. + +A browser test: + +```go +//go:build browser + +package browser_test + +func TestSmoke_ActionButton(t *testing.T) { + t.Parallel() + + app := e2e.Start(t, e2e.WithRealAssets()) + user := browser.NewUser(t, app) + draft, err := factory.Post(context.Background(), app.DB, user.ID) + require.NoError(t, err) + + page := browser.Page(t, app, browser.As(user)) + page.OnDialog(func(d playwright.Dialog) { _ = d.Accept() }) + + _, err = page.Goto("/controls") + require.NoError(t, err) + + row := page.GetByRole("row").Filter(playwright.LocatorFilterOptions{HasText: draft.Subject.String}) + require.NoError(t, row.GetByRole("button").Click()) + + require.NoError(t, browser.Expect.Locator(row).ToHaveCount(0)) + + posts, err := factory.ListPosts(context.Background(), app.DB, user.ID) + require.NoError(t, err) + require.Empty(t, posts) +} +``` ## Make targets `make test-short` runs everything except E2E. `make cover` runs unit, package and E2E tests together under one `GOCOVERDIR` and prints a merged per-package coverage table. The quiet targets `check-q`, `test-q`, `vet-q` and -`cover-q` (see `AGENTS.md`) are for agents and narrow with `PKG=./pkg/links/...`. +`cover-q` (see `AGENTS.md`) are for agents, narrow with `PKG=./pkg/links/...` and take build tags with +`TAGS=browser`. The browser targets are `ui-deps`, `test-ui` and `ui-trace` (above). ## Worked examples diff --git a/e2e/browser/accounts_test.go b/e2e/browser/accounts_test.go new file mode 100644 index 00000000..22562798 --- /dev/null +++ b/e2e/browser/accounts_test.go @@ -0,0 +1,179 @@ +//go:build browser + +package browser_test + +import ( + "context" + "regexp" + "testing" + + "github.com/can3p/gogo/sender" + "github.com/can3p/pcom/e2e" + "github.com/can3p/pcom/e2e/browser" + "github.com/can3p/pcom/pkg/model/core" + "github.com/can3p/pcom/pkg/testutil/factory" + "github.com/mxschmitt/playwright-go" + "github.com/stretchr/testify/require" +) + +// b7ConfirmLinkRE matches the absolute confirmation link the way it is sent +// in the plain-text body of the confirm_signup mail. +var b7ConfirmLinkRE = regexp.MustCompile(`https?://\S+/confirm_signup/\S+`) + +// b7ConfirmLink reads the queued confirm_signup email back from the +// outgoing mail queue and pulls the confirmation link out of its body. +func b7ConfirmLink(t testing.TB, app *e2e.App) string { + t.Helper() + + emails, err := factory.ListOutgoingEmails(context.Background(), app.DB, core.OutgoingEmailWhere.EmailType.EQ("confirm_signup")) + require.NoError(t, err) + require.Len(t, emails, 1) + + var payload sender.Mail + require.NoError(t, emails[0].Payload.Unmarshal(&payload)) + + link := b7ConfirmLinkRE.FindString(payload.Text) + require.NotEmpty(t, link, "confirm_signup mail body: %s", payload.Text) + + return link +} + +// Bad credentials on the login form come back as an in-place error: the form +// is swapped for itself with the error shown, the page never navigates away. +func TestAccounts_LoginBadCredentialsShowErrorInPlace(t *testing.T) { + t.Parallel() + + app := e2e.Start(t, e2e.WithRealAssets()) + user := browser.NewUser(t, app) + + page := browser.Page(t, app) + + _, err := page.Goto("/login") + require.NoError(t, err) + + require.NoError(t, page.GetByLabel("Email address").Fill(user.Email)) + require.NoError(t, page.GetByLabel("Password").Fill("not-the-password")) + require.NoError(t, page.GetByRole("button", playwright.PageGetByRoleOptions{Name: "Log in"}).Click()) + + require.NoError(t, browser.Expect.Locator(page.Locator(".alert-danger")).ToContainText("Bad credentials")) + require.NoError(t, browser.Expect.Page(page).ToHaveURL(regexp.MustCompile(`/login`))) +} + +// A protected page redirects an anonymous visitor to /login with a signed +// return_url; logging in from there lands back on the originally requested +// page instead of the default feed. +func TestAccounts_LoginWithSignedReturnURLLandsOnRequestedPage(t *testing.T) { + t.Parallel() + + app := e2e.Start(t, e2e.WithRealAssets()) + user := browser.NewUser(t, app) + + page := browser.Page(t, app) + + _, err := page.Goto("/controls/settings") + require.NoError(t, err) + + require.NoError(t, browser.Expect.Page(page).ToHaveURL(regexp.MustCompile(`/login\?.*return_url=`))) + + require.NoError(t, page.GetByLabel("Email address").Fill(user.Email)) + require.NoError(t, page.GetByLabel("Password").Fill(browser.Password)) + require.NoError(t, page.GetByRole("button", playwright.PageGetByRoleOptions{Name: "Log in"}).Click()) + + require.NoError(t, browser.Expect.Page(page).ToHaveURL(regexp.MustCompile(`/controls/settings$`))) + require.NoError(t, browser.Expect.Locator(page.GetByRole("button", playwright.PageGetByRoleOptions{Name: "Log out"})).ToBeVisible()) +} + +// Logging out from settings redirects to the home page and the nav goes +// back to showing an anonymous Login link. +func TestAccounts_Logout(t *testing.T) { + t.Parallel() + + app := e2e.Start(t, e2e.WithRealAssets()) + user := browser.NewUser(t, app) + + page := browser.Page(t, app, browser.As(user)) + + _, err := page.Goto("/controls/settings") + require.NoError(t, err) + + require.NoError(t, page.GetByRole("button", playwright.PageGetByRoleOptions{Name: "Log out"}).Click()) + + require.NoError(t, browser.Expect.Page(page).ToHaveURL(regexp.MustCompile(`^`+regexp.QuoteMeta(app.URL)+`/$`))) + require.NoError(t, browser.Expect.Locator(page.GetByRole("navigation").GetByRole("link", playwright.LocatorGetByRoleOptions{Name: "Login", Exact: playwright.Bool(true)})).ToBeVisible()) +} + +// Signing up while registration is open queues a confirmation email; +// following the link in it confirms the address and the account can then +// log in, which only works once the email is confirmed. +func TestAccounts_SignupWhileOpenAndConfirmEmail(t *testing.T) { + t.Parallel() + t.Skip("known bug #139: GET /signup and GET /confirm_signup/:id render header.html from a bare map instead of a page struct built through getBasePage, so ScriptNonce/StyleNonce are empty; the inline bootstrap