diff --git a/AGENTS.md b/AGENTS.md index 1d097171..2f594b69 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -1,52 +1,23 @@ -## Agent skills - -### Issue tracker - -GitHub Issues on eppicenter/sampledb3 (via `gh` CLI). See `docs/agents/issue-tracker.md`. - -### Triage labels - -Canonical triage roles mapped to GitHub labels; see `docs/agents/triage-labels.md`. - -### Domain docs - -`CONTEXT.md` (glossary) and `docs/adr/` (decisions) at repo root. See `docs/agents/domain.md`. - -### Package docs +# Agent guide -- Writing an API test (test database, factories, authenticated routes): `packages/api/src/__tests__/README.md` -- Writing a web test (render helper, API mocks, React Query hooks): `packages/web/src/__tests__/README.md` -- Changing the schema, a migration, or a transaction: `packages/api/src/db/README.md` -- Finding the code behind a UI label: `packages/web/src/components/Sidebar.tsx` (label → path) → `packages/web/src/App.tsx` (path → page) → the page's hooks and `lib/` → `packages/api/src/routes` → `packages/api/src/lib` - -### Tests - -| Package | Command | Runner | -|---------|---------|--------| -| `@sampledb/web` | `cd packages/web && bun run test -- ` | **Vitest** (jsdom, `vi.mock`) | -| `@sampledb/api` | `cd packages/api && bun test src/...` | **Bun** (needs `bun:sqlite`) | -| repo root | `bun run test` | both, via package scripts | - -**Run web tests with `bun run test` (Vitest).** It runs `vitest run`, which exits on its own, so piping to `tail` or `grep` is safe. Bun's own runner (`bun test`) hangs on React component tests. - -Example (completes in ~1s): - -```bash -cd packages/web && bun run test -- src/components/__tests__/BulkImportFlow.test.tsx -``` - -### Pre-commit hooks and CI +## Agent skills -The `Checks` workflow runs `bun run ci:verify` on every PR: typecheck, lint, build, unit tests, and the docs stale-claims check. Lint is clean, so any lint error is yours. +- **Issue tracker**: GitHub Issues on `eppicenter/sampledb3`. "Publish to the issue tracker" is `gh issue create`; "fetch the relevant ticket" is `gh issue view --comments`. +- **Triage labels**: each of the five triage roles is a GitHub label with the role's name: `needs-triage`, `needs-info`, `ready-for-agent`, `ready-for-human`, and `wontfix`. +- **Domain docs**: `CONTEXT.md` is the glossary, `docs/adr/` holds the decisions, and `packages/docs/` is the user guide. Before you name or change a domain concept, read its `CONTEXT.md` entry and the ADRs that touch the area. -Install repo hooks once per clone: +## Running tests and checks -```bash -bun run setup:hooks -``` +- Web tests: `cd packages/web && bun run test -- `. Bun's own runner (`bun test`) hangs on React component tests; Vitest exits on its own, so piping it to `tail` or `grep` is safe. +- API tests: `cd packages/api && NODE_ENV=test bun test src/...`. The package's `test` script sets `NODE_ENV=test`, and `src/db/legacy-baseline.ts` branches on it. +- Contract tests: `cd packages/contract && bun run test`. `bun run test` at the repo root runs the contract, API, and web suites. +- The `Checks` workflow lints every PR, so a lint error comes from your change. +- Run `bun run setup:hooks` once per clone to install the pre-commit hook (it typechecks and lints staged files, and refuses a source file that also has unstaged edits), and `bun run ci:verify` before pushing. The end of Build in `README.md` says what each checks. -The pre-commit hook runs `typecheck` on `@sampledb/contract`, `@sampledb/api`, and/or `@sampledb/web` when staged files touch those packages (or Docker/build config), and lints the staged API/web source files. It refuses a source file with unstaged edits, since eslint would lint the working-tree copy. To mirror CI fully before pushing: +## Where to look -```bash -bun run ci:verify -``` +- Rules for naming domain concepts, contradicting an ADR, failing tests, database access, calling the API from the web, and writing API and web tests: `CODING_STANDARDS.md`. The `code-review` skill checks changes against it. +- Writing an API test: `packages/api/src/__tests__/README.md`. +- Writing a web test: `packages/web/src/__tests__/README.md`. +- Changing the schema, adding a migration, or writing in a transaction: `packages/api/src/db/README.md`. +- Tracing a UI label to code: `packages/web/src/components/Sidebar.tsx` maps the label to a path and `packages/web/src/App.tsx` maps the path to a page. From there follow the page's hooks and `lib/api/` into `packages/api/src/routes` and `packages/api/src/lib`. diff --git a/CODING_STANDARDS.md b/CODING_STANDARDS.md new file mode 100644 index 00000000..97562f0b --- /dev/null +++ b/CODING_STANDARDS.md @@ -0,0 +1,43 @@ +# Coding standards + +## Domain language + +- Name a domain concept with the term `CONTEXT.md` defines for it, in code, test names, issue titles, and proposals. Treat a term the glossary lists under _Avoid_ as a synonym, and replace it with the defined term. +- When a concept has no glossary entry, say so in the issue, PR, or proposal before you introduce a term for it. +- When a change contradicts an ADR in `docs/adr/`, name the ADR and give the reason to reopen it, in the PR or proposal. For example: _Contradicts ADR 0006 (unified container inbound write), but worth reopening because…_ + +## Tests + +- Assert the expected behaviour. When a test fails, fix the code under test and keep the assertion as strict as before. You may land a new test that fails, if it states the expected behaviour. + +## API + +Paths are relative to `packages/api/`. + +### Database + +- Run a write of several statements that `await`s inside `withWriteTransaction` from `src/db/write-transaction.ts`. Drizzle's bun-sqlite `db.transaction(async () => …)` commits at the callback's first `await` instead of awaiting the returned Promise, so the rest of the write runs outside the transaction. ESLint rejects any callback passed to `.transaction(...)` that returns a Promise. +- Change the schema with a numbered migration, following the recipe in `src/db/README.md`, and refresh `initial_schema.sql` to match. Keep DDL out of the connection code (`src/db/open.ts`, `src/db/client.ts`), routes, and `src/lib/`. The schema-evolution modules in `src/db/` (`migration-runner.ts`, `migration-preflight.ts`, `legacy-baseline.ts`, `apply-initial-schema.ts`) run DDL as part of evolution, and tests may create tables inline to build a legacy or partial database, as `src/db/__tests__/client.test.ts` and `src/lib/subjects/__tests__/subject-read.test.ts` do. +- Open a database through `openOperationalDatabase`, which runs schema evolution. Open the file directly only when the code must not migrate it, and say why in a comment. For example, `src/scripts/check-integrity.ts` opens a backup with `new Database(path, { readonly: true })` so that checking it leaves it unchanged, and `src/db/__tests__/initial-schema-snapshot.test.ts` applies `initial_schema.sql` alone so that pending migrations can't hide a stale snapshot. + +### API tests + +- Give each test its own database from `setupTestDatabase()`, and close it with `cleanupTestDatabase()` in `afterEach`. +- Seed rows with the factories in `src/__tests__/helpers/factories.ts`. +- Test a route behind session auth through `setupAuthenticatedRouteTest`. Log in by hand with `loginAndGetCookie` only when the login flow is under test, as in the auth route and middleware tests. +- For a CRUD route with `checkInUse`, write a delete test for both the in-use and the free case. +- Test a route's error cases as well as its success: validation errors, 404s, and duplicates. + +## Web + +Paths are relative to `packages/web/`. + +- Call the API through its domain module, `src/lib/api/`. Its methods resolve to the response body, so read `res.study`. Reference-data `list()` helpers are the exception: their body is `{ data, meta? }` (ADR 0002). + +### Web tests + +- Mock the domain module the code imports with `createMockedDomainModule`. Make each override replace a whole `*Api` object and resolve to the same body the real module returns. +- In a new or changed test, render with `await render(...)` from `src/__tests__/helpers/render`, query elements by role or label (`getByRole`, `getByLabelText`), and drive interactions with `@testing-library/user-event`. Older tests still render with `@testing-library/react`'s `render`, click with `fireEvent`, and query by text; convert a test when you change it. Tests of a provider itself, such as `src/contexts/__tests__/ThemeContext.test.tsx`, render without the helper because it would wrap them in a second copy of the provider. +- Wait for async results with `findBy*` queries or `waitFor`. +- Test a component's loading and error states, not only its loaded state. +- In a mutation hook's test, assert which queries it invalidates. diff --git a/README.md b/README.md index fc499c7b..0d793692 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ bun run build bun run start ``` -Before pushing, run `bun run ci:verify` (typecheck + build) to match the Docker CI step. Install git pre-commit hooks with `bun run setup:hooks` to catch TypeScript errors on commit. +Before pushing, run `bun run ci:verify`, the same checks the `Checks` workflow runs on every PR. Install the pre-commit hook with `bun run setup:hooks`; it typechecks the packages your staged files touch and lints the staged API and web files. ## Project Structure @@ -72,8 +72,15 @@ Put local overrides in **`.env.local`** (gitignored) at the repo root. Bun’s a - **Unit tests (API + web)**: `bun run test` - **End-to-end (Playwright)**: `bun run test:e2e` -- **API**: `bun --filter @sampledb/api test` (Bun). Coverage: `bun --filter @sampledb/api test:coverage` (report in `packages/api/coverage/`). Target 90% lines; see `packages/api/src/__tests__/README.md`. -- **Web**: `bun --filter @sampledb/web test` (Vitest). Coverage: `bun --filter @sampledb/web test:coverage`. Coverage excludes `src/lib/api/**` and `src/**/*.css` so the 90% target applies to testable code. Thresholds in `packages/web/vitest.config.ts` can be raised in steps toward 90%. +- **API**: `bun --filter @sampledb/api test` (Bun). Coverage: `bun --filter @sampledb/api test:coverage` (report in `packages/api/coverage/`). Test helpers: `packages/api/src/__tests__/README.md`. +- **Web**: `bun --filter @sampledb/web test` (Vitest). Coverage: `bun --filter @sampledb/web test:coverage`. Test helpers: `packages/web/src/__tests__/README.md`. + +### Coverage + +This is the one place coverage targets are written down; the config files point here. Both packages target 90% line coverage. + +- **API**: no threshold is enforced. `coverageThreshold` in `packages/api/bunfig.toml` is commented out; set it to make `test:coverage` fail below that line coverage. The thresholds in `packages/api/vitest.config.ts` apply only to Vitest coverage runs, and Vitest cannot run the tests that touch `bun:sqlite`. +- **Web**: the thresholds in `packages/web/vitest.config.ts` are a floor that fails `test:coverage`. Raise them in steps toward 90%. Coverage excludes `src/lib/api/**` and `src/**/*.css`, so the target applies to testable code. ## Database Configuration diff --git a/docs/agents/domain.md b/docs/agents/domain.md deleted file mode 100644 index 1005bb42..00000000 --- a/docs/agents/domain.md +++ /dev/null @@ -1,17 +0,0 @@ -# Domain Docs - -Read these before naming or changing domain concepts: - -- **`CONTEXT.md`** is the domain glossary: entity names, hierarchies, and scope. -- **`docs/adr/`** records design decisions. Read the ADRs that touch the area you're changing. -- **`packages/docs/`** holds the user guide, for user-facing workflow detail. - -## Use the glossary's vocabulary - -When your output names a domain concept (an issue title, a test name, a refactor proposal), use the term as `CONTEXT.md` defines it, not a synonym it lists under _Avoid_. If the concept isn't in the glossary, say so rather than inventing a term. - -## Flag ADR conflicts - -If your change contradicts an ADR, say so explicitly instead of silently overriding it: - -> _Contradicts ADR-0006 (unified container inbound write), but worth reopening because…_ diff --git a/docs/agents/issue-tracker.md b/docs/agents/issue-tracker.md deleted file mode 100644 index cce77ecb..00000000 --- a/docs/agents/issue-tracker.md +++ /dev/null @@ -1,22 +0,0 @@ -# Issue tracker: GitHub - -Issues and PRDs for this repo live as GitHub issues. Use the `gh` CLI for all operations. - -## Conventions - -- **Create an issue**: `gh issue create --title "..." --body "..."`. Use a heredoc for multi-line bodies. -- **Read an issue**: `gh issue view --comments`, filtering comments by `jq` and also fetching labels. -- **List issues**: `gh issue list --state open --json number,title,body,labels,comments --jq '[.[] | {number, title, body, labels: [.labels[].name], comments: [.comments[].body]}]'` with appropriate `--label` and `--state` filters. -- **Comment on an issue**: `gh issue comment --body "..."` -- **Apply / remove labels**: `gh issue edit --add-label "..."` / `--remove-label "..."` -- **Close**: `gh issue close --comment "..."` - -Infer the repo from `git remote -v` — `gh` does this automatically when run inside a clone. - -## When a skill says "publish to the issue tracker" - -Create a GitHub issue. - -## When a skill says "fetch the relevant ticket" - -Run `gh issue view --comments`. diff --git a/docs/agents/triage-labels.md b/docs/agents/triage-labels.md deleted file mode 100644 index b716855d..00000000 --- a/docs/agents/triage-labels.md +++ /dev/null @@ -1,15 +0,0 @@ -# Triage Labels - -The skills speak in terms of five canonical triage roles. This file maps those roles to the actual label strings used in this repo's issue tracker. - -| Label in mattpocock/skills | Label in our tracker | Meaning | -| -------------------------- | -------------------- | ---------------------------------------- | -| `needs-triage` | `needs-triage` | Maintainer needs to evaluate this issue | -| `needs-info` | `needs-info` | Waiting on reporter for more information | -| `ready-for-agent` | `ready-for-agent` | Fully specified, ready for an AFK agent | -| `ready-for-human` | `ready-for-human` | Requires human implementation | -| `wontfix` | `wontfix` | Will not be actioned | - -When a skill mentions a role (e.g. "apply the AFK-ready triage label"), use the corresponding label string from this table. - -Edit the right-hand column to match whatever vocabulary you actually use. diff --git a/packages/api/bunfig.toml b/packages/api/bunfig.toml index 2c589c2c..630ecf32 100644 --- a/packages/api/bunfig.toml +++ b/packages/api/bunfig.toml @@ -1,10 +1,10 @@ [test] -# Only run tests from src directory, exclude dist (compiled files) +# Skip compiled output. The test scripts pass src. exclude = ["dist/**", "node_modules/**"] # Coverage: use Bun's built-in so all tests (including route tests using bun:sqlite) run coverage = true coverageReporter = ["text", "lcov"] coverageDir = "coverage" -# Optional: set to enforce minimum (e.g. 50). Current baseline ~55% lines; raise toward 60% as tests are added. +# Optional: set to fail the run below this line coverage. Targets: Coverage in the root README.md. # coverageThreshold = 50 coveragePathIgnorePatterns = ["node_modules", "src/__tests__", "src/**/*.d.ts", "src/**/*.config.*", "dist"] diff --git a/packages/api/src/__tests__/README.md b/packages/api/src/__tests__/README.md index e5ae6ea6..6013ea73 100644 --- a/packages/api/src/__tests__/README.md +++ b/packages/api/src/__tests__/README.md @@ -1,50 +1,22 @@ -# Backend Testing Guide +# Backend testing guide -## Setup +API tests run on Bun's runner because route and integration tests need `bun:sqlite`. Test files import `describe`, `it`, and `expect` from `'vitest'`; Bun maps that import to `bun:test`. `bun run test:ui` starts Vitest, which only runs tests that never touch `bun:sqlite`. -Tests use **Bun's test runner** (not Vitest) so that route and integration tests can use `bun:sqlite` in db-setup. Coverage uses **Bun's built-in coverage** (`bun test --coverage`) so all 431 tests are included in the report. Run tests with: +This guide covers the helpers. The rules are under API tests in `CODING_STANDARDS.md`. -```bash -bun test src # Run once (431 tests) -bun test --watch src # Watch mode -bun test:ui # Vitest UI (for lib-only tests; route tests require Bun) -bun test:coverage # Run tests with coverage (Bun; report in coverage/) -``` - -Coverage is configured in `packages/api/bunfig.toml` (reporters: text, lcov). Baseline has been raised with lib and route tests; the target is 90% lines. Optional `coverageThreshold` in bunfig.toml can be enabled and raised in steps (e.g. 60 → 75 → 90) as coverage improves. - -## Test Structure - -- `__tests__/helpers/` - Test utilities - - `db-setup.ts`: In-memory database setup and cleanup - - `factories.ts`: Test data factories for creating test entities - - `authenticated-route-test.ts`: Authenticated route test adapter (DB, login, `createRequestApp`, `request`) - - `test-client.ts`: Hono test client utilities and assertion helpers -- `__tests__/fixtures/` - Test data fixtures -- `routes/__tests__/` - Route handler tests -- `lib/__tests__/` - Library/utility function tests (e.g. plate-csv, plate-scan-validation, defaults, container-move, derivations, error-utils, logger, cache, openapi) -- `lib/controls/__tests__/` - Control domain libs (batch-with-specimens, batch-csv-validate, definition-summary, batch-delete, strain-composition, control-read) -- `middleware/__tests__/` - Middleware tests (e.g. auth, rate-limit) - -## Test Database - -Tests use an in-memory SQLite database created in `db-setup.ts`. The `setupTestDatabase()` function: -- Creates a fresh in-memory database for each test suite -- Applies `packages/api/initial_schema.sql` (same Drizzle-generated snapshot production uses via `applyInitialSchema`) -- Returns both the Drizzle database instance and SQLite instance for cleanup - -Regenerate the snapshot after schema changes: `bunx drizzle-kit generate` in `packages/api`, then copy the generated SQL to `initial_schema.sql` (see `drizzle.config.ts`). +## Test database -Integrity tests that seed orphan rows use `withForeignKeysDisabled` from `src/db/apply-initial-schema.ts` so fixtures match production constraints while allowing deliberate corruption. - -Example usage: +`setupTestDatabase()` in `helpers/db-setup.ts` opens an in-memory database through `openOperationalDatabase(':memory:')`, the production path, so it gets `initial_schema.sql` plus any pending migrations. To change the schema, follow `packages/api/src/db/README.md`. ```typescript +import { describe, beforeEach, afterEach } from 'vitest' +import type { Database as SQLiteDatabase } from 'bun:sqlite' +import type { Database } from '../../db/client' import { setupTestDatabase, cleanupTestDatabase } from '../../__tests__/helpers/db-setup' describe('My API', () => { let testDb: Database - let sqlite: any + let sqlite: SQLiteDatabase beforeEach(async () => { const setup = await setupTestDatabase() @@ -52,44 +24,25 @@ describe('My API', () => { sqlite = setup.sqlite }) - afterEach(() => { - if (sqlite) { - cleanupTestDatabase(sqlite) - } - }) + afterEach(() => cleanupTestDatabase(sqlite)) }) ``` -## Test Data Factories +Integrity tests that seed orphan rows wrap the inserts in `withForeignKeysDisabled` from `src/db/apply-initial-schema.ts`. + +## Factories -Use factories from `factories.ts` to create test data: +`helpers/factories.ts` exports a `createTest*` function per entity, taking the database and the fields to set: ```typescript -import { createTestState, createTestStorageContainer } from '../../__tests__/helpers/factories' +import { createTestStudy } from '../../__tests__/helpers/factories' -const testState = await createTestState(testDb, { name: 'Test State' }) +const study = await createTestStudy(testDb, { title: 'Test Study', shortCode: 'TS' }) ``` -Available factories: -- `createTestTag` -- `createTestStorageType` -- `createTestSpecimenType` -- `createTestStrain` -- `createTestStorageContainer` -- `createTestLocation` -- `createTestMicronixPlate`, `createTestCryovialBox`, `createTestBox`, `createTestBag`, `createTestSheet` -- `createTestMicronixTube`, `createTestCryovialTube`, `createTestStaticWell`, `createTestPaper` (give an existing storage container its subtype row) -- `createTestControlDefinition` -- `createTestSpecimen` -- `createTestUnit` -- `createTestSpecimenTypeContainerType`, `createTestContainerTypeUnit` (allow a container type for a specimen type, and a unit for a container type) -- `createTestStudy` -- `createTestStudySubject` -- `createTestControlBatch` - ## Authenticated route tests -For route handlers behind session auth, use `setupAuthenticatedRouteTest` instead of duplicating DB setup, settings, user creation, and login: +`setupAuthenticatedRouteTest` creates the database, auth settings, users, and a logged-in session, then sends requests with that session's cookie: ```typescript import { setupAuthenticatedRouteTest, type AuthenticatedRouteTestContext } from '../../__tests__/helpers/authenticated-route-test' @@ -104,7 +57,7 @@ describe('Search API', () => { settings: { pagination: true }, // when list routes use validateLimit additionalUsers: [{ key: 'member', email: 'member@test.com', role: 'member' }], seed: async ({ db }) => { /* optional fixtures before login */ }, - mount: (app, { db, sqlite }) => { + mount: (app, { db }) => { app.route('/api/search', createSearchRoutes(db)) }, }) @@ -118,120 +71,23 @@ describe('Search API', () => { }) it('returns 401 without auth', async () => { - const res = await authenticatedRequest(ctx.createRequestApp(), '/api/search?q=test', { method: 'GET' }) + const res = await ctx.request('/api/search?q=test', { cookie: null }) expect(res.status).toBe(401) }) }) ``` -Per-test routes (e.g. CRUD factory): omit `mount` and use `ctx.createRequestApp((app) => app.route('/api/tags', routes))`. - -### Integration workflows - -`__tests__/integration/` uses the same adapter for cross-route HTTP workflows (e.g. derivation create + list, collection move). Lib tests in `lib/__tests__/` still own pure logic; integration tests assert the full HTTP seam. - -## Writing Tests - -### Testing Routes with CRUD Factory - -For routes using `createCrudRoutes`, create the routes dynamically in tests with the test database: - -```typescript -import { createCrudRoutes } from '../../lib/crud-routes' -import { z } from 'zod' - -describe('States API', () => { - let statesRoutes: Hono - - beforeEach(async () => { - const { db } = await setupTestDatabase() - - const createSchema = z.object({ - name: z.string().min(1), - }) - - statesRoutes = createCrudRoutes({ - table: state, - database: db, - entityName: 'State', - pluralName: 'states', - singularName: 'state', - createSchema, - }) - }) -}) -``` - -### Testing Route Handlers - -```typescript -import { describe, it, expect, beforeEach } from 'vitest' -import { testClient } from 'hono/testing' -import { Hono } from 'hono' -import myRoutes from '../my-routes' - -describe('My API', () => { - let app: Hono - - beforeEach(() => { - app = new Hono() - app.route('/api/my-route', myRoutes) - }) - - it('should do something', async () => { - const client = testClient(app) - const res = await client.api['my-route'].$get() - expect(res.status).toBe(200) - const data = await res.json() - expect(data).toHaveProperty('items') - }) -}) -``` - -## Coverage - -Coverage thresholds are set to 60% for: -- Statements -- Branches -- Functions -- Lines - -Run coverage reports: - -```bash -bun test:coverage -``` - -Coverage reports are generated in: -- `coverage/` directory (HTML, JSON, text formats) - -## Test Utilities - -### `test-client.ts` Helpers - -- `createTestClient(app)`: Creates a Hono test client -- `getResponseData(response)`: Unpacks `{ data }` from standardized API responses -- `authenticatedRequest(app, path, options)`: HTTP request with session cookie -- `loginAndGetCookie` / `createAuthenticatedClientWrapper`: Session helpers for route tests - -## Running Tests From Repo Root - -From the repository root you can run all package tests and coverage: - -```bash -bun run test # Run API and web tests -bun run test:coverage # Run API and web tests with coverage -``` +`ctx.cookies[key]` holds each additional user's session cookie. To mount routes per test, as the `createCrudRoutes` tests do, omit `mount` and call `ctx.createRequestApp((app) => app.route('/api/tags', routes))`. -## New Tests and Failing Behavior +`__tests__/integration/` uses the same adapter for workflows that cross routes, such as derivation create then list. Pure logic stays in `lib/__tests__/`. -New tests are allowed to **fail** initially if they correctly express expected behavior. Follow-up work will update the implementation to satisfy the tests. Do not remove or relax assertions solely to make tests pass; fix the code under test instead. +## Other helpers -## Best Practices +`helpers/test-client.ts`: -1. **Always use test database**: Use `setupTestDatabase()` to get a fresh database for each test -2. **Clean up**: Always call `cleanupTestDatabase()` in `afterEach` -3. **Use factories**: Use test data factories instead of manual inserts -4. **Test edge cases**: Include tests for validation errors, 404s, duplicates, etc. -5. **Test "in use" scenarios**: For routes with `checkInUse`, test both in-use and not-in-use cases +- `getResponseData(response)` unpacks `{ data }` from a standard API response. +- `authenticatedRequest(app, path, options)` sends a request with a session cookie. `ctx.request` calls it for you. +- `createAuthenticatedClientWrapper(client, ctx.cookie)` adds the session cookie to a typed `hono/testing` client, for tests that build one on top of `setupAuthenticatedRouteTest`. +- `loginAndGetCookie` logs in and returns the session cookie. Call it directly only in tests of the login flow itself, such as `routes/__tests__/auth.test.ts` and `middleware/__tests__/auth.test.ts`. +`helpers/auth-helpers.ts` creates users and sessions and seeds the password, session, and pagination settings. diff --git a/packages/api/src/db/README.md b/packages/api/src/db/README.md index c02308f4..b40792b2 100644 --- a/packages/api/src/db/README.md +++ b/packages/api/src/db/README.md @@ -2,13 +2,39 @@ SampleDB stores **operational data** (studies, subjects, specimens, containers, controls, and related records) in SQLite. This package owns how that file is opened, versioned, and evolved. -See [ADR-0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evolution.md) for rationale (snapshot + deltas, no drizzle-kit migrate, fail-hard on error). +See [ADR 0003: SQLite schema evolution](../../../docs/adr/0003-sqlite-schema-evolution.md) for rationale (snapshot + deltas, no drizzle-kit migrate, fail-hard on error). + +## Changing the schema + +1. Edit Drizzle definitions in `schema.ts`. +2. Generate SQL: `bunx drizzle-kit generate` in `packages/api` (see `drizzle.config.ts`). +3. Add `packages/api/src/db/migrations/00N_short_name.sql` (statements separated with `--> statement-breakpoint`). +4. Bump `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`. +5. Run `bun run build` in `packages/api` — `scripts/runtime-assets.ts` copies migrations into `dist/db/migrations` and verifies the dist layout. If you add other on-disk runtime assets, register them in `RUNTIME_ASSETS` in that script. +6. Refresh `initial_schema.sql` so **new** empty databases match the latest shape (merge generated SQL; keep `INSERT INTO schema_version` at `CURRENT_SCHEMA_VERSION`). `__tests__/initial-schema-snapshot.test.ts` fails until the snapshot matches a database built by the migrations: columns, types, NOT NULL, defaults, indexes, and foreign keys. +7. Extend the legacy fixtures if the migration touches a table they don't create. Older-version tests in `__tests__/schema-evolution.test.ts` build minimal databases through `createTablesForLaterMigrations`, and `__tests__/client.test.ts` creates its own tables inline; both must contain every table and column a later migration alters. + +## How evolution works + +| Database state | Behaviour | +|----------------|-----------| +| Empty file | Apply `initial_schema.sql` snapshot (includes current version row), then any pending migrations `002+`. | +| Has tables, no `schema_version` | Run legacy baseline `001` once, then pending migrations. | +| Has `schema_version` | Apply numbered migrations where `version < CURRENT`. | + +Migration `001` is **only** for legacy files; fresh installs from the snapshot skip it. + +## Schema version + +- Table: `schema_version` with a single row (`version INTEGER NOT NULL`). +- Constant: `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`, the version the code expects. "How evolution works" calls it `CURRENT`. +- Inspect in support: `SELECT version FROM schema_version;` ## Opening a database Use `openOperationalDatabase(optionalPath?)` from the client module. It resolves the file path, enables WAL, turns on foreign keys, runs schema evolution, and returns Drizzle + raw `bun:sqlite` handles. Importing the client module does **not** open a connection. -Tests should use the same entry point (e.g. `openOperationalDatabase(':memory:')` via `setupTestDatabase()`), except a test that checks the snapshot itself: `__tests__/initial-schema-snapshot.test.ts` applies `initial_schema.sql` alone, because the entry point would run pending migrations on top and hide a stale snapshot. +The rules for transactions, DDL, and which entry point to open a database with are under Database in `CODING_STANDARDS.md`. **Exception: read-only inspection.** `scripts/check-integrity.ts` opens the file with `new Database(path, { readonly: true })` instead. It is often pointed at a backup, and `openOperationalDatabase` would migrate it and enable WAL (both are writes). Resolve the path with `resolveOperationalDatabasePath` so it finds the same file. Any other tool that only reads, and must leave the file as it found it, should do the same. @@ -21,8 +47,6 @@ The API shares one `bun:sqlite` connection across all requests, so a transaction | `withWriteTransaction(db, fn)` | Run multi-statement writes that `await` | `COMMIT`, or `ROLLBACK` if `fn` throws | | `withRolledBackTransaction(db, fn)` | Check that a write would succeed by performing it (validate routes) | `ROLLBACK`, always | -Drizzle's bun-sqlite `db.transaction(async () => …)` commits when the callback first yields; it does not await the returned Promise. ESLint (`eslint-rules/no-promise-transaction-callback.mjs`, tests included) rejects any callback passed to `.transaction(...)` whose return type is Promise-like: an `async` function, a named async function, or a sync function that returns a Promise. - ### Concurrency model - **One transaction at a time per connection.** A process-wide lock per connection serializes the helpers. A call from another async context, such as another request, waits for the lock and then runs its own `BEGIN`. @@ -40,34 +64,6 @@ Writes outside the helpers include plain `db.insert/update/delete` calls (about When you add a multi-statement write, use `withWriteTransaction` instead of adding to that list. -## Schema version - -- Table: `schema_version` with a single row (`version INTEGER NOT NULL`). -- Constant: `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`. -- Inspect in support: `SELECT version FROM schema_version;` - -## How evolution works - -| Database state | Behaviour | -|----------------|-----------| -| Empty file | Apply `initial_schema.sql` snapshot (includes current version row), then any pending migrations `002+`. | -| Has tables, no `schema_version` | Run legacy baseline `001` once, then pending migrations. | -| Has `schema_version` | Apply numbered migrations where `version < CURRENT`. | - -Migration `001` is **only** for legacy files; fresh installs from the snapshot skip it. - -## Changing the schema - -1. Edit Drizzle definitions in `schema.ts`. -2. Generate SQL: `bunx drizzle-kit generate` in `packages/api` (see `drizzle.config.ts`). -3. Add `packages/api/src/db/migrations/00N_short_name.sql` (statements separated with `--> statement-breakpoint`). -4. Bump `CURRENT_SCHEMA_VERSION` in `schema-evolution.ts`. -5. Run `bun run build` in `packages/api` — `scripts/runtime-assets.ts` copies migrations into `dist/db/migrations` and verifies the dist layout. If you add other on-disk runtime assets, register them in `RUNTIME_ASSETS` in that script. -6. Refresh `initial_schema.sql` so **new** empty databases match the latest shape (merge generated SQL; keep `INSERT INTO schema_version` at `CURRENT_SCHEMA_VERSION`). `__tests__/initial-schema-snapshot.test.ts` fails until the snapshot holds every table and column in `schema.ts`, and until its columns, declared types, NOT NULL, indexes and foreign keys match a version 1 install (`__tests__/fixtures/initial_schema_v1.sql`, never edited) brought forward by the migrations. -7. Extend the legacy fixtures if the migration touches a table they don't create. Older-version tests in `__tests__/schema-evolution.test.ts` build minimal databases through `createTablesForLaterMigrations`, and `__tests__/client.test.ts` creates its own tables inline; both must contain every table and column a later migration alters. - -Do **not** add inline DDL to `open.ts` or connection code. - ## Failures Each migration runs in a transaction. On failure, the transaction rolls back, `schema_version` is unchanged, and startup throws `SchemaMigrationError` with the migration number. Restore from backup or fix the database manually; there are no down-migrations. diff --git a/packages/api/vitest.config.ts b/packages/api/vitest.config.ts index 34a6e7ba..e91696eb 100644 --- a/packages/api/vitest.config.ts +++ b/packages/api/vitest.config.ts @@ -1,30 +1,14 @@ import { defineConfig } from 'vitest/config' import path from 'path' -/** - * Vitest Configuration - * - * NOTE: Known Issue - Stack Overflow During Cleanup - * - * You may see a "Maximum call stack size exceeded" error after tests complete. - * This is a known Vitest/tinypool issue with native modules (better-sqlite3). - * - * - All tests pass successfully before the error - * - Error only occurs during worker cleanup/teardown - * - This is NON-BLOCKING and can be safely ignored - * - Do not attempt to "fix" this - it's a Vitest/tinypool bug - * - * See: packages/api/src/__tests__/README.md for more details - */ +// Vitest backs only `bun run test:ui`. The suite runs on Bun's runner (bunfig.toml), and +// Vitest can run only the tests that never touch bun:sqlite. export default defineConfig({ test: { globals: true, environment: 'node', include: ['src/**/*.{test,spec}.{js,mjs,cjs,ts,mts,cts,jsx,tsx}'], exclude: ['node_modules', 'dist', '.idea', '.git', '.cache'], - // Use forks pool for native modules (better-sqlite3) - // This prevents segmentation faults and worker termination issues - // Note: This may cause a non-blocking stack overflow during cleanup pool: 'forks', coverage: { provider: 'v8', @@ -36,6 +20,7 @@ export default defineConfig({ 'src/**/__tests__/**', 'dist/**', ], + // Applies only to Vitest coverage runs. Targets: Coverage in the root README.md. thresholds: { statements: 60, branches: 60, diff --git a/packages/web/src/__tests__/README.md b/packages/web/src/__tests__/README.md index 8d4189a6..f73a6d38 100644 --- a/packages/web/src/__tests__/README.md +++ b/packages/web/src/__tests__/README.md @@ -1,76 +1,31 @@ -# Frontend Testing Guide +# Frontend testing guide -## Important: Known Vitest Issue +Web tests run on Vitest with jsdom and React Testing Library. `src/__tests__/setup.ts` runs before every file. It stubs `IntersectionObserver`, `scrollIntoView` and `localStorage` and mocks the `auth` and `settings` domain modules; override those per file as needed. -**Stack Overflow During Worker Cleanup (Non-Blocking)** +This guide covers the helpers. The rules are under Web tests in `CODING_STANDARDS.md`. -When running tests, you may see a `RangeError: Maximum call stack size exceeded` error after all tests complete. This is a **known non-blocking issue** with Vitest/tinypool during worker cleanup. +## Rendering -- **Impact**: None - all tests pass successfully before the error occurs -- **When it happens**: Only during worker cleanup/teardown, after all tests complete -- **Action required**: None - this can be safely ignored -- **Status**: All frontend tests pass; this is a cosmetic cleanup issue - -**Do not attempt to "fix" this error** - it's a known Vitest/tinypool bug that doesn't affect test results. - -## Setup - -Tests use Vitest + React Testing Library. **Do not use `bun test`** for this package — that invokes Bun's native runner, which lacks Vitest's `vi.importActual` / jsdom setup and can hang indefinitely on component tests. Use the npm scripts (Vitest) instead: - -```bash -bun run test # Run once (vitest run) -bun run test:watch # Watch mode -bun run test:ui # UI mode -bun run test:coverage # With coverage -``` - -## HTTP client mocks - -See [ADR 0002: HTTP client returns response bodies](../../../docs/adr/0002-http-client-response-unwrap.md). - -`lib/api/client` `api` methods resolve to the **response body**, not `AxiosResponse`. Domain mocks return body shapes directly (`{ user }`, `{ studies: [] }`), not `{ data: { … }, status, headers }`. - -Reference-data list helpers still return `{ data: T[], meta? }` — keep that envelope in mocks for `specimenTypesApi.list()` and similar. - -## Test Structure - -- `__tests__/helpers/` - Test utilities - - `render.tsx`: Custom render function with providers (QueryClient, BrowserRouter, ToastProvider) - - `mock-api.ts`: `createMockedDomainModule()` — `importActual` per `lib/api/` + per-test overrides - - `mock-api-templates.ts`: domain presets (`statisticsPageMock`, `studiesPageMock`, etc.) for common page/hook tests - - `setup.ts`: Global test setup (IntersectionObserver and `scrollIntoView` stubs, in-memory localStorage; default mocks for `auth` and `settings`) -- `__tests__/fixtures/` - Test data fixtures -- `lib/__tests__/` - Lib unit tests (e.g. commands, constants, plate-filename-match, container-types, localUserHistory, hotkeys, error-logger) -- `components/__tests__/` - Component tests (e.g. BulkImportFlow, ContainerDerivationModal) -- `components/forms/__tests__/` - Form tests (SubjectForm, StudyForm, ControlDefinitionForm, SpecimenForm) -- `components/wizards/__tests__/` - Wizard step tests (e.g. BatchInfoStep) -- `hooks/__tests__/` - Custom hook tests -- `pages/__tests__/` - Page smoke tests - -## Test Utilities - -### Custom Render Function - -The `render` function from `helpers/render.tsx` automatically wraps components with: -- `QueryClientProvider` (React Query) -- `BrowserRouter` (React Router) +`render` from `helpers/render.tsx` wraps the component in a fresh `QueryClient` (no retries), a router, and the theme, user, and toast providers. It is async because it flushes the user provider's first fetch. Pass `initialEntries` to use a `MemoryRouter` at a given URL. ```typescript import { render, screen } from '../../__tests__/helpers/render' +import userEvent from '@testing-library/user-event' import MyComponent from '../MyComponent' -it('renders component', () => { - render() - expect(screen.getByText('Hello')).toBeInTheDocument() +it('handles a click', async () => { + const user = userEvent.setup() + await render() + await user.click(screen.getByRole('button', { name: 'Save' })) + expect(await screen.findByRole('alert')).toHaveTextContent('Saved') }) ``` -### Mocking API Calls +## Mocking the API -Mock the **domain module** your component imports (`lib/api/studies`, `lib/api/client`, etc.). `setup.ts` provides default `auth` and `settings` (table view) mocks; override per file as needed. +`createMockedDomainModule(domain, overrides)` in `helpers/mock-api.ts` loads the real module with `vi.importActual` and replaces each export named in `overrides`. What to pass is under Web tests in `CODING_STANDARDS.md`: ```typescript -import { vi } from 'vitest' import { studiesApi } from '../../lib/api/studies' vi.mock('../../lib/api/studies', async () => { @@ -86,18 +41,11 @@ vi.mocked(studiesApi.list).mockResolvedValue({ }) ``` -Replace whole `*Api` objects in overrides (do not patch single methods on the real object). For raw axios usage, mock `lib/api/client` and pass `{ default: { get: vi.fn() } }`: +Import `createMockedDomainModule` with a dynamic `import()` inside the factory. Vitest hoists `vi.mock` above the file's imports, so a top-level import hits a TDZ error when the file also imports `helpers/render`. -```typescript -vi.mock('../../lib/api/client', async () => { - const { createMockedDomainModule } = await import('../../__tests__/helpers/mock-api') - return createMockedDomainModule('client', { default: { get: vi.fn() } }) -}) -``` +For raw axios calls, mock `lib/api/client` with `createMockedDomainModule('client', { default: { get: vi.fn() } })`. -Use dynamic `import()` inside the factory (not a top-level import of `createMockedDomainModule`) when the test file also imports `helpers/render` — Vitest hoists `vi.mock` and causes a TDZ error otherwise. - -Prefer `mock-api-templates` for repeated page setups (one `vi.mock` per affected domain): +`helpers/mock-api-templates.ts` has presets for common pages and hooks (`statisticsPageMock`, `studiesHooksMock`, and others). Use one `vi.mock` per affected domain: ```typescript vi.mock('../../lib/api/statistics', async () => { @@ -107,117 +55,22 @@ vi.mock('../../lib/api/statistics', async () => { }) ``` -Production and tests both import from domain modules under `src/lib/api/`. There is no aggregate barrel. - -## Writing Tests - -### Component Tests - -```typescript -import { describe, it, expect } from 'vitest' -import { render, screen } from '../../__tests__/helpers/render' -import userEvent from '@testing-library/user-event' -import MyComponent from '../MyComponent' - -describe('MyComponent', () => { - it('renders correctly', () => { - render() - expect(screen.getByText('Hello')).toBeInTheDocument() - }) +`helpers/settings-mocks.ts` builds settings values and `settingsApi.getValue` mocks. - it('handles user interaction', async () => { - const user = userEvent.setup() - render() - - const button = screen.getByRole('button') - await user.click(button) - - expect(screen.getByText('Clicked')).toBeInTheDocument() - }) -}) -``` +## React Query hooks -### React Query Hook Tests +Hook tests use `renderHook` with a wrapper that provides a fresh `QueryClient` with retries off, as in `hooks/__tests__/useStudies.test.tsx`: ```typescript -import { describe, it, expect, vi, beforeEach } from 'vitest' -import { renderHook, waitFor } from '@testing-library/react' -import { QueryClient, QueryClientProvider } from '@tanstack/react-query' -import { ReactNode } from 'react' -import { useStudies } from '../useStudies' -import { studiesApi } from '../../lib/api/studies' - -vi.mock('../../lib/api/studies', async () => { - const { createMockedDomainModule } = await import('../../__tests__/helpers/mock-api') - return createMockedDomainModule('studies', { studiesApi: { list: vi.fn() } }) -}) - function createWrapper() { const queryClient = new QueryClient({ - defaultOptions: { - queries: { retry: false, gcTime: 0 }, - mutations: { retry: false }, - }, + defaultOptions: { queries: { retry: false, gcTime: 0 }, mutations: { retry: false } }, }) - return ({ children }: { children: ReactNode }) => ( - - {children} - + {children} ) } -describe('useStudies', () => { - it('should fetch studies', async () => { - vi.mocked(studiesApi.list).mockResolvedValue({ - studies: [], - pagination: { total: 0, page: 1, limit: 10, totalPages: 0 }, - }) - - const { result } = renderHook(() => useStudies(), { - wrapper: createWrapper(), - }) - - await waitFor(() => { - expect(result.current.isSuccess).toBe(true) - }) - }) -}) +const { result } = renderHook(() => useStudies(), { wrapper: createWrapper() }) +await waitFor(() => expect(result.current.isSuccess).toBe(true)) ``` - -## Coverage - -Coverage excludes `src/**/*.css` and `src/lib/api.ts` (api.ts is exercised via hooks and page tests). Thresholds are set to a baseline and can be raised toward 90% as more tests are added. - -Run coverage reports: - -```bash -bun test:coverage -``` - -Coverage reports are generated in: -- `coverage/` directory (HTML, JSON, text formats) - -## Running Tests From Repo Root - -From the repository root you can run all package tests and coverage: - -```bash -bun run test # Run API and web tests -bun run test:coverage # Run API and web tests with coverage -``` - -## New Tests and Failing Behavior - -New tests are allowed to **fail** initially if they correctly express expected behavior. Follow-up work will update the implementation to satisfy the tests. Do not remove or relax assertions solely to make tests pass; fix the code under test instead. - -## Best Practices - -1. **Use custom render**: Always use `render` from `helpers/render.tsx` for components -2. **Mock API calls**: Use `vi.mock` to mock API modules in hook tests -3. **Test user interactions**: Use `@testing-library/user-event` for user interactions -4. **Test loading/error states**: Include tests for loading and error states -5. **Test cache invalidation**: For mutations, verify that queries are invalidated -6. **Use waitFor**: For async operations, use `waitFor` or `findBy` queries -7. **Test accessibility**: Use semantic queries (`getByRole`, `getByLabelText`) when possible - diff --git a/packages/web/vitest.config.ts b/packages/web/vitest.config.ts index d703b145..e238ca36 100644 --- a/packages/web/vitest.config.ts +++ b/packages/web/vitest.config.ts @@ -8,21 +8,6 @@ const appBuildId = process.env.VITE_APP_BUILD_ID || process.env.APP_BUILD_ID || // fails, wherever CI runs. Test workers inherit this. process.env.TZ = 'America/Los_Angeles' -/** - * Vitest Configuration - * - * NOTE: Known Issue - Stack Overflow During Cleanup - * - * You may see a "Maximum call stack size exceeded" error after tests complete. - * This is a known Vitest/tinypool issue with worker cleanup. - * - * - All tests pass successfully before the error - * - Error only occurs during worker cleanup/teardown - * - This is NON-BLOCKING and can be safely ignored - * - Do not attempt to "fix" this - it's a Vitest/tinypool bug - * - * See: packages/web/src/__tests__/README.md for more details - */ export default defineConfig({ define: { 'import.meta.env.VITE_APP_BUILD_ID': JSON.stringify(appBuildId), @@ -53,7 +38,7 @@ export default defineConfig({ 'src/lib/api/**', 'dist/**', ], - // Baseline; raise in steps (e.g. 50 → 65 → 80 → 90) as coverage improves (see test coverage plan). CSS and api.ts are excluded. + // A floor, not the target. Targets: Coverage in the root README.md. thresholds: { statements: 35, branches: 26,