marcel/familienarchiv
1
0
Fork 0
Code Issues 118 Pull Requests 2 Actions 4 Packages Projects Releases Wiki Activity

audit: factory mocks → prop injection migration (sveltest pattern) #554

New Issue
Closed
opened 2026-05-13 10:02:51 +02:00 by marcel · 1 comment
marcel commented 2026-05-13 10:02:51 +02:00
Owner
Copy Link

Context

Follow-up from #553 / ADR-012. The fix in #553 closes the two named birpc-race triggers (async factory body, duplicate ID across spellings). It does not address the underlying ergonomics question: every vi.mock(module, factory) in browser mode is a class of birpc race we can't hit if it doesn't exist.

Familienarchiv currently has ~92 factory-style vi.mock calls across src/**/*.svelte.{spec,test}.ts. The ecosystem trend — verified against sveltest, the official SvelteKit basics smoke, and Scott Spence's reference guide — is fewer mocks, more prop injection / test-host wrappers. We already have one canonical example in the repo: confirm.test-host.svelte.

Scope

Audit only. Produce a report; do not migrate.

For each of the ~92 factory vi.mock call sites:

  1. Classify into one of:

    • (a) Prop-injection candidate — could be replaced by threading the service/dep as a $props() field, with a default that matches today's import. Mirrors confirm.test-host.svelte. Best fit for domain services.
    • (b) __mocks__/ redirect candidate — a stable, never-changing fake that lives next to the source. Best fit for SvelteKit virtual modules ($app/state, $env/static/public) and pure utility modules.
    • (c) Keep as factory — no viable alternative (e.g. modules that genuinely need per-test override and have no injection seam).

  2. Tally the result. Output: a table of file → mocked module → classification → rationale, plus a summary count per class.

  3. Propose a roadmap. Recommend an order — typically (a) first, since each migration removes one route handler and one cleanup slot; (b) second, since it consolidates many duplicate factories; (c) is the residual.

Non-goals

  • Not in scope to migrate. Output is a markdown report committed under docs/audits/, not code changes.
  • Not in scope to revisit ADR-012's sync-factory invariant or the patch-package backport — those are settled.

Acceptance

  • docs/audits/<date>-vi-mock-migration-audit.md exists on main
  • Every factory vi.mock call site in src/**/*.svelte.{spec,test}.ts is classified
  • Roadmap section recommends a migration order with rough effort per class
  • Issue links the audit report and is closed

References

  • ADR-012 — Browser-mode test mocking strategy
  • #553 — Sync-factory invariant + duplicate-ID fix
  • frontend/src/lib/shared/services/confirm.test-host.svelte — canonical prop-injection example in this repo
  • sveltest — apps/website/src/lib/components/nav.svelte.test.ts shows the test-host pattern at scale
