Files
familienarchiv/docs/adr/012-browser-test-mocking-strategy.md
Marcel addf5c98db
Some checks failed
CI / Unit & Component Tests (push) Failing after 1m49s
CI / OCR Service Tests (push) Successful in 17s
CI / Backend Unit Tests (push) Successful in 4m13s
CI / Compose Bucket Idempotency (push) Has been cancelled
CI / fail2ban Regex (push) Has been cancelled
docs(adr-012): record the sync-factory invariant and the $app/state migration
The previous revision allowed vi.mock for virtual modules on the "consumer
import is static" argument. #553 proved that argument wrong: a statically-
imported module with an async factory body whose dynamic import landed
after teardown still produced the race. The factory body — not the
consumer — is the failure surface.

- Drop the "residual exceptions" table.
- Add the binding invariant: factory bodies under `**/*.svelte.{test,spec}.ts`
  must be synchronous (no `await`, no `import(...)`).
- Document the canonical vi.hoisted + getter pattern, with file references.
- Record the $app/stores → $app/state architectural call (Markus's
  recommendation), removing one of the last two deprecated-import
  outliers.
- Record the preload-data=off hardening (Tobias's recommendation) as a
  pattern note.
- Update the Enforcement section to list all four defence layers (ESLint,
  CI grep, in-suite meta-test, CI birpc assert) and the coverage-flake-
  probe verification workflow.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
2026-05-12 22:14:31 +02:00

8.5 KiB
Raw Blame History

ADR 012 — Browser-Mode Test Mocking Strategy

Status: Accepted
Date: 2026-05-11 (revised 2026-05-12)
Issues: #535 — original incident · #553 — revision


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:

// 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:

<!-- PdfViewer.svelte -->
let { url, ..., libLoader = undefined } = $props();
const renderer = untrack(() => createPdfRenderer(libLoader));

Tests supply a synchronous fake — no vi.mock needed:

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.


Binding invariant: factory bodies must be synchronous (#553)

The original revision of this ADR allowed vi.mock(virtualModule, factory) for SvelteKit/Vite virtual modules on the argument that their consumer imports were resolved at static-import time. That reasoning is wrong. What matters is what the factory body does, not where the mocked module is consumed.

EnrichmentBlock.svelte.spec.ts (issue #553) was statically imported and still produced the race: its vi.mock('$app/stores', async () => { const mod = await import(...); return mod; }) factory performed a dynamic import in its body, and that body was invoked asynchronously when Chromium fetched the manually-mocked module — sometimes after the worker's birpc channel had already closed.

Therefore: under **/*.svelte.{test,spec}.ts, every vi.mock factory body must be synchronous. No await, no import(...).

If a factory needs to share state with the spec (a mutable ref, a vi.fn, a writable store), use vi.hoisted() to lift the reference above vi.mock's implicit hoist:

const { mockNavigating } = vi.hoisted(() => ({
    mockNavigating: { type: null as string | null }
}));

vi.mock('$app/state', () => ({
    get navigating() {
        return mockNavigating;
    }
}));

The getter defers the read until consumption time; vi.hoisted guarantees the reference is initialised before the (also hoisted) vi.mock factory runs. See DropZone.svelte.spec.ts:9, NotificationBell.svelte.spec.ts:6-10, and EnrichmentBlock.svelte.spec.ts for canonical examples.

Architectural follow-on: prefer $app/state over $app/stores

$app/stores is the deprecated subscription-based store API; $app/state is the modern reactive proxy. New components should import from $app/state. As part of #553 we migrated EnrichmentBlock.svelte from $app/stores.navigating to $app/state.navigating with !!navigating.type — matching the pattern already established in routes/aktivitaeten/+page.svelte:117 and routes/documents/+page.svelte:261. Migration eliminated the need to mock a store at all in that spec.

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.

Pattern note (#553): Browser-mode tests run with data-sveltekit-preload-data="off" (set in src/test-setup.ts via the client project's setupFiles). Hover-prefetch otherwise fires real fetch requests for route loader chunks; those requests go through the same Playwright route handler that serves mocked modules. An in-flight prefetch landing after iframe teardown can hit the handler with a closed birpc channel, raising an unhandled rejection.


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 (four layers, defence in depth):
    1. ESLint no-restricted-syntax in eslint.config.js (scoped to **/*.{spec,test}.ts) flags two patterns: (a) the literal vi.mock('pdfjs-dist', ...) — enforces the libLoader pattern — and (b) any vi.mock(..., async () => { ... await import(...) ... }) — enforces the synchronous-factory invariant. Both messages point at this ADR. Failure surfaces at save time.
    2. CI grep guard in .gitea/workflows/ci.yml runs before the test suite launches. Mirrors the ESLint patterns with grep -Pzn. ~10s round-trip.
    3. In-suite meta-test at frontend/src/__meta__/no-async-mock-factories.test.ts globs src/**/*.svelte.{test,spec}.ts and asserts none match the banned pattern. Catches at every vitest invocation — the layer hardest to disable.
    4. CI birpc assert runs after the coverage step and fails the build if [birpc] rpc is closed appears in any log line. Catches the symptom even if all the upstream layers were bypassed.
  • Acceptance verification: coverage-flake-probe.yml is a workflow_dispatch-triggered matrix workflow that runs the coverage suite 20× in parallel against a single SHA and asserts zero birpc lines. One fire, parallel cost, deterministic signal — replaces accumulating 20 sequential push events.
  • 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.