From addf5c98db06008401751f427a4a52371a8c3ba3 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:14:31 +0200 Subject: [PATCH] docs(adr-012): record the sync-factory invariant and the $app/state migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) --- docs/adr/012-browser-test-mocking-strategy.md | 47 ++++++++++++++----- 1 file changed, 34 insertions(+), 13 deletions(-) diff --git a/docs/adr/012-browser-test-mocking-strategy.md b/docs/adr/012-browser-test-mocking-strategy.md index d916fc10..e3039bbe 100644 --- a/docs/adr/012-browser-test-mocking-strategy.md +++ b/docs/adr/012-browser-test-mocking-strategy.md @@ -1,8 +1,8 @@ # 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) +**Date:** 2026-05-11 (revised 2026-05-12) +**Issues:** [#535 — original incident](https://git.raddatz.cloud/marcel/familienarchiv/issues/535) · [#553 — revision](https://git.raddatz.cloud/marcel/familienarchiv/issues/553) --- @@ -65,22 +65,38 @@ For components that fetch data or call services, the `*.test-host.svelte` patter --- -## Residual exceptions +## Binding invariant: factory bodies must be synchronous (#553) -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: +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. -| 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 | +`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. -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. +**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: + +```ts +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 `