## Context Follow-up from #553 / [ADR-012](../src/branch/main/docs/adr/012-browser-test-mocking-strategy.md). The fix in #553 closes the two named birpc-race triggers (async factory body, duplicate ID across spellings). It does not address the underlying ergonomics question: **every `vi.mock(module, factory)` in browser mode is a class of birpc race we can't hit if it doesn't exist.** Familienarchiv currently has ~92 factory-style `vi.mock` calls across `src/**/*.svelte.{spec,test}.ts`. The ecosystem trend — verified against [sveltest](https://github.com/spences10/sveltest), the [official SvelteKit basics smoke](https://github.com/sveltejs/kit/tree/main/packages/create-svelte/templates/default), and Scott Spence's reference guide — is *fewer mocks, more prop injection / test-host wrappers*. We already have one canonical example in the repo: `confirm.test-host.svelte`. ## Scope **Audit only.** Produce a report; do not migrate. For each of the ~92 factory `vi.mock` call sites: 1. **Classify** into one of: - (a) **Prop-injection candidate** — could be replaced by threading the service/dep as a `$props()` field, with a default that matches today's import. Mirrors `confirm.test-host.svelte`. Best fit for domain services. - (b) **`__mocks__/` redirect candidate** — a stable, never-changing fake that lives next to the source. Best fit for SvelteKit virtual modules (`$app/state`, `$env/static/public`) and pure utility modules. - (c) **Keep as factory** — no viable alternative (e.g. modules that genuinely need per-test override and have no injection seam). 2. **Tally** the result. Output: a table of `file → mocked module → classification → rationale`, plus a summary count per class. 3. **Propose a roadmap.** Recommend an order — typically (a) first, since each migration removes one route handler and one cleanup slot; (b) second, since it consolidates many duplicate factories; (c) is the residual. ## Non-goals - **Not in scope to migrate.** Output is a markdown report committed under `docs/audits/`, not code changes. - Not in scope to revisit ADR-012's sync-factory invariant or the patch-package backport — those are settled. ## Acceptance - [ ] `docs/audits/<date>-vi-mock-migration-audit.md` exists on `main` - [ ] Every factory `vi.mock` call site in `src/**/*.svelte.{spec,test}.ts` is classified - [ ] Roadmap section recommends a migration order with rough effort per class - [ ] Issue links the audit report and is closed ## References - ADR-012 — Browser-mode test mocking strategy - #553 — Sync-factory invariant + duplicate-ID fix - `frontend/src/lib/shared/services/confirm.test-host.svelte` — canonical prop-injection example in this repo - [sveltest](https://github.com/spences10/sveltest) — `apps/website/src/lib/components/nav.svelte.test.ts` shows the test-host pattern at scale
marcel added the P3-lateraudittest labels 2026-05-13 10:02:59 +02:00
marcel commented 2026-05-14 10:37:35 +02:00
Author
Owner
Copy Link

Audit complete. Report filed as #560.

Results: 87 in-scope call sites across 12 mocked modules — 72 → __mocks__/ redirect, 10 → prop-injection, 5 → keep as factory. Full classification table and migration roadmap (~5 days total) in the linked issue.

Audit complete. Report filed as #560. Results: 87 in-scope call sites across 12 mocked modules — 72 → `__mocks__/` redirect, 10 → prop-injection, 5 → keep as factory. Full classification table and migration roadmap (~5 days total) in the linked issue.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
P0-critical
Blocks a core user journey, causes data loss, or is a security/accessibility barrier. Work on this before P1+.
 P1-high
Significant friction on a main user journey; workaround exists but is non-obvious. Next up after P0.
 P2-medium
Noticeable improvement; doesn't block anything; schedule opportunistically.
 P3-later
Cosmetic or parking-lot work; fine to stay open indefinitely.
 audit
Read-only audit / assessment work; produces a report rather than changing code
 bug
Something isn't working
 cleanup
Removal of dead code, vague comments, naming violations, and scratch artifacts
 collaboration
We want to extend the family archive to have more collaboration tools
 conversation
descoped
We will do that later
 devops
documentation
README, ARCHITECTURE, GLOSSARY, CONTRIBUTING, per-domain docs
 epic
Marker for epic-level issues that group multiple child issues
 feature
file-upload
legibility
Codebase Legibility Refactor — preparing the codebase for human evaluation and bus-factor reduction
 notification
person
phase-1: security
Security hygiene — must be done first
 phase-2: container-images
Production-ready multi-stage Docker images
 phase-3: prod-compose
Production compose overlay + Caddy reverse proxy
 phase-4: spring-prod-profile
Spring Boot production configuration profile
 phase-5: backups
Database and object storage backup strategy
 phase-6: deployment-docs
.env.example, DEPLOYMENT.md, runbook
 phase-7: monitoring
Prometheus, Loki, Grafana, Alertmanager
 refactor
Code restructuring without behaviour change
 security
test
ui
UI/UX and accessibility issues
 user
No Label P3-later audit test
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
Jens marcel
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: marcel/familienarchiv#554
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?