Consolidate shared vi.mock bodies + migrate confirm/notification specs (#560) #719

Merged
marcel merged 17 commits from feat/issue-560-shared-vimock-mocks into main 2026-06-03 11:38:23 +02:00

17 Commits

Author SHA1 Message Date
Marcel
30a8e36563 test(notification): make setNotifications authoritative in bell a11y tests
All checks were successful
CI / Unit & Component Tests (pull_request) Successful in 3m40s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m36s
CI / fail2ban Regex (pull_request) Successful in 45s
CI / Semgrep Security Scan (pull_request) Successful in 23s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
CI showed the single/many a11y tests failing with count 0: init()'s async
fetchUnreadCount resolved to {count:0} AFTER setNotifications() ran,
clobbering the seeded count (the flake Sara predicted in review). Stub
fetch to never settle so the announced count is driven solely by
setNotifications — deterministic, no race. Also rewrites the 'error' test
to seed a count then fail the load and assert the count SURVIVES, so it is
a meaningful state distinct from 'empty' (was byte-identical, flagged by
Felix/Sara/Leonie). Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 10:32:00 +02:00
Marcel
cdfcb02903 revert(test): abandon shared-mock dedup — infeasible in vitest browser mode
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m42s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m31s
CI / fail2ban Regex (pull_request) Successful in 46s
CI / Semgrep Security Scan (pull_request) Successful in 21s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m5s
CI proved cross-file sharing of a virtual-module mock body cannot work in
@vitest/browser-playwright 4.1.6: the static-import spread fails the hoist
("no top level variables"), and the await-vi.hoisted-import form fails to
parse ("Unexpected identifier 'vi'"). vi.hoisted has the same hoist
constraint as vi.mock, so there is no way to thread an external module's
body into the factory here.

Reverts Phase 1: restores the 4 $app/forms/$app/navigation specs to their
inline factories, inlines NotificationBell.spec's forms stub, deletes the
src/__mocks__/$app/* modules and the $mocks alias (vite, vitest-coverage,
kit). The no-factory-ban meta-test stays (no-factory vi.mock is still
banned). ADR-012 amended to record the infeasibility. Everything else
($app/state migration, confirm context-inject, notification refactor, the
pin, the meta-test) is unaffected. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 08:21:02 +02:00
Marcel
f13999d006 fix(test): load shared mocks via vi.hoisted, not a static import
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 2m39s
CI / OCR Service Tests (pull_request) Successful in 24s
CI / Backend Unit Tests (pull_request) Successful in 3m37s
CI / fail2ban Regex (pull_request) Successful in 45s
CI / Semgrep Security Scan (pull_request) Successful in 22s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m3s
CI caught that vi.mock('$app/forms', () => ({ ...formsMock })) with a
static `import * as formsMock` fails: vitest hoists vi.mock above the
import, so the factory references an uninitialised binding
("no top level variables inside"). Load the shared mock module via
`const formsMock = await vi.hoisted(() => import('$mocks/...'))` instead —
the factory may reference a vi.hoisted binding, and the dynamic import runs
at collection time (not in the lazily-invoked factory), so it stays clear
of ADR-012's birpc race and the no-async-mock-factories guard. Applies to
all 5 shared-mock consumers ($app/forms x4, $app/navigation x1). Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-03 07:52:37 +02:00
Marcel
140b321868 refactor(notification): provide notification store via context + fixture
Some checks failed
CI / Unit & Component Tests (pull_request) Failing after 1m50s
CI / OCR Service Tests (pull_request) Successful in 22s
CI / Backend Unit Tests (pull_request) Successful in 3m33s
CI / fail2ban Regex (pull_request) Successful in 44s
CI / Semgrep Security Scan (pull_request) Successful in 21s
CI / Compose Bucket Idempotency (pull_request) Successful in 1m6s
Converts the module-singleton notificationStore into a context-provided
store so its specs can drive it without mocking the module. notifications.svelte
now exports createNotificationStore() (the former singleton body), plus
provideNotificationStore()/getNotificationStore()/NOTIFICATION_KEY mirroring
the confirm service. Root +layout provides it; NotificationBell and the
Chronik page read it via getNotificationStore().

Tests:
- notifications.svelte.spec drives a fresh createNotificationStore() per test
  (replacing __resetForTest/__setNavigateForTest with setNavigate()).
- notification.test-fixture.svelte wraps the bell, provides the store, and
  exposes setNotifications(items) via onReady (option b).
- NotificationBell.svelte.spec asserts the announced unread count across the
  empty / single / many / error a11y states (AC#5), stubbing EventSource+fetch.
- aktivitaeten page spec injects a real store via render context.

Per the recorded Phase-2b decision (full context refactor). Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
13c6b7d27f test: inject real ConfirmService via context (batch 2/2)
Completes Phase 2a: geschichten/[id], persons/[id]/edit and admin/tags/[id]
page specs now provide a real createConfirmService() via render context
instead of mocking confirm.svelte. Zero confirm.svelte vi.mocks remain
across the client suite (AC#4). Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
a4d4ff35d9 test: inject real ConfirmService via context (batch 1/2)
Replaces the vi.mock('$lib/shared/services/confirm.svelte') stub with a
real createConfirmService() provided through render's context map, mirroring
the existing admin/tags/[id]/page.svelte.spec.ts pattern. The generic
confirm.test-fixture.svelte renders only ConfirmDialog and cannot wrap an
arbitrary page; none of these specs trigger confirm(), so the children's
getConfirmService() simply reads the provided context instead of a module
mock. No vi.mock of confirm.svelte remains in these 5 specs. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
e5cf98e1d4 test(hooks): migrate useUnsavedWarning spec to shared $app/navigation mock
Replaces the local beforeNavigate-capture plumbing and simulateNavigate
helper with the shared $mocks/$app/navigation module via a sync factory.
The per-test reset now comes from the shared module's embedded beforeEach.
Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
cc76d4de91 test(mocks): add shared $app/navigation mock with simulateNavigate
Exports the standard nav functions as vi.fn() and a beforeNavigate that
captures the registered callback. The exported simulateNavigate(href)
helper fires that callback and returns the cancel spy — the whole
capture-and-fire pattern lives in the shared module, not the raw callback.
An embedded beforeEach clears the captured callback and the mock call
histories before every test. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
8b1eeba91c test: migrate remaining 3 $app/forms consumers to shared mock
Completes Phase 1a after the load-bearing ChronikFuerDichBox spec proved
the pattern. ChronikFuerDichBox.test and NotificationDropdown.test (rich
result-firing interceptors) keep their submit-fired assertions
(optimisticMarkRead/MarkAllRead) and use formsMock.setFormResult for the
failure branch. NotificationBell.spec used the simpler intercept-only
factory and renders no form of its own, so it adopts the shared superset
purely as a render-time stub. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
d2fb023876 test(activity): migrate ChronikFuerDichBox spec to shared $app/forms mock
Load-bearing first migration (ADR-012): this is the hardest case — its
enhance submit callback actually fires and reads the form result. Replaces
the duplicated 23-line interceptor factory with vi.mock('$app/forms',
() => ({ ...formsMock })) via $mocks, and the per-test mockFormResult
mutation with formsMock.setFormResult({ type: 'failure' }). The reset now
comes from the shared module's embedded beforeEach. The existing
optimisticMarkRead/optimisticMarkAllRead-on-submit assertions remain as the
positive proof the callback fired. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
01c28ca13c test(mocks): add shared $app/forms interceptor mock body
Single home for the non-trivial form-interceptor enhance() shared by the
four complex consumers: it intercepts submit, invokes the SubmitFunction,
and fires the returned callback with a configurable result. setFormResult()
drives the success/failure branch; an embedded beforeEach resets it before
every test so isolation is structural. Consumed via vi.mock('$app/forms',
() => ({ ...formsMock })) through the $mocks alias. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
f6778b0eff build(frontend): register $mocks in kit.alias for tsconfig resolution
The vite resolve.alias (added for the client + coverage runs) does not
reach svelte-check, which resolves paths through the generated tsconfig.
Declaring $mocks in kit.alias feeds both the generated tsconfig paths and
the sveltekit() vite plugin, so editor/type-check resolve it too. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
a0bcdcd19a docs(adr): amend ADR-012 with no-factory ban + shared-mock dedup (#560)
Records the 2026-06-02 revision from #560: (1) no-factory vi.mock of a
SvelteKit virtual module is forbidden (the PR #657 partial-mock failure),
guarded by a seventh enforcement layer; (2) shared mock body + per-spec
sync factory via the $mocks alias is the sanctioned dedup; (3) Option C
config-level auto-resolve is rejected. Also corrects the stale 4.1.0
patch filename to 4.1.6 and links #657. Part of #560.
2026-06-02 20:27:19 +02:00
Marcel
fa95cc5e21 test(meta): ban no-factory vi.mock of virtual modules
A vi.mock('$app/navigation') with no factory does not auto-resolve to a
__mocks__ file for SvelteKit virtual modules — it substitutes some
exports and leaves others (replaceState) bound to the live router, which
is exactly the PR #657 failure. This Node-mode source scan, mirroring
no-async-mock-factories and no-duplicate-mock-ids, fails at every vitest
invocation if any *.svelte.{spec,test}.ts reintroduces the pattern, and
forecloses ADR-012's rejected Option C. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
7c2d1807df build(frontend): add $mocks alias for shared browser-test mock bodies
Declares $mocks -> src/__mocks__ in both vite.config.ts and
vitest.client-coverage.config.ts so shared mock modules resolve in the
client test run and the coverage job alike. Enables the sync-factory
dedup pattern from ADR-012 (vi.mock('$app/forms', () => ({ ...formsMock }))).
Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
e1bd090acd build(frontend): exact-pin @vitest/browser-playwright to 4.1.6
Drop the caret so the version cannot float off the patched release.
patches/@vitest+browser-playwright+4.1.6.patch backports vitest PR #10267
(the duplicate-mock-id birpc race, ADR-012) and only applies to 4.1.6; a
caret range could resolve to a version the patch rejects. A top-level
"//" key records the removal condition since package.json forbids
comments. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00
Marcel
c56aa1c645 refactor(admin-tags): migrate tag-edit page from $app/stores to $app/state
The legacy $app/stores subscription API is replaced with the modern
$app/state reactive proxy (page.url.pathname), per ADR-012's
architectural follow-on. The two spec mocks of $app/stores are replaced
with sync-factory $app/state mocks, matching the existing convention in
aktivitaeten/documents specs. Part of #560.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-06-02 20:27:19 +02:00