From db29bdec6199fc3a30bca67c29f96c4521fc902d Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 29 Jul 2026 22:22:32 +0000 Subject: [PATCH 1/4] Document the test setup in docs/dev/testing.md The vitest environment split had no home in the docs tree. `jsdom` appeared once in all of docs/ (incidentally, in offline-cache.md) and `environmentMatchGlobs` appeared nowhere - the only record was an inline comment in vite.config.ts plus a commit message. Same for the other test infrastructure: the Deno island's two-task split, the postcss and markdownlint guardrail tests, integration-test gating on VENICE_INFERENCE_KEY, the Playwright specs, and the __test namespace convention were scattered or unwritten. The new doc collects all of it, following the feature-doc shape (Files / Contracts / Interactions / Gotchas). CLAUDE.md gets a pointer rather than a copy: it is read at session start but attention to it decays over a long session, while the dev docs are what a session consults when it is unsure. The one thing CLAUDE.md states outright is the jsdom trap, because it is actionable at the moment of writing a test rather than at the moment of reading a doc. The existing inline comment in vite.config.ts is reactive - it tells you to add your file to the list once you see `document is not defined`. That is the right instruction for a fresh branch and useless for one that forked before the split, where the old jsdom default makes the test pass locally, the rebase stays clean, the files stay disjoint, and only CI fails. Two stale figures fixed while in the neighbourhood: the manual pnpm fallback omitted knip, which now gates, and the Rollup chunk warning was quoted at 500 kB where vite.config.ts sets chunkSizeWarningLimit to 750. --- CLAUDE.md | 21 +++- docs/dev/README.md | 4 + docs/dev/build-deploy.md | 5 + docs/dev/testing.md | 264 +++++++++++++++++++++++++++++++++++++++ 4 files changed, 292 insertions(+), 2 deletions(-) create mode 100644 docs/dev/testing.md diff --git a/CLAUDE.md b/CLAUDE.md index fee6ba1a..85fea1a9 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -740,7 +740,10 @@ covers the full import graph each function deploys with. If you prefer raw pnpm (or mise isn't available - ephemeral sandboxes, first-time checkouts), the manual sequence is -`pnpm install && pnpm test && pnpm check && pnpm lint && pnpm build`. +`pnpm install && pnpm test && pnpm check && pnpm lint && pnpm build +&& pnpm knip`. Cloud sandboxes have hit `aqua:charmbracelet/gum: no +versions found matching date filter`, which fails `mise run check` +before any gate task runs - the raw sequence is the way through. `pnpm build` is in the gate because Vite/Rollup failures (IIFE/code-splitting in worker bundles, PWA manifest injection, dynamic-import graphs tsc is happy with but Rollup chokes on) only @@ -756,6 +759,20 @@ markdown-only changes. The test suite includes a postcss parse of every stylesheet under `src/` (`tests/styles.test.ts`) and a markdownlint pass over the doc tree (`tests/markdownlint.test.ts`). +**Adding a test that mounts a component or touches a DOM global? +Register it in `environmentMatchGlobs` in `vite.config.ts` in the +same change.** vitest defaults to the `node` environment and jsdom +is opt-in per file BY NAME, so a new file that needs a DOM dies on +`ReferenceError: document is not defined`. The list cannot know +about a file that did not exist when it was written, which makes +this invisible on a branch that forked before the split: the test +passes locally under the old jsdom default, the rebase is clean, the +files are disjoint, and it fails only in CI. + +Full details - the other two test islands, the guardrail tests, +integration-test gating, `tests/setup.ts` - are in +[`docs/dev/testing.md`](docs/dev/testing.md). + ### Check exit codes, not piped output Piping a gate command (`mise run check 2>&1 | tail -2`) replaces @@ -778,7 +795,7 @@ bitten us: imported by Y, dynamic import will not move module into another chunk.` Means your code-splitting isn't actually splitting because another module pulls the same target statically. -- `(!) Some chunks are larger than 500 kB after minification.` +- `(!) Some chunks are larger than 750 kB after minification.` Advisory; the asset list tells you which chunk is over. When doing bundle-shape work, grep the build output for `(!)` and diff --git a/docs/dev/README.md b/docs/dev/README.md index c21444a7..64beba44 100644 --- a/docs/dev/README.md +++ b/docs/dev/README.md @@ -170,6 +170,10 @@ changing a contract that other features depend on. - [Local dev stack](./local-stack.md) — a throwaway local Supabase backend (`mise run dev-start`) isolated from the linked cloud project, for schema work without touching prod. +- [Testing](./testing.md) — the three test islands (vitest, + Deno, Playwright), the vitest node/jsdom environment split + and how to register a new DOM test, the CSS + markdown + guardrail tests, and integration-test gating. ### Future work diff --git a/docs/dev/build-deploy.md b/docs/dev/build-deploy.md index 92975053..755f6a6d 100644 --- a/docs/dev/build-deploy.md +++ b/docs/dev/build-deploy.md @@ -170,6 +170,11 @@ docs imported via `import.meta.glob`. data-touching feature depends on. A broken schema migration halts the deploy; a broken build halts the publish. +- **Testing** — the gate (`mise run check`) exists to keep a + green Tests job from landing a change that breaks the + deploy: `pnpm build` runs in the gate, and the postcss and + markdownlint guardrail tests close the CSS and markdown + versions of the same hole. See `./testing.md`. - **Help** — Vite's `import.meta.glob` is what makes the user docs ship with the PWA. Each doc file becomes its own lazy chunk in `dist/assets/`. See `./help.md`. diff --git a/docs/dev/testing.md b/docs/dev/testing.md new file mode 100644 index 00000000..cfa4bdaf --- /dev/null +++ b/docs/dev/testing.md @@ -0,0 +1,264 @@ +# Testing + +How the test suites are wired, what each one exists to catch, and +the setup details that are easy to trip over. + +## Role in the app + +There are **three independent test islands**, and they do not see +each other: + +1. **vitest** (`tests/*.test.ts`) - the main-thread suite. Pure + logic, UI-behavior primitives, and component mounts. +2. **Deno** (`supabase/functions/tests/`) - the edge functions have + their own toolchain and are invisible to vitest. +3. **Playwright** (`e2e/*.spec.ts`) - browser-driven end-to-end + specs, run on demand rather than in the gate. + +The vitest suite also carries two **guardrail tests** that are not +really unit tests: a postcss parse of every stylesheet, and a +markdownlint pass over the doc tree. Both exist because their +failure modes otherwise surface only at deploy time or as a +silently-wrong render. + +## Files + +- `vite.config.ts` - the `test` block: environment split, setup + file, include/exclude globs, integration-test gating. +- `tests/setup.ts` - `setupFiles` for **every** vitest file, node + and jsdom alike. +- `tests/styles.test.ts` - postcss parse of every stylesheet under + `src/`. +- `tests/markdownlint.test.ts` - markdownlint over the tracked + markdown. +- `playwright.config.ts`, `e2e/` - the browser specs. +- `.mise.toml` - the gate task graph (`check` and its parallel + dependencies). +- `supabase/functions/deno.json` - import map the Deno tasks point + at. + +## The vitest environment split + +**`environment: 'node'` is the default. jsdom is opt-in, per file, +by name.** + +```ts +environment: 'node', +environmentMatchGlobs: [ + ['tests/ascii-spinner.test.ts', 'jsdom'], + // ... +], +``` + +Most test files are pure logic with no DOM dependency, and jsdom +costs roughly **350ms of bootstrap per file**. With ~128 test files +that was tens of seconds of aggregate environment time and 4-5s of +wall clock on every run. Listing the dozen files that genuinely need +a DOM buys that back. + +**When you add a test that mounts a component or touches a DOM +global, add it to `environmentMatchGlobs` in the same change.** +Things that need jsdom: component mounts (`@testing-library/svelte`), +`localStorage` / `sessionStorage`, the history API, DOMPurify, +`fake-indexeddb`. + +### Why this bites on a branch that forked before the split + +The list enumerates files **by name**, so it cannot know about a +file that did not exist when it was written. That makes the failure +invisible to every signal a branch normally trusts: + +- the test passes locally if the branch forked while jsdom was still + the default; +- the rebase is clean, because the two changes touch different + files; +- the gate is green before the rebase, and `git` reports no + conflict after it. + +It then fails in CI with `ReferenceError: document is not defined`. + +The inline comment in `vite.config.ts` tells you to add the file +*once you see that error*, which is the right instruction for a +fresh branch and useless for a rebased one. Treat "did I add a DOM +test?" as a rebase checklist item, not something the tooling will +remind you about. + +To confirm an entry is actually load-bearing rather than +cargo-culted, delete it and run the file - a CLI +`--environment node` flag will **not** override +`environmentMatchGlobs`, so the flag alone proves nothing. + +## `tests/setup.ts` + +Runs for every file in both environments, so everything in it is +either environment-agnostic or guarded: + +- **`@testing-library/jest-dom/vitest`** is imported here rather + than per file, so any component test gets + `expect(el).toBeInTheDocument()` without a local import. +- **`globalThis.crypto`** is polyfilled from `node:crypto` if absent + or missing `subtle`. +- **`Element.prototype.animate`** is shimmed because jsdom does not + implement it and Svelte's `slide` / `fade` transitions call it. + Guarded on `typeof Element !== 'undefined'` so the node + environment skips it. The shim only needs `cancel()` for Svelte's + lifecycle to clean up; the animation is a no-op, which is fine + because tests assert on state, not interpolated styles. + +## Guardrail tests + +Neither of these tests a feature. Both exist because a green Tests +job used to be compatible with a broken deploy. + +- **`tests/styles.test.ts`** runs postcss over every stylesheet + under `src/`. A stray `}` in `src/styles.css` once got through: + Vite's dev server kept rendering, `pnpm check` and `pnpm test` + do not parse CSS, and the error surfaced only at `pnpm build` - + in the deploy workflow, after Tests had gone green. +- **`tests/markdownlint.test.ts`** runs markdownlint-cli2's + programmatic `main()` (not a child process) over the tracked + markdown. Docs render in three places - GitHub, the in-app Help + modal, and Claude sessions reading `CLAUDE.md` - and a broken + fence ladder shows up as a wrong render rather than a loud + failure. + +**Consequence: run the suite for CSS-only and markdown-only +changes too.** There is no such thing as a change too cosmetic for +the gate. + +## Integration tests + +Files matching `tests/**/*.integration.test.ts` hit live Venice and +are **excluded unless `VENICE_INFERENCE_KEY` is set**: + +```sh +VENICE_INFERENCE_KEY= pnpm test tests/web-search.integration.test.ts +``` + +Presence of the key **is** the opt-in - there is no separate flag to +remember. The default `pnpm test` stays hermetic, so CI never +depends on outbound network or a credential. + +## The Deno island + +The edge functions run on their own toolchain and are invisible to +vitest. Two tasks cover them, and the gate runs both: + +- **`mise run functions-test`** - offline unit tests (fake fetch, no + network, no Supabase) over the pure logic in `_shared`. Handler + glue is exercised via `dev-start`'s `functions serve`, not here. +- **`mise run functions-check`** - `deno check` over every function + **entrypoint**. + +`functions-check` is the one that is easy to think redundant and +is not. `deno test` only type-checks the graph its tests import, +and `supabase functions deploy` bundles with esbuild and never +type-checks at all. Without the entrypoint check, a type error in a +handler-graph corner no test imports surfaces **nowhere** and ships +latent. + +It `depends = ["bundle-docs"]` because the venice function imports +the gitignored research-docs corpus, which does not exist on a +fresh clone until the bundler runs. + +## Playwright + +`playwright.config.ts` plus `e2e/` (`setup.spec.ts`, +`setup-hash.spec.ts`), run with `pnpm test:e2e`. **Not in the +gate** - they drive a real browser against a running stack. + +Cloud agent sessions cannot run these, or verify anything visual; +see the "Verifying UI changes" section of `CLAUDE.md` for the +posture that replaces them there. + +## Test-only exports + +Production modules expose internals to tests through a single +`__test = { ... }` namespace export at the bottom of the file +rather than widening their real API one symbol at a time. See +`src/lib/session.ts`, `src/lib/routing.svelte.ts`, +`src/lib/offline-sync.svelte.ts`, `src/lib/pdf-pages.ts`. + +Knip runs in the gate, so an export with no external consumer is a +gate failure, not a lint suggestion. + +## Contracts + +- **The gate is `mise run check`**, and it is what + `.github/workflows/tests.yml` runs, so green locally means green + CI. Its components are parallel `depends`, not a serial `run` + list, so all of them report even when one fails. +- **Every gate task `depends = ["deps"]`**, which runs `pnpm + install --frozen-lockfile`. No separate install step from a fresh + clone or worktree. +- **UI-behavior primitives are tested as plain vitest cases** - no + mount, no harness. That is the point of extracting them out of + `.svelte` files; see `./frontend-organization.md`. +- **New DOM-touching test files register in + `environmentMatchGlobs`.** + +## Interactions with other features + +- **Build & deploy** - the gate exists to keep a green Tests job + from landing a change that breaks the deploy. `pnpm build` is in + the gate for exactly that reason, and the two guardrail tests + close the CSS and markdown versions of the same hole. See + `./build-deploy.md`. +- **Frontend organization** - the extract-primitives-to- + `src/lib/ui/` rule is what keeps most of the suite in the fast + node environment; logic that stays in a `.svelte` file can only + be tested by a jsdom mount. See `./frontend-organization.md`. +- **Help** - `tests/markdownlint.test.ts` guards the docs the Help + modal renders. See `./help.md`. +- **Local dev stack** - edge-function handler glue is exercised + against `dev-start`'s `functions serve`, not by + `functions-test`. See `./local-stack.md`. +- **Every feature with a QA walkthrough** - `docs/qa/use-cases/` + covers the seams these suites cannot reach. See + `docs/qa/README.md`. + +## Gotchas + +- **A CLI `--environment` flag does not override + `environmentMatchGlobs`.** Passing + `--environment node` to a file that is on the jsdom list still + gets jsdom. To test what an unlisted file would do, remove the + entry. +- **`mise` may not resolve its tools in an ephemeral sandbox.** + Cloud sessions have hit `aqua:charmbracelet/gum: no versions + found matching date filter`, which fails `mise run check` before + any gate task runs. The documented fallback is the raw pnpm + sequence (`pnpm install && pnpm test && pnpm check && pnpm lint + && pnpm build && pnpm knip`) - note it must include `knip`, which + is part of the gate. +- **Piping a gate command replaces its exit code** with the pipe + tail's, so a failed gate reads as success and a chained `git + commit` runs anyway. Capture the status explicitly or run the + gate un-piped. See the `CLAUDE.md` section on this - it has + shipped broken commits twice. +- **Exit 0 is necessary but not sufficient.** `pnpm build` emits + Rollup warnings that do not fail the gate but do signal real + problems - grep the output for `(!)` when doing bundle-shape + work. +- **A script-less `.svelte` file has no inferable component type.** + svelte-check fails its importers with "Could not find a + declaration file for module ... implicitly has an 'any' type". + A component that needs no props still needs an empty + `