diff --git a/.vscode/extensions.json b/.vscode/extensions.json index 8f86e1fd..1148e2c1 100644 --- a/.vscode/extensions.json +++ b/.vscode/extensions.json @@ -7,7 +7,8 @@ "bradlc.vscode-tailwindcss", "lokalise.i18n-ally", "antfu.iconify", - "vitest.explorer" + "vitest.explorer", + "ms-playwright.playwright" ], "unwantedRecommendations": [ "octref.vetur" diff --git a/.vscode/settings.json b/.vscode/settings.json index 02f70bf4..48dc1dcc 100644 --- a/.vscode/settings.json +++ b/.vscode/settings.json @@ -11,6 +11,5 @@ "eslint.workingDirectories": [ "web", "server/auth-realtime" - ], - "vitest.rootConfig": "web/vitest.config.ts" + ] } diff --git a/Makefile b/Makefile index 025a39f3..4ba77344 100644 --- a/Makefile +++ b/Makefile @@ -30,17 +30,20 @@ stop: # the e2e stack runs on its own ports with throwaway state, so it can be # up at the same time as the dev stack. # -# make only ever builds; starting the stack belongs to the suite's own -# setup hook, so there is exactly one thing that runs `compose up`. -# Building has to live here because the hook cannot do it: goreleaser -# produces core's image, which compose cannot build at all, and the two -# images compose does own are only refreshed by an explicit build. -# Iterate with `make e2e-stack-build` once, then `cd e2e && pnpm test`. +# the playwright config owns the whole cycle: globalSetup builds the stack +# and brings it up, globalTeardown stops it and drops its volumes. That is +# what makes the play button in the Playwright VS Code extension do exactly +# what this target does, so this target only starts the run. The build +# stays a make target because the hook cannot express it — goreleaser +# produces core's image, which compose cannot build at all — and the hook +# shells back out to e2e-stack-build, which is also what keeps +# E2E_COMPOSE_EXTRA working from CI. +# +# Every run is therefore a full build and teardown. To iterate without +# paying that, run e2e-stack-build once and drive the tests directly. .PHONY: e2e -e2e: e2e-stack-build - @( cd e2e && pnpm run test ); status=$$?; \ - $(MAKE) --no-print-directory e2e-stack-stop; \ - exit $$status +e2e: + @( cd e2e && pnpm run test ) .PHONY: e2e-stack-build e2e-stack-build: diff --git a/e2e/CLAUDE.md b/e2e/CLAUDE.md index ebbb70f2..27ce815e 100644 --- a/e2e/CLAUDE.md +++ b/e2e/CLAUDE.md @@ -27,7 +27,7 @@ instead. Package manager is **pnpm**. From the repository root: ```sh -make e2e # one-shot: build images, start the stack, run the tests, tear down +make e2e # run the suite; the config builds, starts and tears down make e2e-stack-build # build the stack's images, for iterating make e2e-stack-stop # stop the stack and drop its data ``` @@ -36,7 +36,7 @@ From `e2e/`: ```sh pnpm setup # deps + the playwright chromium browser -pnpm test # playwright test (brings the stack up first, see below) +pnpm test # playwright test (full cycle, see below) pnpm check-lint # check-types + check-eslint + check-fmt + check-knip pnpm lint # the fixing counterpart of all four @@ -48,34 +48,46 @@ pnpm check-fmt # fixing counterparts) pnpm check-knip # dead exports, files and dependencies ``` -`pnpm test` runs [global-setup.ts](global-setup.ts), which brings the -stack up itself, so it works whether or not the stack is already running. -**What it never does is rebuild.** Compose builds `web` and -`auth-realtime` when their images are missing outright, but a source -change after that is invisible to it — refreshing them takes an -explicit build. -Core it cannot build at all: the service has no build section, so -`make build-go` is the only thing that produces that image. Teaching the -hook to build would drop goreleaser into the inner loop of every run. +**The playwright config owns the whole cycle.** +[global-setup.ts](global-setup.ts) builds the stack and brings it up; +[global-teardown.ts](global-teardown.ts) stops it and drops its volumes. +Nothing has to be running first, and nothing is left behind. -That is the division of labour, and each verb has exactly one owner. -**Make only builds** — goreleaser for core, `compose build` for the two -images compose owns — and **the setup hook is the only thing that runs -`compose up`.** So: after changing backend or frontend source, -`make e2e-stack-build`; after changing only a test, `pnpm test`. Running -`pnpm test` against source you just edited silently tests the previous -build. +That is deliberate, and the reason is that a test suite has several front +doors — `make e2e`, `pnpm test`, and the play button in the Playwright VS +Code extension — and only the config is common to all of them. Putting +the build in the caller meant the extension tested whatever image was +last left behind, which passes and lies. Now every entry point runs the +same cycle, so `make e2e` is a one-line target that only starts the run. + +The build still lives in the Makefile, and the hook shells back out to +`make e2e-stack-build` rather than reimplementing it. Two reasons: the +hook cannot express it — goreleaser produces core's image, which compose +cannot build at all, since the service has no build section — and going +through make is what keeps `E2E_COMPOSE_EXTRA` working, which is how CI +layers the buildx cache config in +[docker-compose.ci.yaml](docker-compose.ci.yaml) over the base file. + +**The cost is that every run is a full build and teardown.** With warm +caches and no source change that is tens of seconds, not minutes, and a +dropped volume per run means each run starts from an empty database. To +iterate without paying it, build once with `make e2e-stack-build` and +drive playwright directly with `pnpm exec playwright test`, which is the +same binary without the make wrapper — but understand that you are then +responsible for rebuilding after a source change, and that a stale image +fails silently. Both build steps run quietly, under a ticking line, and replay their whole log if they fail. Docker and goreleaser say a great deal that matters only when something breaks. **The stack has no pnpm scripts of its own**, and should not grow any. -Starting it belongs to `global-setup.ts` and building to make; anything -else — tearing it down by hand, reading logs — is plain `docker compose` -run from this directory, which discovers `docker-compose.yaml` without -being told where it is. A `pnpm stack:*` wrapper would only be a second -name for a command that is already shorter than its alias. +Its lifecycle belongs to the setup and teardown hooks, which reach the +Makefile through [helpers/stack.ts](helpers/stack.ts); anything else — +reading logs, poking at a container — is plain `docker compose` run from +this directory, which discovers `docker-compose.yaml` without being told +where it is. A `pnpm stack:*` wrapper would only be a second name for a +command that is already shorter than its alias. ## The stack @@ -407,8 +419,8 @@ unawaited assertion described above. It is scoped to **knip** guards dead exports, files and dependencies, and needs no config file: its Playwright plugin reads [playwright.config.ts](playwright.config.ts), -resolves `globalSetup` from it, and treats the test files as entry -points. Nothing here has `web/`'s auto-import or vendored-directory +resolves `globalSetup` and `globalTeardown` from it, and treats the test +files as entry points. Nothing here has `web/`'s auto-import or vendored-directory blind spots, so a `knip.ts` would only be a file to keep in sync. TypeScript is strict, with `noUnusedLocals`, `noUnusedParameters`, diff --git a/e2e/global-setup.ts b/e2e/global-setup.ts index 8ee4e80e..dfd16c13 100644 --- a/e2e/global-setup.ts +++ b/e2e/global-setup.ts @@ -1,19 +1,26 @@ -import { spawn } from "node:child_process" -import { fileURLToPath } from "node:url" import { BASE_URL, MAILPIT_URL } from "./helpers/config" +import { buildStack, startStack } from "./helpers/stack" const READY_TIMEOUT = 180_000 const POLL_INTERVAL = 2_000 const TICK_INTERVAL = 1_000 -const E2E_DIR = fileURLToPath(new URL(".", import.meta.url)) - -// bring the stack up and wait until every service the tests touch answers -// through the front door. `up --wait` only covers the containers that -// declare a healthcheck, and core cannot declare one — its image is -// distroless, with no shell or http client to probe itself with. +// build the stack from this checkout, bring it up, and wait until every +// service the tests touch answers through the front door. `up --wait` only +// covers the containers that declare a healthcheck, and core cannot +// declare one — its image is distroless, with no shell or http client to +// probe itself with. +// +// The build runs here rather than in the caller so that every way of +// starting the suite behaves the same: `make e2e`, `pnpm test`, and the +// play button in the Playwright VS Code extension all test the code that +// is in the tree right now, not whatever image was last left behind. export default async function globalSetup(): Promise { - await progress("starting the stack", () => compose(["up", "-d", "--wait"])) + // make prints its own labelled step per image, so it is not wrapped + // in a progress line of this file's own. + await buildStack() + + await progress("starting the stack", startStack) await progress("waiting for services", () => Promise.all([ @@ -50,43 +57,6 @@ async function progress( process.stdout.write(" ok\n") } -// compose swallows docker's output and replays it only on failure: the -// container-by-container churn says nothing while it is working, and -// everything once it is not. -function compose(args: string[]): Promise { - const child = spawn("docker", ["compose", ...args], { - cwd: E2E_DIR, - stdio: ["ignore", "pipe", "pipe"], - }) - - // decoded as text rather than concatenated as buffers, so a multi-byte - // character split across two chunks survives the join. - child.stdout.setEncoding("utf8") - child.stderr.setEncoding("utf8") - - let output = "" - - const collect = (chunk: string) => { - output += chunk - } - - child.stdout.on("data", collect) - child.stderr.on("data", collect) - - return new Promise((resolve, reject) => { - child.on("error", reject) - child.on("close", (code) => { - if (code === 0) { - resolve() - - return - } - - reject(new Error(`docker compose ${args.join(" ")} failed\n\n${output}`)) - }) - }) -} - async function waitForService(name: string, url: string): Promise { const deadline = Date.now() + READY_TIMEOUT diff --git a/e2e/global-teardown.ts b/e2e/global-teardown.ts new file mode 100644 index 00000000..55051443 --- /dev/null +++ b/e2e/global-teardown.ts @@ -0,0 +1,8 @@ +import { stopStack } from "./helpers/stack" + +// tears the stack down and drops its volumes, so the next run starts from +// an empty database rather than inheriting the accounts and workspaces +// this one created. make prints its own progress line for the step. +export default async function globalTeardown(): Promise { + await stopStack() +} diff --git a/e2e/helpers/stack.ts b/e2e/helpers/stack.ts new file mode 100644 index 00000000..e0328e1a --- /dev/null +++ b/e2e/helpers/stack.ts @@ -0,0 +1,112 @@ +import { spawn } from "node:child_process" +import { fileURLToPath } from "node:url" + +const E2E_DIR = fileURLToPath(new URL("..", import.meta.url)) +const ROOT_DIR = fileURLToPath(new URL("../..", import.meta.url)) + +// the stack is driven through the root Makefile rather than through +// docker compose directly, so the build stays defined in one place: it is +// two steps, and only one of them is compose's. Core's image is produced +// by goreleaser, which compose cannot do. Running make also keeps +// E2E_COMPOSE_EXTRA working — CI sets it to layer the buildx cache config +// in docker-compose.ci.yaml over the base file. +// +// Every make target here already runs its steps through +// scripts/run-quietly.sh, which ticks a line per step and replays the log +// only on failure. So they inherit this process's streams: capturing them +// would hide the per-step labels and replace two of them ("building go +// images", "building stack images") with one anonymous row of dots. +export function buildStack(): Promise { + return run("make", ["e2e-stack-build"], ROOT_DIR, "inherit") +} + +export function stopStack(): Promise { + return run("make", ["e2e-stack-stop"], ROOT_DIR, "inherit") +} + +// compose is the exception: it is called directly, with nothing in front +// of it to quieten the container-by-container churn, so its output is +// held back and replayed only if the step fails. +export function startStack(): Promise { + return run("docker", ["compose", "up", "-d", "--wait"], E2E_DIR, "capture") +} + +// a step that did not finish. Playwright prints an error's stack, and a +// JS trace through this file says nothing about a docker build that broke +// or a run someone stopped on purpose — the message is the whole story, +// so it stands in for the trace. +class StepError extends Error { + constructor(message: string) { + super(message) + this.name = "StepError" + this.stack = message + } +} + +function run( + command: string, + args: string[], + cwd: string, + output: "inherit" | "capture", +): Promise { + const child = spawn(command, args, { + cwd, + stdio: output === "inherit" ? "inherit" : ["ignore", "pipe", "pipe"], + }) + + let captured = "" + + if (child.stdout && child.stderr) { + // decoded as text rather than concatenated as buffers, so a + // multi-byte character split across two chunks survives the join. + child.stdout.setEncoding("utf8") + child.stderr.setEncoding("utf8") + + const collect = (chunk: string) => { + captured += chunk + } + + child.stdout.on("data", collect) + child.stderr.on("data", collect) + } + + return new Promise((resolve, reject) => { + child.on("error", reject) + child.on("close", (code, signal) => { + if (code === 0) { + resolve() + + return + } + + const step = `${command} ${args.join(" ")}` + + // ctrl-c goes to the whole foreground process group, so the + // child is already gone by the time this runs — stopped on + // purpose, not broken. It shows up either as the signal + // itself or, where a shell script traps it, as the 128+n + // status that script exited with. + if (interrupted(code, signal)) { + reject(new StepError(`${step} was interrupted`)) + + return + } + + // an inherited command has already printed whatever it had + // to say, so only the captured form replays a log here. + const reason = signal ? `on ${signal}` : `with code ${String(code)}` + + reject( + new StepError( + `${step} exited ${reason}${captured ? `\n\n${captured}` : ""}`, + ), + ) + }) + }) +} + +function interrupted(code: number | null, signal: string | null): boolean { + return ( + signal === "SIGINT" || signal === "SIGTERM" || code === 130 || code === 143 + ) +} diff --git a/e2e/playwright.config.ts b/e2e/playwright.config.ts index 17d893cf..5c3be908 100644 --- a/e2e/playwright.config.ts +++ b/e2e/playwright.config.ts @@ -24,6 +24,7 @@ export default defineConfig({ ? [["github"], ["html", { open: "never" }]] : [["list"]], globalSetup: "./global-setup.ts", + globalTeardown: "./global-teardown.ts", use: { baseURL: BASE_URL, trace: "on-first-retry",