From 4aa8950dbaf3c63b17d8848ff8bdb9b6d3b9de08 Mon Sep 17 00:00:00 2001 From: Luke Bennett Date: Sat, 4 Jul 2026 10:15:45 +1000 Subject: [PATCH] Add test-writing guidance in docs/TESTING.md (#61) Consolidate the two conflicting Testing sections in CONVENTIONS.md into a single pointer, and move the docs-tools tests out of __tests__ to sit beside their sources per the colocation rule. --- AGENTS.md | 4 +- docs/CONVENTIONS.md | 17 +-- docs/TESTING.md | 107 ++++++++++++++++++ knip.config.ts | 2 +- .../{__tests__ => }/discover-exports.test.ts | 2 +- .../{__tests__ => }/fixtures/sample-barrel.ts | 0 .../fixtures/sample-component.ts | 0 .../src/kit/primitive/index.tsx | 0 .../sample-package/src/sample/index.tsx | 0 .../src/single/primitive/index.tsx | 0 .../fixtures/sample-type-alias.ts | 0 .../fixtures/sample-wrapper.ts | 0 .../package-docs-catalog.test.ts | 2 +- .../src/{__tests__ => }/parse-types.test.ts | 2 +- .../src/{__tests__ => }/render-index.test.ts | 4 +- .../{__tests__ => }/render-llms-full.test.ts | 4 +- .../src/{__tests__ => }/render-page.test.ts | 6 +- packages/@luke-ui/docs-tools/vite.config.ts | 2 +- 18 files changed, 125 insertions(+), 27 deletions(-) create mode 100644 docs/TESTING.md rename packages/@luke-ui/docs-tools/src/{__tests__ => }/discover-exports.test.ts (98%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-barrel.ts (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-component.ts (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-package/src/kit/primitive/index.tsx (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-package/src/sample/index.tsx (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-package/src/single/primitive/index.tsx (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-type-alias.ts (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/fixtures/sample-wrapper.ts (100%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/package-docs-catalog.test.ts (96%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/parse-types.test.ts (98%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/render-index.test.ts (95%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/render-llms-full.test.ts (96%) rename packages/@luke-ui/docs-tools/src/{__tests__ => }/render-page.test.ts (97%) diff --git a/AGENTS.md b/AGENTS.md index 51c0f73d..0fd62cf3 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -2,8 +2,8 @@ - Use `catalog:` in `package.json` for dependency versions (catalog in `pnpm-workspace.yaml`). Do not add raw versions. -- See [docs/CONVENTIONS.md](docs/CONVENTIONS.md), [docs/STYLING.md](docs/STYLING.md) for conventions - and styling. +- See [docs/CONVENTIONS.md](docs/CONVENTIONS.md), [docs/STYLING.md](docs/STYLING.md), and + [docs/TESTING.md](docs/TESTING.md) for conventions, styling, and testing. - Run tasks through turbo from the repo root (`pnpm run check`, `pnpm run build`, …). Running package-local scripts directly skips turbo's `generate` dependencies, so generated files (`.generated/`, `routeTree.gen.ts`, spritesheet) may be missing. diff --git a/docs/CONVENTIONS.md b/docs/CONVENTIONS.md index 87f84b83..e7a0f393 100644 --- a/docs/CONVENTIONS.md +++ b/docs/CONVENTIONS.md @@ -18,12 +18,10 @@ Managed by `oxfmt`. Tabs, 2 width, 80 width, single quotes (TS), double quotes ( ## Testing -Use the smallest test surface that proves the behavior. - -- **Unit tests**: pure logic, generators, scripts, docs tooling, package metadata, and non-React utilities. Put these in `src/**/__tests__/**/*.test.ts`. -- **Storybook play tests**: React component behavior that belongs in a real story. For `@luke-ui/react` components, stories are the component tests; do not add separate `*.test.tsx` component tests unless Storybook cannot exercise the behavior cleanly. -- **Storybook visual tests**: public UI states and visual variants worth reviewing for regressions. -- **Browser Vitest tests**: non-component DOM logic that needs real browser APIs and does not fit a story. +See [TESTING.md](TESTING.md) for choosing a test type, placement, and how to write tests. The short +version: use the smallest test surface that proves the behavior, colocate tests with the source +they cover, test behavior through public APIs and role-based queries, and start bugfixes with a +failing test. ## Component Pattern @@ -46,13 +44,6 @@ underpin a composed component are exported at `[composed]/primitive` (`@luke-ui/react/text-field/primitive`, `@luke-ui/react/combobox-field/primitive`, `@luke-ui/react/field/primitive`). The `button/primitive` path follows the same pattern. -## Testing - -Tests colocate with the source file they cover; no `__tests__` directories except for suites that -don't map to a single file (e.g. e2e). Never add DOM shims (happy-dom, jsdom) — DOM-dependent tests -run in a real browser (`*.browser.test.{ts,tsx}`, see `vitest.config.ts`), preferring real APIs over -stubs. - ## Exports Managed by `tsdown` entry globs. Do not hand-edit `package.json#exports`. Create files in paths the diff --git a/docs/TESTING.md b/docs/TESTING.md new file mode 100644 index 00000000..c7c94db3 --- /dev/null +++ b/docs/TESTING.md @@ -0,0 +1,107 @@ +# Testing + +How to choose, place, and write tests in this repo. + +## Choosing a test type + +Use the smallest test surface that proves the behavior. + +- **Unit tests** (`*.test.ts`): pure logic, generators, scripts, docs tooling, package metadata, + and non-React utilities. +- **Storybook play tests**: React component behavior that belongs in a real story. For + `@luke-ui/react` components, stories are the component tests; do not add separate `*.test.tsx` + component tests unless Storybook cannot exercise the behavior cleanly. +- **Storybook visual tests**: public UI states and visual variants worth reviewing for regressions. +- **Browser Vitest tests** (`*.browser.test.{ts,tsx}`): non-component DOM logic that needs real + browser APIs and does not fit a story — including style-contract tests for CSS recipes (see + below). + +## Placement + +Tests colocate with the source file they cover (`foo.test.ts` beside `foo.ts`); no `__tests__` +directories except for suites that don't map to a single file (e.g. e2e). Never add DOM shims +(happy-dom, jsdom) — DOM-dependent tests run in a real browser (see `vitest.config.ts`), preferring +real APIs over stubs. + +## Write the test first + +For bugfixes, always start with a failing test that reproduces the bug, and watch it fail for the +right reason before touching the fix. The test is what proves the fix and keeps the bug from +returning. + +For features, prefer writing the test first when the behavior is specifiable up front. Exploratory +UI work may need the component sketched before a story or play test makes sense — that's fine, but +the tests land in the same change as the behavior they cover. + +## Test behavior, not implementation + +A test should survive any refactor that preserves behavior. Exercise the code the way its consumers +do — through the public API for modules, through roles and interactions for components — and assert +the outcome a consumer can observe. + +Never assert on internals: private functions, call counts of the repo's own modules, generated +class names, or the text of CSS selectors. If a test can only be written by reaching into +internals, that's a signal the module's interface is missing something, not that the test needs a +back door. + +Mock only at true system boundaries (network, clock, external processes), and prefer real +implementations everywhere else — the same philosophy as running DOM tests in a real browser. +Filesystem-dependent tests use temp directories and the real `fs` rather than mocks. + +## Behavior tests: queries and interactions + +Behavior tests — play functions and anything else simulating a user — find elements the way +assistive technology does: + +- Query by **role with accessible name** (`getByRole('combobox', { name: 'Country' })`), falling + back to label or visible text when no role fits. +- **No test IDs, no CSS selectors.** This is a component library: if an element can't be found by + role or accessible name, assistive-technology users can't find it either. Fix the component, not + the test. +- Interact through **`userEvent` only** (click, keyboard, tab). Never `fireEvent`, manual event + dispatch, or mutating attributes/state to fake an interaction the user would perform. + +## Declarative over imperative + +State what the behavior is; don't narrate a journey. + +- **Name and assert one behavior at a time.** Each test (or `step()`, below) states a single + behavior — "selecting an option closes the popover" — and asserts that outcome. Don't add + checkpoint assertions after every interaction; steps exist only to reach the state being + asserted. +- **Set up state with props, not interactions.** Start from the state under test + (`defaultValue`, `isReadOnly`) instead of scripting clicks to arrive there. Interactions appear + only when the interaction itself is the behavior under test. +- **Map behaviors to `step()`, not to new stories.** Keep one story per meaningful state or props + combination; inside its play function, group each behavior in a named `step()` from + `storybook/test`. Steps report individually without adding sidebar entries or visual-test + snapshots of identical-looking states. + +```ts +play: async ({ canvasElement, step }) => { + const canvas = within(canvasElement); + const combobox = canvas.getByRole('combobox', { name: 'Country' }); + + await step('selecting an option closes the popover and fills the input', async () => { + await userEvent.click(combobox); + await userEvent.click(within(document.body).getByRole('option', { name: 'Australia' })); + await expect(combobox).toHaveValue('Australia'); + await expect(combobox).toHaveAttribute('aria-expanded', 'false'); + }); +}, +``` + +## Style-contract tests for CSS recipes + +Recipes (`src/recipes/*.css.ts`) have no roles or user interactions — their contract is "given a +DOM structure in a given state, the element computes these styles". Tests for them are the one +place raw DOM construction and `querySelector` are appropriate: there is no user to impersonate. + +- Assert **computed styles** (via `getComputedStyle`, resolving tokens to concrete values) — the + outcome a user sees. Never assert generated class names or selector strings; those are the + implementation. +- Build the DOM the recipe documents (e.g. a control containing an input and a trigger button), + not incidental markup that happens to pass. +- Every state a recipe test covers must also exist as a story on at least one consuming component, + so the recipe is exercised against real component markup in visual tests. If the story is + missing, add it in the same change. diff --git a/knip.config.ts b/knip.config.ts index 95a5df1a..cdf9c8bb 100644 --- a/knip.config.ts +++ b/knip.config.ts @@ -17,7 +17,7 @@ export default { project: ['src/**/*.{ts,tsx}'], }, 'packages/@luke-ui/docs-tools': { - entry: ['src/**/__tests__/**/*.test.ts'], + entry: ['src/**/*.test.ts'], project: ['src/**/*.ts'], }, 'packages/@luke-ui/react': { diff --git a/packages/@luke-ui/docs-tools/src/__tests__/discover-exports.test.ts b/packages/@luke-ui/docs-tools/src/discover-exports.test.ts similarity index 98% rename from packages/@luke-ui/docs-tools/src/__tests__/discover-exports.test.ts rename to packages/@luke-ui/docs-tools/src/discover-exports.test.ts index 9e49a86c..be052aa0 100644 --- a/packages/@luke-ui/docs-tools/src/__tests__/discover-exports.test.ts +++ b/packages/@luke-ui/docs-tools/src/discover-exports.test.ts @@ -1,6 +1,6 @@ import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vite-plus/test'; -import { discoverExports } from '../discover-exports.js'; +import { discoverExports } from './discover-exports.js'; const packageRoot = fileURLToPath(new URL('./fixtures/sample-package/', import.meta.url)); diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-barrel.ts b/packages/@luke-ui/docs-tools/src/fixtures/sample-barrel.ts similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-barrel.ts rename to packages/@luke-ui/docs-tools/src/fixtures/sample-barrel.ts diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-component.ts b/packages/@luke-ui/docs-tools/src/fixtures/sample-component.ts similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-component.ts rename to packages/@luke-ui/docs-tools/src/fixtures/sample-component.ts diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-package/src/kit/primitive/index.tsx b/packages/@luke-ui/docs-tools/src/fixtures/sample-package/src/kit/primitive/index.tsx similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-package/src/kit/primitive/index.tsx rename to packages/@luke-ui/docs-tools/src/fixtures/sample-package/src/kit/primitive/index.tsx diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-package/src/sample/index.tsx b/packages/@luke-ui/docs-tools/src/fixtures/sample-package/src/sample/index.tsx similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-package/src/sample/index.tsx rename to packages/@luke-ui/docs-tools/src/fixtures/sample-package/src/sample/index.tsx diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-package/src/single/primitive/index.tsx b/packages/@luke-ui/docs-tools/src/fixtures/sample-package/src/single/primitive/index.tsx similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-package/src/single/primitive/index.tsx rename to packages/@luke-ui/docs-tools/src/fixtures/sample-package/src/single/primitive/index.tsx diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-type-alias.ts b/packages/@luke-ui/docs-tools/src/fixtures/sample-type-alias.ts similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-type-alias.ts rename to packages/@luke-ui/docs-tools/src/fixtures/sample-type-alias.ts diff --git a/packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-wrapper.ts b/packages/@luke-ui/docs-tools/src/fixtures/sample-wrapper.ts similarity index 100% rename from packages/@luke-ui/docs-tools/src/__tests__/fixtures/sample-wrapper.ts rename to packages/@luke-ui/docs-tools/src/fixtures/sample-wrapper.ts diff --git a/packages/@luke-ui/docs-tools/src/__tests__/package-docs-catalog.test.ts b/packages/@luke-ui/docs-tools/src/package-docs-catalog.test.ts similarity index 96% rename from packages/@luke-ui/docs-tools/src/__tests__/package-docs-catalog.test.ts rename to packages/@luke-ui/docs-tools/src/package-docs-catalog.test.ts index ec1dcb1c..2474ce22 100644 --- a/packages/@luke-ui/docs-tools/src/__tests__/package-docs-catalog.test.ts +++ b/packages/@luke-ui/docs-tools/src/package-docs-catalog.test.ts @@ -1,6 +1,6 @@ import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vite-plus/test'; -import { resolvePackageDocsCatalog } from '../package-docs-catalog.js'; +import { resolvePackageDocsCatalog } from './package-docs-catalog.js'; const packageRoot = fileURLToPath(new URL('./fixtures/sample-package/', import.meta.url)); diff --git a/packages/@luke-ui/docs-tools/src/__tests__/parse-types.test.ts b/packages/@luke-ui/docs-tools/src/parse-types.test.ts similarity index 98% rename from packages/@luke-ui/docs-tools/src/__tests__/parse-types.test.ts rename to packages/@luke-ui/docs-tools/src/parse-types.test.ts index 48985241..7ee0bcde 100644 --- a/packages/@luke-ui/docs-tools/src/__tests__/parse-types.test.ts +++ b/packages/@luke-ui/docs-tools/src/parse-types.test.ts @@ -1,6 +1,6 @@ import { fileURLToPath } from 'node:url'; import { describe, expect, it } from 'vite-plus/test'; -import { parseBarrel, parseComponent } from '../parse-types.js'; +import { parseBarrel, parseComponent } from './parse-types.js'; const fixturePath = fileURLToPath(new URL('./fixtures/sample-component.ts', import.meta.url)); const typeAliasFixturePath = fileURLToPath( diff --git a/packages/@luke-ui/docs-tools/src/__tests__/render-index.test.ts b/packages/@luke-ui/docs-tools/src/render-index.test.ts similarity index 95% rename from packages/@luke-ui/docs-tools/src/__tests__/render-index.test.ts rename to packages/@luke-ui/docs-tools/src/render-index.test.ts index 4636f31c..88e3996b 100644 --- a/packages/@luke-ui/docs-tools/src/__tests__/render-index.test.ts +++ b/packages/@luke-ui/docs-tools/src/render-index.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vite-plus/test'; -import type { PackageDocsCatalogMetadata } from '../package-docs-catalog.js'; -import { renderIndex } from '../render-index.js'; +import type { PackageDocsCatalogMetadata } from './package-docs-catalog.js'; +import { renderIndex } from './render-index.js'; const sampleEntries: Array = [ { diff --git a/packages/@luke-ui/docs-tools/src/__tests__/render-llms-full.test.ts b/packages/@luke-ui/docs-tools/src/render-llms-full.test.ts similarity index 96% rename from packages/@luke-ui/docs-tools/src/__tests__/render-llms-full.test.ts rename to packages/@luke-ui/docs-tools/src/render-llms-full.test.ts index fcadfc3d..5c5f6a6d 100644 --- a/packages/@luke-ui/docs-tools/src/__tests__/render-llms-full.test.ts +++ b/packages/@luke-ui/docs-tools/src/render-llms-full.test.ts @@ -1,6 +1,6 @@ import { describe, expect, it } from 'vite-plus/test'; -import type { PackageDocsCatalogMetadata } from '../package-docs-catalog.js'; -import { renderLlmsFull, sortLlmsFullEntries } from '../render-llms-full.js'; +import type { PackageDocsCatalogMetadata } from './package-docs-catalog.js'; +import { renderLlmsFull, sortLlmsFullEntries } from './render-llms-full.js'; type LlmsFullFixture = Pick & { md: string; diff --git a/packages/@luke-ui/docs-tools/src/__tests__/render-page.test.ts b/packages/@luke-ui/docs-tools/src/render-page.test.ts similarity index 97% rename from packages/@luke-ui/docs-tools/src/__tests__/render-page.test.ts rename to packages/@luke-ui/docs-tools/src/render-page.test.ts index 776b7f58..d6aa2dbe 100644 --- a/packages/@luke-ui/docs-tools/src/__tests__/render-page.test.ts +++ b/packages/@luke-ui/docs-tools/src/render-page.test.ts @@ -1,7 +1,7 @@ import { describe, expect, it } from 'vite-plus/test'; -import type { PackageDocsBarrelEntry, PackageDocsComponentEntry } from '../package-docs-catalog.js'; -import type { ParsedComponent } from '../parse-types.js'; -import { renderPage } from '../render-page.js'; +import type { PackageDocsBarrelEntry, PackageDocsComponentEntry } from './package-docs-catalog.js'; +import type { ParsedComponent } from './parse-types.js'; +import { renderPage } from './render-page.js'; const sampleParsed: ParsedComponent = { description: 'Sample composed button.', diff --git a/packages/@luke-ui/docs-tools/vite.config.ts b/packages/@luke-ui/docs-tools/vite.config.ts index be56e2b1..7a9f518c 100644 --- a/packages/@luke-ui/docs-tools/vite.config.ts +++ b/packages/@luke-ui/docs-tools/vite.config.ts @@ -3,6 +3,6 @@ import { defineConfig } from 'vite-plus'; export default defineConfig({ test: { environment: 'node', - include: ['src/**/__tests__/**/*.test.ts'], + include: ['src/**/*.test.ts'], }, }); -- 2.51.2