WIP: test(mocks): migrate vi.mock factories to __mocks__ + test-fixtures (#560) #657

marcel · 2026-05-21T18:20:56+02:00

marcel commented

2026-05-21 18:20:56 +02:00

Summary

Implements issue #560 in incremental commits. Long-running migration; opened as draft (WIP:) for CI gating.

First commit is the load-bearing experiment: adds __mocks__/$app/navigation.ts and migrates a single consumer (DocumentRow.svelte.spec.ts) from a factory mock to vi.mock('$app/navigation') (no factory). If CI is green, the same redirect pattern will be applied across the remaining 35 $app/navigation factories, then to $app/state, $app/forms, and the paraglide modules. Subsequent commits will follow the phase plan agreed in the implementation plan.

Personas have already reviewed #560; key decisions captured:

Test-fixture naming follows existing *.test-fixture.svelte convention
Notification fixture uses option (b): setNotifications(items) setter via onReady
Notification store will be refactored to context-provided in Phase 2
@vitest/browser-playwright exact-pin is out of scope (separate devops issue to file)

Plan

Phase 1 (__mocks__/ redirects): $app/navigation → $app/state → $env/static/public → $lib/paraglide/* → $app/stores page migration + spec cleanup → $app/forms (with shared form-interceptor in the mock to cover the 4 complex consumers Felix flagged).
Phase 2 (test-fixtures): 8 specs to confirm.test-fixture, then notification store context refactor + new notification.test-fixture.svelte covering the 2 remaining specs.
Phase 3: 5 component-stub factories left as class (c) per ADR-012.

Refs ADR-012. Closes #560 (on green).

Test plan

CI green on this first experimental commit (proves __mocks__/ resolution works for SvelteKit virtual modules)
CI green after every phase commit
Meta-tests (no-async-mock-factories, no-duplicate-mock-ids) green throughout
Zero [birpc] rpc is closed lines in CI coverage logs
Manual coverage-flake-probe.yml trigger before un-WIP'ing this PR

🤖 Generated with Claude Code

## Summary Implements issue #560 in incremental commits. Long-running migration; opened as draft (WIP:) for CI gating. **First commit is the load-bearing experiment:** adds `__mocks__/$app/navigation.ts` and migrates a single consumer (`DocumentRow.svelte.spec.ts`) from a factory mock to `vi.mock('$app/navigation')` (no factory). If CI is green, the same redirect pattern will be applied across the remaining 35 `$app/navigation` factories, then to `$app/state`, `$app/forms`, and the paraglide modules. Subsequent commits will follow the phase plan agreed in the implementation plan. Personas have already reviewed #560; key decisions captured: - Test-fixture naming follows existing `*.test-fixture.svelte` convention - Notification fixture uses option (b): `setNotifications(items)` setter via `onReady` - Notification store will be refactored to context-provided in Phase 2 - `@vitest/browser-playwright` exact-pin is **out of scope** (separate devops issue to file) ## Plan 1. Phase 1 (`__mocks__/` redirects): `$app/navigation` → `$app/state` → `$env/static/public` → `$lib/paraglide/*` → `$app/stores` page migration + spec cleanup → `$app/forms` (with shared form-interceptor in the mock to cover the 4 complex consumers Felix flagged). 2. Phase 2 (test-fixtures): 8 specs to `confirm.test-fixture`, then notification store context refactor + new `notification.test-fixture.svelte` covering the 2 remaining specs. 3. Phase 3: 5 component-stub factories left as class (c) per ADR-012. Refs ADR-012. Closes #560 (on green). ## Test plan - [ ] CI green on this first experimental commit (proves `__mocks__/` resolution works for SvelteKit virtual modules) - [ ] CI green after every phase commit - [ ] Meta-tests (`no-async-mock-factories`, `no-duplicate-mock-ids`) green throughout - [ ] Zero `[birpc] rpc is closed` lines in CI coverage logs - [ ] Manual `coverage-flake-probe.yml` trigger before un-WIP'ing this PR 🤖 Generated with [Claude Code](https://claude.com/claude-code)

marcel added 1 commit 2026-05-21 18:20:56 +02:00

test(mocks): wire DocumentRow spec to shared __mocks__/$app/navigation

CI / Unit & Component Tests (pull_request) Successful in 3m33s

Details

CI / OCR Service Tests (pull_request) Successful in 20s

Details

CI / Backend Unit Tests (pull_request) Successful in 3m35s

Details

CI / fail2ban Regex (pull_request) Successful in 43s

Details

CI / Semgrep Security Scan (pull_request) Successful in 20s

Details

CI / Compose Bucket Idempotency (pull_request) Successful in 1m0s

Details

aea37250f4

Smallest possible first step of issue #560. Adds a shared
__mocks__/$app/navigation.ts exporting every nav function as vi.fn(),
and switches one consumer (DocumentRow.svelte.spec.ts) from a per-spec
factory to vi.mock('\$app/navigation') (no factory) to verify Vitest's
__mocks__/ redirect resolves for SvelteKit virtual modules in
browser-mode tests.

CI is the gate: if the migrated spec passes alongside the 35 unchanged
factory call sites, the same redirect pattern is applied across the
remaining $app/navigation, $app/state, $app/forms and paraglide
factories.

Per ADR-012, eliminating factory mocks shrinks the birpc teardown-race
surface; this commit is the first concrete cut.

Refs #560.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel added 1 commit 2026-05-21 18:43:14 +02:00

test(mocks): switch $app/navigation factory mocks to shared __mocks__ stub

CI / Unit & Component Tests (pull_request) Failing after 2m51s

Details

CI / OCR Service Tests (pull_request) Successful in 21s

Details

CI / Backend Unit Tests (pull_request) Successful in 3m43s

Details

CI / fail2ban Regex (pull_request) Failing after 42s

Details

CI / Semgrep Security Scan (pull_request) Successful in 20s

Details

CI / Compose Bucket Idempotency (pull_request) Successful in 1m1s

Details

9fcd0553bd

Migrates 36 of the remaining 37 vi.mock('$app/navigation', factory) call
sites in frontend/src/**/*.svelte.{spec,test}.ts to
vi.mock('$app/navigation') (no factory), so they all resolve to the
shared src/__mocks__/$app/navigation.ts stub introduced in commit
aea37250.

Special handling:
- DropZone.svelte.spec.ts: removed vi.hoisted invalidateAllMock; test
  body now imports invalidateAll from $app/navigation and uses
  vi.mocked(invalidateAll) for assertions.
- documents/bulk-edit/page.svelte.test.ts: removed the module-scope
  const gotoSpy = vi.fn() that was injected into the factory; test
  body now imports goto from $app/navigation and uses
  vi.mocked(goto) for assertions and mockClear().

Deferred: src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts uses a
non-trivial beforeNavigate wrapper that captures the callback into
module-scope state (registeredBeforeNavigate) so tests can simulate
navigation events. That hybrid factory cannot be replaced with the
plain-vi.fn() shared stub without redesigning the test; it is left in
place and will be addressed separately.

Per ADR-012: eliminating factory bodies removes 36 birpc teardown-race
surface points. The shared mock keeps every nav export as a vi.fn().

Refs #560.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

marcel commented

2026-05-21 19:15:07 +02:00

Closing without merge — empirical finding invalidates the load-bearing premise

CI run #1857 (commit 9fcd0553) failed with 2 of 274 tests:

FAIL  chromium  src/routes/admin/tags/[id]/page.svelte.spec.ts > Admin edit tag page – merge success banner > shows merge success banner when data.mergeSuccess is true
FAIL  chromium  src/routes/admin/tags/[id]/page.svelte.test.ts  > admin/tags/[id] page > shows the merge-success banner when data.mergeSuccess is set

Error: Cannot call replaceState(...) before router is initialized
  ❯ replaceState node_modules/@sveltejs/kit/src/runtime/client/client.js:2483:9
  ❯ $effect src/routes/admin/tags/[id]/+page.svelte:48:1

What this means

vi.mock('$app/navigation') without a factory does NOT fully replace the SvelteKit virtual module via the shared src/__mocks__/$app/navigation.ts. Vitest's auto-mock substitutes some exports (e.g. goto becomes a vi.fn() — DocumentRow's tests verified that in run #1856) but leaves others bound to the real implementation (e.g. replaceState here). The result is partial mocking: code paths that exercise the unmocked exports hit the real router and fail.

The personas (Felix, Markus, Sara, Nora, Elicit, Leonie, Tobias) and the issue body all assumed full auto-resolve via __mocks__/. None of the reviews verified this empirically. The first commit on this PR (run #1856) only appeared to validate the approach because DocumentRow.svelte.spec.ts happens not to exercise the partially-mocked exports.

Pivot options surfaced by the failure

Captured in follow-up discussion issue (to be filed). Three alternatives:

Shared-mock + sync factory. Keep src/__mocks__/$app/navigation.ts, but each spec uses import * as navMock from '$mocks/...'; vi.mock('$app/navigation', () => ({ ...navMock }));. Two lines per spec, consolidates the mock body, works because the factory is explicit. Sync factories are not banned by the meta-test (only async + dynamic import is).
Drop the __mocks__/ plan; keep status quo of per-spec factories. The original goal in ADR-012 — preventing the birpc teardown race from async + dynamic import factories — is already enforced by ESLint, CI grep, and the in-suite meta-test. Sync factories don't trigger the race. The migration in #560 would be cosmetic at best, broken at worst.
Try a config-level fix. Look into Vitest's resolve.alias, setupFiles-based registration, or a custom Vitest module resolver to make the redirect work. Higher complexity.

Action

Closing this PR without merging.
Both commits will be reverted (or the branch deleted — they were never useful on their own).
Discussion issue filed; #560 to be re-scoped or closed pending that discussion.

Refs #560.

## Closing without merge — empirical finding invalidates the load-bearing premise CI run [#1857](https://git.raddatz.cloud/marcel/familienarchiv/actions/runs/1857) (commit `9fcd0553`) failed with 2 of 274 tests: ``` FAIL chromium src/routes/admin/tags/[id]/page.svelte.spec.ts > Admin edit tag page – merge success banner > shows merge success banner when data.mergeSuccess is true FAIL chromium src/routes/admin/tags/[id]/page.svelte.test.ts > admin/tags/[id] page > shows the merge-success banner when data.mergeSuccess is set Error: Cannot call replaceState(...) before router is initialized ❯ replaceState node_modules/@sveltejs/kit/src/runtime/client/client.js:2483:9 ❯ $effect src/routes/admin/tags/[id]/+page.svelte:48:1 ``` ### What this means `vi.mock('$app/navigation')` **without a factory** does NOT fully replace the SvelteKit virtual module via the shared `src/__mocks__/$app/navigation.ts`. Vitest's auto-mock substitutes some exports (e.g. `goto` becomes a `vi.fn()` — DocumentRow's tests verified that in run #1856) but **leaves others bound to the real implementation** (e.g. `replaceState` here). The result is partial mocking: code paths that exercise the unmocked exports hit the real router and fail. The personas (Felix, Markus, Sara, Nora, Elicit, Leonie, Tobias) and the issue body all assumed full auto-resolve via `__mocks__/`. None of the reviews verified this empirically. The first commit on this PR (run #1856) **only appeared** to validate the approach because `DocumentRow.svelte.spec.ts` happens not to exercise the partially-mocked exports. ### Pivot options surfaced by the failure Captured in follow-up discussion issue (to be filed). Three alternatives: 1. **Shared-mock + sync factory.** Keep `src/__mocks__/$app/navigation.ts`, but each spec uses `import * as navMock from '$mocks/...'; vi.mock('$app/navigation', () => ({ ...navMock }));`. Two lines per spec, consolidates the mock body, works because the factory is explicit. Sync factories are not banned by the meta-test (only `async + dynamic import` is). 2. **Drop the `__mocks__/` plan; keep status quo of per-spec factories.** The original goal in ADR-012 — preventing the birpc teardown race from `async + dynamic import` factories — is already enforced by ESLint, CI grep, and the in-suite meta-test. Sync factories don't trigger the race. The migration in #560 would be cosmetic at best, broken at worst. 3. **Try a config-level fix.** Look into Vitest's `resolve.alias`, `setupFiles`-based registration, or a custom Vitest module resolver to make the redirect work. Higher complexity. ### Action - Closing this PR without merging. - Both commits will be reverted (or the branch deleted — they were never useful on their own). - Discussion issue filed; #560 to be re-scoped or closed pending that discussion. Refs #560.

marcel closed this pull request

2026-05-21 19:15:12 +02:00

~~marcel referenced this pull request 2026-05-21 19:16:13 +02:00~~

Consolidate shared vi.mock bodies into __mocks__/ via sync factories + migrate confirm/notification specs to test-fixtures #560

marcel referenced this pull request

2026-06-02 19:02:09 +02:00

Consolidate shared vi.mock bodies into __mocks__/ via sync factories + migrate confirm/notification specs to test-fixtures #560

~~marcel referenced this pull request 2026-06-02 19:04:28 +02:00~~

Consolidate shared vi.mock bodies into __mocks__/ via sync factories + migrate confirm/notification specs to test-fixtures #560

~~marcel referenced this pull request 2026-06-02 19:04:43 +02:00~~

Consolidate shared vi.mock bodies into __mocks__/ via sync factories + migrate confirm/notification specs to test-fixtures #560

~~marcel referenced this pull request 2026-06-02 19:04:59 +02:00~~