Files
familienarchiv/docs/adr/012-browser-test-mocking-strategy.md
2026-05-12 17:43:11 +02:00

93 lines
5.8 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# ADR 012 — Browser-Mode Test Mocking Strategy
**Status:** Accepted
**Date:** 2026-05-11
**Issue:** [#535 — Unit & Component Tests job exits 1 from vitest-browser teardown race](https://git.raddatz.cloud/marcel/familienarchiv/issues/535)
---
## Context
Vitest browser-mode tests (the `client` project, run with `@vitest/browser-playwright` / Chromium) use a different module resolution path than Node-environment tests. When a spec calls `vi.mock('some-module', factory)`, vitest registers a `ManualMockedModule`. At runtime, every time Chromium requests that module, a playwright route handler intercepts the request and calls the Node worker over **birpc** (`resolveManualMock`) to evaluate the factory and return the module body.
This is safe for modules that are imported **statically** at spec module-eval time (e.g. `$app/navigation`, `$env/static/public`): those requests resolve before the first test runs and well before any teardown occurs.
It is **unsafe** for modules that are imported **dynamically** (e.g. inside an `async onMount`, inside a lazy-loaded chunk): Chromium may fetch the module after the worker's birpc channel has already closed, producing:
```
Error: [birpc] rpc is closed, cannot call "resolveManualMock"
ManualMockedModule.factory node_modules/@vitest/browser/dist/index.js:3221:34
```
This raises an unhandled rejection that exits the vitest process with code 1, even though every test in the run reported green.
`pdfjs-dist` and `pdfjs-dist/build/pdf.worker.min.mjs?url` are loaded via `await Promise.all([import('pdfjs-dist'), import('pdfjs-dist/build/pdf.worker.min.mjs?url')])` inside `usePdfRenderer.svelte.ts::init()`, which is called from `onMount`. These dynamic imports triggered the race.
---
## Decision
**Prefer prop injection over `vi.mock(module, factory)` for any module that is loaded dynamically in browser-mode specs.**
### The libLoader pattern (for external rendering libraries)
When a component depends on a large external library loaded via dynamic import, extract the import into an injectable loader function with a production default:
```typescript
// usePdfRenderer.svelte.ts
type LibLoader = () => Promise<readonly [typeof import('pdfjs-dist'), { default: string }]>;
const defaultLibLoader: LibLoader = () =>
Promise.all([import('pdfjs-dist'), import('pdfjs-dist/build/pdf.worker.min.mjs?url')]);
export function createPdfRenderer(libLoader: LibLoader = defaultLibLoader) { ... }
```
The component threads the loader as an optional prop:
```svelte
<!-- PdfViewer.svelte -->
let { url, ..., libLoader = undefined } = $props();
const renderer = untrack(() => createPdfRenderer(libLoader));
```
Tests supply a synchronous fake — no `vi.mock` needed:
```typescript
const fakePdfjs = { GlobalWorkerOptions: ..., getDocument: vi.fn(), TextLayer: class {} };
const fakeLoader = vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const);
render(PdfViewer, { url: '...', libLoader: fakeLoader });
```
### The test-host pattern (for component behaviour)
For components that fetch data or call services, the `*.test-host.svelte` pattern threads the dependency as a prop rather than mocking the module. See `PersonMentionEditor.test-host.svelte` for the canonical example.
---
## Residual exceptions
The following `vi.mock(module, factory)` calls in browser specs are **acceptable** because the mocked modules are loaded statically at spec module-eval time and cannot produce a teardown race:
| Module | Why it stays as vi.mock |
|--------|------------------------|
| `$app/navigation` | SvelteKit virtual module — no DI seam |
| `$app/forms` | SvelteKit virtual module — no DI seam |
| `$app/state` | SvelteKit virtual module — no DI seam |
| `$app/stores` | SvelteKit virtual module — no DI seam |
| `$env/static/public` | Vite env virtual module — no DI seam |
These modules are resolved at static import time (before any test runs). Their `vi.mock` factories are served by birpc synchronously during module graph resolution, not after worker teardown.
**Pattern note:** When an overlay or dropdown triggers a navigation action, use `<button type="button">` with an `onclick` handler that calls `goto(path)` — do **not** use `<a href="…">` with `e.preventDefault()`. SvelteKit registers its link interceptor as a capture-phase `document` listener, so it fires before the component's bubble-phase `onclick`. By the time `e.preventDefault()` runs the router has already initiated navigation, which tears down the vitest-browser Playwright orchestrator iframe. A `<button>` carries no `href`, so the capture-phase interceptor never fires. See `NotificationDropdown.svelte` for the canonical example.
---
## Consequences
- New browser-mode specs that need to stub an external library **must not** use `vi.mock(externalLib, factory)`. Add a loader/factory parameter to the underlying hook or service instead.
- The CI `unit-tests` job includes a permanent grep guard that fails the build if `rpc is closed` appears in any coverage run log. This catches regressions before they reach the acceptance criterion.
- Acceptance criterion for #535: 60 consecutive green `workflow_dispatch` CI runs against `main` after the fix is merged, with zero `rpc is closed` lines in any log.
- **Enforcement:** An ESLint `no-restricted-syntax` rule in `eslint.config.js` (scoped to `**/*.spec.ts` and `**/*.test.ts`) flags any `vi.mock('pdfjs-dist', ...)` call with a message referencing this ADR. This turns a ~2-minute CI wait into an immediate lint error on save. The CI birpc grep guard remains as belt-and-suspenders for the coverage run. A CI static grep step (added in issue #546) also catches the pattern before the test suite launches.
- **When to revisit the LibLoader home:** If three or more components adopt this pattern, consider extracting a shared `$lib/types/lib-loader.ts` or a generic `DynamicImportLoader<T>` type to avoid parallel type definitions across modules.