From 9f1b8b42151ff576a2639a60b7de2174dfa38dab Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:00:44 +0200 Subject: [PATCH 01/13] =?UTF-8?q?fix(enrichment-block):=20migrate=20$app/s?= =?UTF-8?q?tores=20=E2=86=92=20$app/state=20to=20eliminate=20birpc=20race?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The async vi.mock factory in EnrichmentBlock.svelte.spec.ts performed an `await import(...)` in its body — the same mechanism #535/#546 fixed for pdfjs-dist. Issue #553: when Chromium's playwright route handler fetches the mocked module after the worker's birpc channel has closed, the factory's RPC roundtrip raises `[birpc] rpc is closed, cannot call "resolveManualMock"` and the run exits 1. Migrate EnrichmentBlock from the deprecated `$app/stores.navigating` (store) to the modern `$app/state.navigating` (reactive proxy). The spec uses vi.hoisted + a sync vi.mock factory with a getter that defers the read — no dynamic import in the factory body. Delete the now-unused __mocks__/navigatingStore.ts. Fix path applied: $app/state migration (Markus's recommendation / Felix's Path 2). See ADR-012. Refs #553 Co-Authored-By: Claude Opus 4.7 (1M context) --- .../src/lib/document/EnrichmentBlock.svelte | 4 +-- .../document/EnrichmentBlock.svelte.spec.ts | 28 +++++++++++-------- .../lib/document/__mocks__/navigatingStore.ts | 3 -- 3 files changed, 18 insertions(+), 17 deletions(-) delete mode 100644 frontend/src/lib/document/__mocks__/navigatingStore.ts diff --git a/frontend/src/lib/document/EnrichmentBlock.svelte b/frontend/src/lib/document/EnrichmentBlock.svelte index a4327cf3..77a9e96a 100644 --- a/frontend/src/lib/document/EnrichmentBlock.svelte +++ b/frontend/src/lib/document/EnrichmentBlock.svelte @@ -1,5 +1,5 @@ diff --git a/frontend/src/lib/document/EnrichmentBlock.svelte.spec.ts b/frontend/src/lib/document/EnrichmentBlock.svelte.spec.ts index ac5167d1..35039cb2 100644 --- a/frontend/src/lib/document/EnrichmentBlock.svelte.spec.ts +++ b/frontend/src/lib/document/EnrichmentBlock.svelte.spec.ts @@ -2,19 +2,23 @@ import { describe, it, expect, afterEach, vi } from 'vitest'; import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; -// The store must live in a separate module because vi.mock factories are -// hoisted and cannot reference top-level variables defined in this file. -import { navigatingStore } from './__mocks__/navigatingStore'; import EnrichmentBlock from './EnrichmentBlock.svelte'; -vi.mock('$app/stores', async () => { - const mod = await import('./__mocks__/navigatingStore'); - return { navigating: mod.navigatingStore }; -}); +// Hoist the mutable navigation reference so vi.mock's factory (also hoisted) +// can read it via a getter. Sync factory, no dynamic import: ADR-012 invariant. +const { mockNavigating } = vi.hoisted(() => ({ + mockNavigating: { type: null as string | null } +})); + +vi.mock('$app/state', () => ({ + get navigating() { + return mockNavigating; + } +})); afterEach(() => { cleanup(); - navigatingStore.set(null); + mockNavigating.type = null; }); type Doc = { id: string; title: string; uploadedAt: string }; @@ -65,8 +69,8 @@ describe('EnrichmentBlock', () => { await expect.element(page.getByTestId('dashboard-needs-metadata')).toBeInTheDocument(); }); - it('renders the skeleton when $navigating is active and topDocs is empty', async () => { - navigatingStore.set({ type: 'link' }); + it('renders the skeleton when navigation is active and topDocs is empty', async () => { + mockNavigating.type = 'link'; render(EnrichmentBlock, { topDocs: [], totalCount: 0, @@ -76,8 +80,8 @@ describe('EnrichmentBlock', () => { await expect.element(page.getByTestId('enrichment-block-skeleton')).toBeInTheDocument(); }); - it('does not render the skeleton when topDocs is non-empty even during $navigating', async () => { - navigatingStore.set({ type: 'link' }); + it('does not render the skeleton when topDocs is non-empty even during navigation', async () => { + mockNavigating.type = 'link'; render(EnrichmentBlock, { topDocs: [doc('d1')], totalCount: 1, diff --git a/frontend/src/lib/document/__mocks__/navigatingStore.ts b/frontend/src/lib/document/__mocks__/navigatingStore.ts deleted file mode 100644 index 932122e4..00000000 --- a/frontend/src/lib/document/__mocks__/navigatingStore.ts +++ /dev/null @@ -1,3 +0,0 @@ -import { writable } from 'svelte/store'; - -export const navigatingStore = writable(null); -- 2.49.1 From 3c9e40ca718efd11ba5e7d15817e5c471982c7e9 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:02:57 +0200 Subject: [PATCH 02/13] test(setup): disable SvelteKit hover prefetch in browser-mode runs Hover-prefetch 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 that exits the run with code 1 even when every individual test was green. Add `src/test-setup.ts` that sets `document.body.dataset.sveltekitPreloadData = 'off'` and wire it via `setupFiles` in both `vite.config.ts` (client project) and `vitest.client-coverage.config.ts` (Istanbul coverage config). Add `src/__meta__/browser-preload-disabled.svelte.test.ts` asserting the setup ran. Zero production impact. Issue #553 secondary trigger. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../browser-preload-disabled.svelte.test.ts | 16 ++++++++++++++++ frontend/src/test-setup.ts | 8 ++++++++ frontend/vite.config.ts | 1 + frontend/vitest.client-coverage.config.ts | 1 + 4 files changed, 26 insertions(+) create mode 100644 frontend/src/__meta__/browser-preload-disabled.svelte.test.ts create mode 100644 frontend/src/test-setup.ts diff --git a/frontend/src/__meta__/browser-preload-disabled.svelte.test.ts b/frontend/src/__meta__/browser-preload-disabled.svelte.test.ts new file mode 100644 index 00000000..60e79b2a --- /dev/null +++ b/frontend/src/__meta__/browser-preload-disabled.svelte.test.ts @@ -0,0 +1,16 @@ +import { describe, it, expect } from 'vitest'; + +// Browser-mode tests must run with SvelteKit's hover-prefetch disabled. +// Hover-prefetch fires real `fetch` requests for the target route's loader +// chunks; those go through the same Playwright route handler that serves +// mocked modules. Even after `cleanup()` tears down the iframe, an in-flight +// prefetch can still hit the handler — and if the worker's birpc channel has +// closed by then, the handler raises an unhandled rejection. ADR-012 / #553. +// +// This test enforces that the test-setup file ran and switched preload-data +// off on `document.body` before any spec started rendering. +describe('browser test setup', () => { + it('disables SvelteKit hover prefetch on document.body', () => { + expect(document.body.dataset.sveltekitPreloadData).toBe('off'); + }); +}); diff --git a/frontend/src/test-setup.ts b/frontend/src/test-setup.ts new file mode 100644 index 00000000..b7317f9c --- /dev/null +++ b/frontend/src/test-setup.ts @@ -0,0 +1,8 @@ +// Disable SvelteKit hover-prefetch in browser-mode tests. ADR-012 / #553. +// +// Hover-prefetch 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 +// route handler with a closed birpc channel, raising an unhandled rejection +// that exits the run with code 1 even when every individual test was green. +document.body.dataset.sveltekitPreloadData = 'off'; diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index ebf88051..cdde2860 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -77,6 +77,7 @@ export default defineConfig({ screenshotDirectory: 'test-results/screenshots', screenshotFailures: true }, + setupFiles: ['./src/test-setup.ts'], include: ['src/**/*.svelte.{test,spec}.{js,ts}'], exclude: ['src/lib/server/**'] } diff --git a/frontend/vitest.client-coverage.config.ts b/frontend/vitest.client-coverage.config.ts index c1435ceb..d6ada2f5 100644 --- a/frontend/vitest.client-coverage.config.ts +++ b/frontend/vitest.client-coverage.config.ts @@ -32,6 +32,7 @@ export default defineConfig({ screenshotDirectory: 'test-results/screenshots', screenshotFailures: true }, + setupFiles: ['./src/test-setup.ts'], include: ['src/**/*.svelte.{test,spec}.{js,ts}'], exclude: ['src/lib/server/**'], coverage: { -- 2.49.1 From 636d61a81b00f7de307967c09539920d0dc5a4d1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:05:43 +0200 Subject: [PATCH 03/13] test(meta): scan src/**/*.svelte.{test,spec}.ts for async vi.mock factories MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In-suite belt-and-braces detector for the birpc teardown race named in ADR-012 / #553. Catches `vi.mock(, async ... { ... await import(...) ... })` in any browser spec on every vitest invocation — the layer hardest to disable or scope around (ESLint can be silenced; CI grep runs only in CI; this test runs whenever the suite runs). Demonstrated red→green by planting a synthetic offender locally and watching the live-scan assertion fail; removing the offender returned it to green. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../__meta__/no-async-mock-factories.test.ts | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) create mode 100644 frontend/src/__meta__/no-async-mock-factories.test.ts diff --git a/frontend/src/__meta__/no-async-mock-factories.test.ts b/frontend/src/__meta__/no-async-mock-factories.test.ts new file mode 100644 index 00000000..ef3622ea --- /dev/null +++ b/frontend/src/__meta__/no-async-mock-factories.test.ts @@ -0,0 +1,82 @@ +import { describe, it, expect } from 'vitest'; +import { readdirSync, readFileSync } from 'fs'; +import path from 'path'; +import { fileURLToPath } from 'url'; + +// Belt-and-braces detector for the birpc teardown race named in ADR-012 / #553. +// ESLint catches the pattern at save time, CI grep catches it before the test +// suite launches, and this in-suite test catches it at every vitest invocation — +// the layer hardest to disable or scope around. +// +// We scan source text rather than parsing AST: fast, no parser dependency, +// good enough for the named anti-pattern. The pattern matches +// `vi.mock(, async ... { ... await import(...) ... })`. + +const ASYNC_MOCK_WITH_DYNAMIC_IMPORT = /vi\.mock\([^)]*,\s*async[^{]*\{[\s\S]*?await\s+import\s*\(/; + +export function hasAsyncMockFactoryWithDynamicImport(source: string): boolean { + return ASYNC_MOCK_WITH_DYNAMIC_IMPORT.test(source); +} + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const SRC_ROOT = path.resolve(__dirname, '..'); + +function findBrowserSpecs(): string[] { + const entries = readdirSync(SRC_ROOT, { recursive: true, withFileTypes: true }); + return entries + .filter( + (e) => + e.isFile() && (e.name.endsWith('.svelte.test.ts') || e.name.endsWith('.svelte.spec.ts')) + ) + .map((e) => path.join(e.parentPath ?? (e as { path: string }).path, e.name)); +} + +describe('scan: hasAsyncMockFactoryWithDynamicImport', () => { + it('flags async vi.mock factory with await import in body', () => { + const fixture = `vi.mock('$app/stores', async () => { + const mod = await import('./__mocks__/navigatingStore'); + return { navigating: mod.navigatingStore }; + });`; + expect(hasAsyncMockFactoryWithDynamicImport(fixture)).toBe(true); + }); + + it('does not flag sync vi.mock factory', () => { + const fixture = `vi.mock('$app/state', () => ({ navigating: { type: null } }));`; + expect(hasAsyncMockFactoryWithDynamicImport(fixture)).toBe(false); + }); + + it('does not flag async vi.mock factory without dynamic import', () => { + const fixture = `vi.mock('foo', async () => { + const x = await Promise.resolve(42); + return { bar: x }; + });`; + expect(hasAsyncMockFactoryWithDynamicImport(fixture)).toBe(false); + }); + + it('does not flag dynamic import outside any vi.mock', () => { + const fixture = `async function load() { + const mod = await import('./something'); + return mod.default; + }`; + expect(hasAsyncMockFactoryWithDynamicImport(fixture)).toBe(false); + }); + + it('flags async factory written as async function expression', () => { + const fixture = `vi.mock('foo', async function () { + const mod = await import('./bar'); + return mod; + });`; + expect(hasAsyncMockFactoryWithDynamicImport(fixture)).toBe(true); + }); +}); + +describe('browser specs: no async vi.mock factory contains await import', () => { + it('every src/**/*.svelte.{test,spec}.ts file is clean', () => { + const specFiles = findBrowserSpecs(); + expect(specFiles.length).toBeGreaterThan(0); + const offenders = specFiles.filter((file) => + hasAsyncMockFactoryWithDynamicImport(readFileSync(file, 'utf-8')) + ); + expect(offenders).toEqual([]); + }); +}); -- 2.49.1 From 5afebde3827f0de2b7f210bb69e17033ff699d8b Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:09:07 +0200 Subject: [PATCH 04/13] refactor(eslint): ban async vi.mock factories with dynamic import in body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Generalise the no-restricted-syntax rule from the literal pdfjs-dist selector (added in #535) to also catch the underlying mechanism named in ADR-012 / #553: any `vi.mock(..., async () => { ... await import(...) ... })` produces a late birpc roundtrip during worker teardown. Selector: vi.mock CallExpression whose second argument is an ArrowFunctionExpression with async=true and whose subtree contains an AwaitExpression > ImportExpression. Both rules coexist — the literal pdfjs-dist rule still enforces the libLoader prop injection pattern (catches sync forms too); the new rule enforces the sync-factory invariant universally. Demonstrated by planting a synthetic offender locally and watching ESLint flag it with the new rule's message. Co-Authored-By: Claude Opus 4.7 (1M context) --- frontend/eslint.config.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/frontend/eslint.config.js b/frontend/eslint.config.js index a56c710b..88aa8a53 100644 --- a/frontend/eslint.config.js +++ b/frontend/eslint.config.js @@ -82,6 +82,17 @@ export default defineConfig( "CallExpression[callee.object.name='vi'][callee.property.name='mock'] > Literal[value=/^pdfjs-dist/]", message: "Banned: vi.mock('pdfjs-dist', factory) causes a birpc teardown race in browser-mode specs — see ADR 012. Use the libLoader prop injection pattern instead." + }, + { + // ADR 012 / #553. The named mechanism: an async vi.mock factory whose + // body performs `await import(...)` produces a late birpc roundtrip + // during worker teardown. The factory body must be synchronous; if + // you need to share state between the spec and the mock, use + // `vi.hoisted` (see DropZone.svelte.spec.ts). + selector: + "CallExpression[callee.object.name='vi'][callee.property.name='mock'][arguments.1.type='ArrowFunctionExpression'][arguments.1.async=true]:has(AwaitExpression > ImportExpression)", + message: + 'Banned: vi.mock(..., async () => { await import(...) }) causes a birpc teardown race in browser-mode specs — see ADR 012. Use a synchronous factory + vi.hoisted instead.' } ] } -- 2.49.1 From 67cd56acc73c137a6cdabcd25cb54a8e32c7de61 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:11:09 +0200 Subject: [PATCH 05/13] ci(unit-tests): extend grep guard to async vi.mock with dynamic import The pdfjs-dist literal grep added in 9260866f only caught one named trigger of the birpc teardown race; the underlying mechanism (ADR 012 / #553) is any async vi.mock factory whose body performs `await import(...)`. Add a second PCRE-multiline grep matching that shape. Scoped to **/*.{spec,test}.ts under frontend/src/, excluding __meta__ (which holds the fixture strings exercising the meta-test). Defence in depth pairs with the ESLint rule (saves at edit time) and the in-suite meta-test (catches when tests run). Verified locally with real GNU grep against a planted synthetic offender. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitea/workflows/ci.yml | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml index aa65e565..5aa5d748 100644 --- a/.gitea/workflows/ci.yml +++ b/.gitea/workflows/ci.yml @@ -39,10 +39,22 @@ jobs: - name: Assert no banned vi.mock patterns shell: bash run: | + # Literal pdfjs-dist (libLoader pattern — ADR 012) if grep -rF "vi.mock('pdfjs-dist'" frontend/src/; then echo "FAIL: banned vi.mock('pdfjs-dist') pattern found — see ADR 012. Use the libLoader prop injection pattern instead." exit 1 fi + # Async factory with dynamic import in body (named mechanism — ADR 012 / #553). + # Multiline PCRE matches `vi.mock(, async ... { ... await import(...) ... })` + # across line breaks. __meta__ is excluded because it contains fixture strings + # demonstrating the very pattern this check is meant to forbid. + if grep -rPzln 'vi\.mock\([^)]+,\s*async[^{]*\{[\s\S]*?await\s+import\s*\(' \ + --include='*.spec.ts' --include='*.test.ts' \ + --exclude-dir='__meta__' \ + frontend/src/; then + echo "FAIL: banned async vi.mock factory with dynamic import in body — see ADR 012 / #553. Use a synchronous factory + vi.hoisted instead." + exit 1 + fi - name: Run unit and component tests with coverage shell: bash -- 2.49.1 From c82088476534e54e1771be7d165829653c8b654e Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:12:04 +0200 Subject: [PATCH 06/13] ci(coverage-flake-probe): add workflow_dispatch matrix job (20 parallel runs) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Verification mechanism for the 20-run acceptance criterion of issue #553. Triggered manually via workflow_dispatch, runs the full coverage suite 20× in parallel against a single SHA, asserts zero `[birpc] rpc is closed` lines in every cell. One fire, parallel cost (~one main-job's wall-clock), deterministic signal for the teardown race. Cheaper than 20 sequential push events and tests the same property the AC names. Closes the verification gap raised by Tobias and Elicit in the issue discussion. Co-Authored-By: Claude Opus 4.7 (1M context) --- .gitea/workflows/coverage-flake-probe.yml | 64 +++++++++++++++++++++++ 1 file changed, 64 insertions(+) create mode 100644 .gitea/workflows/coverage-flake-probe.yml diff --git a/.gitea/workflows/coverage-flake-probe.yml b/.gitea/workflows/coverage-flake-probe.yml new file mode 100644 index 00000000..0258ba7a --- /dev/null +++ b/.gitea/workflows/coverage-flake-probe.yml @@ -0,0 +1,64 @@ +name: Coverage Flake Probe + +# Manually-triggered probe for the birpc teardown race documented in ADR 012 +# / #553. Runs the full coverage suite 20× in parallel against a single SHA +# and asserts zero `[birpc] rpc is closed` lines across every cell. Verifies +# the acceptance criterion that the race no longer surfaces under coverage. + +on: + workflow_dispatch: + +jobs: + coverage-flake-probe: + name: Coverage flake probe (run ${{ matrix.run }}) + runs-on: ubuntu-latest + container: + image: mcr.microsoft.com/playwright:v1.58.2-noble + strategy: + fail-fast: false + matrix: + run: [1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16, 17, 18, 19, 20] + steps: + - uses: actions/checkout@v4 + + - name: Cache node_modules + id: node-modules-cache + uses: actions/cache@v4 + with: + path: frontend/node_modules + key: node-modules-${{ hashFiles('frontend/package-lock.json') }} + + - name: Install dependencies + if: steps.node-modules-cache.outputs.cache-hit != 'true' + run: npm ci + working-directory: frontend + + - name: Compile Paraglide i18n + run: npx @inlang/paraglide-js compile --project ./project.inlang --outdir ./src/lib/paraglide + working-directory: frontend + + - name: Run unit and component tests with coverage + shell: bash + run: | + set -eo pipefail + npm run test:coverage 2>&1 | tee /tmp/coverage-test-${{ github.run_id }}-${{ matrix.run }}.log + working-directory: frontend + env: + TZ: Europe/Berlin + + - name: Assert no birpc teardown race + shell: bash + if: always() + run: | + if grep -qF "[birpc] rpc is closed" /tmp/coverage-test-${{ github.run_id }}-${{ matrix.run }}.log 2>/dev/null; then + echo "FAIL: [birpc] rpc is closed teardown race detected in run ${{ matrix.run }}" + grep -F "[birpc] rpc is closed" /tmp/coverage-test-${{ github.run_id }}-${{ matrix.run }}.log + exit 1 + fi + + - name: Upload coverage log on failure + if: failure() + uses: actions/upload-artifact@v4 + with: + name: coverage-log-run-${{ matrix.run }} + path: /tmp/coverage-test-${{ github.run_id }}-${{ matrix.run }}.log -- 2.49.1 From addf5c98db06008401751f427a4a52371a8c3ba3 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 22:14:31 +0200 Subject: [PATCH 07/13] 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 `