From c56aa1c645ea938c1a25268cdfe9959ce37822a1 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 2 Jun 2026 19:24:52 +0200 Subject: [PATCH 01/17] 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 --- frontend/src/routes/admin/tags/[id]/+page.svelte | 4 ++-- .../src/routes/admin/tags/[id]/page.svelte.spec.ts | 9 +++------ .../src/routes/admin/tags/[id]/page.svelte.test.ts | 11 ++++------- 3 files changed, 9 insertions(+), 15 deletions(-) diff --git a/frontend/src/routes/admin/tags/[id]/+page.svelte b/frontend/src/routes/admin/tags/[id]/+page.svelte index 8278d7c5..7fbd52c0 100644 --- a/frontend/src/routes/admin/tags/[id]/+page.svelte +++ b/frontend/src/routes/admin/tags/[id]/+page.svelte @@ -1,7 +1,7 @@ + + diff --git a/frontend/src/lib/notification/notifications.svelte.spec.ts b/frontend/src/lib/notification/notifications.svelte.spec.ts index 15345286..72ac98e3 100644 --- a/frontend/src/lib/notification/notifications.svelte.spec.ts +++ b/frontend/src/lib/notification/notifications.svelte.spec.ts @@ -39,19 +39,20 @@ vi.stubGlobal('EventSource', MockEventSource); const mockFetch = vi.fn(); vi.stubGlobal('fetch', mockFetch); -const { notificationStore, __resetForTest, __setNavigateForTest } = - await import('./notifications.svelte'); +const { createNotificationStore } = await import('./notifications.svelte'); -let navigateSpy: ReturnType; +let store: ReturnType; +let navigateSpy: ReturnType void>>; beforeEach(() => { mockFetch.mockReset(); mockFetch.mockResolvedValue(new Response(JSON.stringify({ count: 0 }), { status: 200 })); lastEventSource = null; eventSourceCount = 0; - navigateSpy = vi.fn(); - __setNavigateForTest(navigateSpy); - __resetForTest(); + navigateSpy = vi.fn<(url: string) => void>(); + // A fresh instance per test replaces the old __resetForTest() singleton reset. + store = createNotificationStore(); + store.setNavigate(navigateSpy); }); function makeNotification(overrides: Partial = {}): NotificationItem { @@ -69,70 +70,70 @@ function makeNotification(overrides: Partial = {}): Notificati }; } -describe('notificationStore (singleton)', () => { +describe('notification store', () => { it('opens a single EventSource across multiple init() calls', () => { - notificationStore.init(); - notificationStore.init(); - notificationStore.init(); + store.init(); + store.init(); + store.init(); expect(eventSourceCount).toBe(1); }); it('closes the EventSource only after every init() is matched with destroy()', () => { - notificationStore.init(); - notificationStore.init(); + store.init(); + store.init(); const es = lastEventSource!; - notificationStore.destroy(); + store.destroy(); expect(es.close).not.toHaveBeenCalled(); - notificationStore.destroy(); + store.destroy(); expect(es.close).toHaveBeenCalledTimes(1); }); it('reopens a fresh EventSource after full teardown', () => { - notificationStore.init(); - notificationStore.destroy(); - notificationStore.init(); + store.init(); + store.destroy(); + store.init(); expect(eventSourceCount).toBe(2); }); it('SSE notification event prepends notification and increments unreadCount', () => { - notificationStore.init(); + store.init(); const notification = makeNotification({ id: 'sse-1', read: false }); lastEventSource!.simulate('notification', JSON.stringify(notification)); - expect(notificationStore.notifications[0].id).toBe('sse-1'); - expect(notificationStore.unreadCount).toBe(1); + expect(store.notifications[0].id).toBe('sse-1'); + expect(store.unreadCount).toBe(1); }); it('optimisticMarkRead marks the notification read and decrements unreadCount without fetching', () => { - notificationStore.init(); + store.init(); const notification = makeNotification({ id: 'sse-1', read: false }); lastEventSource!.simulate('notification', JSON.stringify(notification)); mockFetch.mockReset(); // clear the fetchUnreadCount call from init - notificationStore.optimisticMarkRead('sse-1'); + store.optimisticMarkRead('sse-1'); - expect(notificationStore.notifications[0].read).toBe(true); - expect(notificationStore.unreadCount).toBe(0); + expect(store.notifications[0].read).toBe(true); + expect(store.unreadCount).toBe(0); expect(mockFetch).not.toHaveBeenCalled(); }); it('optimisticMarkRead on an already-read notification does not decrement unreadCount below 0', () => { - notificationStore.init(); + store.init(); const notification = makeNotification({ id: 'sse-1', read: true }); lastEventSource!.simulate('notification', JSON.stringify(notification)); - notificationStore.optimisticMarkRead('sse-1'); + store.optimisticMarkRead('sse-1'); - expect(notificationStore.unreadCount).toBe(0); + expect(store.unreadCount).toBe(0); }); it('optimisticMarkAllRead resets unreadCount and marks all notifications read without fetching', () => { - notificationStore.init(); + store.init(); lastEventSource!.simulate( 'notification', JSON.stringify(makeNotification({ id: 'n1', read: false })) @@ -143,18 +144,18 @@ describe('notificationStore (singleton)', () => { ); mockFetch.mockReset(); - notificationStore.optimisticMarkAllRead(); + store.optimisticMarkAllRead(); - expect(notificationStore.unreadCount).toBe(0); - expect(notificationStore.notifications.every((n) => n.read)).toBe(true); + expect(store.unreadCount).toBe(0); + expect(store.notifications.every((n) => n.read)).toBe(true); expect(mockFetch).not.toHaveBeenCalled(); }); }); -describe('notificationStore onerror handler', () => { +describe('notification store onerror handler', () => { it('redirects to /login when readyState is CLOSED and server returns 401', async () => { mockFetch.mockResolvedValue(new Response(null, { status: 401 })); - notificationStore.init(); + store.init(); const es = lastEventSource!; es.readyState = MockEventSource.CLOSED; @@ -164,7 +165,7 @@ describe('notificationStore onerror handler', () => { }); it('does not redirect when readyState is CLOSED and session is still valid', async () => { - notificationStore.init(); + store.init(); const es = lastEventSource!; es.readyState = MockEventSource.CLOSED; @@ -174,7 +175,7 @@ describe('notificationStore onerror handler', () => { }); it('does not close or redirect before the error threshold when readyState is CONNECTING', async () => { - notificationStore.init(); + store.init(); const es = lastEventSource!; es.readyState = MockEventSource.CONNECTING; @@ -187,7 +188,7 @@ describe('notificationStore onerror handler', () => { it('closes and redirects after 3 consecutive CONNECTING errors when session returns 401', async () => { mockFetch.mockResolvedValue(new Response(null, { status: 401 })); - notificationStore.init(); + store.init(); const es = lastEventSource!; es.readyState = MockEventSource.CONNECTING; @@ -200,7 +201,7 @@ describe('notificationStore onerror handler', () => { }); it('closes but does not redirect after threshold when session is still valid', async () => { - notificationStore.init(); + store.init(); const es = lastEventSource!; es.readyState = MockEventSource.CONNECTING; @@ -213,7 +214,7 @@ describe('notificationStore onerror handler', () => { }); it('resets error count after a successful reconnect (onopen)', async () => { - notificationStore.init(); + store.init(); const es = lastEventSource!; es.readyState = MockEventSource.CONNECTING; diff --git a/frontend/src/lib/notification/notifications.svelte.ts b/frontend/src/lib/notification/notifications.svelte.ts index 1e52cfe3..8d975d0e 100644 --- a/frontend/src/lib/notification/notifications.svelte.ts +++ b/frontend/src/lib/notification/notifications.svelte.ts @@ -1,121 +1,161 @@ +import { getContext, setContext } from 'svelte'; import { type NotificationItem, parseNotificationEvent } from '$lib/notification/notifications'; export type { NotificationItem }; -let notifications = $state([]); -let unreadCount = $state(0); -let eventSource: EventSource | null = null; -let refCount = 0; -let errorCount = 0; -let navigate: (url: string) => void = (url) => { - window.location.href = url; -}; +export const NOTIFICATION_KEY = Symbol('notification'); -async function fetchNotifications(): Promise { - try { - const res = await fetch('/api/notifications?size=10'); - if (res.ok) { - const data = await res.json(); - notifications = data.content ?? []; - } - } catch (e) { - console.error('Failed to fetch notifications', e); - } +export interface NotificationStore { + readonly notifications: NotificationItem[]; + readonly unreadCount: number; + fetchNotifications(): Promise; + fetchUnreadCount(): Promise; + optimisticMarkRead(id: string): void; + optimisticMarkAllRead(): void; + init(): void; + destroy(): void; + /** Test-only: seed the notification list and derive the unread count. */ + setNotifications(items: NotificationItem[]): void; + /** Test-only: override the 401 → redirect side-effect. */ + setNavigate(fn: (url: string) => void): void; } -async function fetchUnreadCount(): Promise { - try { - const res = await fetch('/api/notifications/unread-count'); - if (res.ok) { - const data = await res.json(); - unreadCount = data.count; - } - } catch (e) { - console.error('Failed to fetch unread count', e); - } -} - -function optimisticMarkRead(id: string): void { - const notification = notifications.find((n) => n.id === id); - if (notification && !notification.read) { - notification.read = true; - unreadCount = Math.max(0, unreadCount - 1); - } -} - -function optimisticMarkAllRead(): void { - for (const n of notifications) { - n.read = true; - } - unreadCount = 0; -} - -function init(): void { - refCount += 1; - if (refCount > 1) return; - - fetchUnreadCount(); - eventSource = new EventSource('/api/notifications/stream'); - eventSource.addEventListener('notification', (e) => { - const notification = parseNotificationEvent((e as MessageEvent).data); - if (!notification) return; - notifications = [notification, ...notifications]; - if (!notification.read) unreadCount += 1; - }); - eventSource.onopen = () => { - fetchUnreadCount(); - errorCount = 0; +export function createNotificationStore(): NotificationStore { + let notifications = $state([]); + let unreadCount = $state(0); + let eventSource: EventSource | null = null; + let refCount = 0; + let errorCount = 0; + let navigate: (url: string) => void = (url) => { + window.location.href = url; }; - eventSource.onerror = async () => { - if (eventSource?.readyState === EventSource.CLOSED) { - const res = await fetch('/api/notifications/unread-count'); - if (res.status === 401) navigate('/login'); - return; + + async function fetchNotifications(): Promise { + try { + const res = await fetch('/api/notifications?size=10'); + if (res.ok) { + const data = await res.json(); + notifications = data.content ?? []; + } + } catch (e) { + console.error('Failed to fetch notifications', e); } - errorCount += 1; - if (errorCount >= 3) { + } + + async function fetchUnreadCount(): Promise { + try { + const res = await fetch('/api/notifications/unread-count'); + if (res.ok) { + const data = await res.json(); + unreadCount = data.count; + } + } catch (e) { + console.error('Failed to fetch unread count', e); + } + } + + function optimisticMarkRead(id: string): void { + const notification = notifications.find((n) => n.id === id); + if (notification && !notification.read) { + notification.read = true; + unreadCount = Math.max(0, unreadCount - 1); + } + } + + function optimisticMarkAllRead(): void { + for (const n of notifications) { + n.read = true; + } + unreadCount = 0; + } + + function init(): void { + refCount += 1; + if (refCount > 1) return; + + fetchUnreadCount(); + eventSource = new EventSource('/api/notifications/stream'); + eventSource.addEventListener('notification', (e) => { + const notification = parseNotificationEvent((e as MessageEvent).data); + if (!notification) return; + notifications = [notification, ...notifications]; + if (!notification.read) unreadCount += 1; + }); + eventSource.onopen = () => { + fetchUnreadCount(); + errorCount = 0; + }; + eventSource.onerror = async () => { + if (eventSource?.readyState === EventSource.CLOSED) { + const res = await fetch('/api/notifications/unread-count'); + if (res.status === 401) navigate('/login'); + return; + } + errorCount += 1; + if (errorCount >= 3) { + eventSource?.close(); + eventSource = null; + errorCount = 0; + const res = await fetch('/api/notifications/unread-count'); + if (res.status === 401) navigate('/login'); + } + }; + } + + function destroy(): void { + if (refCount === 0) return; + refCount -= 1; + if (refCount === 0) { eventSource?.close(); eventSource = null; - errorCount = 0; - const res = await fetch('/api/notifications/unread-count'); - if (res.status === 401) navigate('/login'); + } + } + + return { + get notifications() { + return notifications; + }, + get unreadCount() { + return unreadCount; + }, + fetchNotifications, + fetchUnreadCount, + optimisticMarkRead, + optimisticMarkAllRead, + init, + destroy, + setNotifications(items: NotificationItem[]): void { + notifications = items; + unreadCount = items.filter((n) => !n.read).length; + }, + setNavigate(fn: (url: string) => void): void { + navigate = fn; } }; } -function destroy(): void { - if (refCount === 0) return; - refCount -= 1; - if (refCount === 0) { - eventSource?.close(); - eventSource = null; +/** + * Create a notification store and put it on the context. Call once high in the + * tree (root +layout.svelte). Descendants read it via getNotificationStore(). + */ +export function provideNotificationStore(): NotificationStore { + const store = createNotificationStore(); + setContext(NOTIFICATION_KEY, store); + return store; +} + +export function getNotificationStore(): NotificationStore { + let store: NotificationStore | undefined; + try { + store = getContext(NOTIFICATION_KEY); + } catch { + throw new Error( + 'NotificationStore not found — call provideNotificationStore() in +layout.svelte' + ); } + if (!store) + throw new Error( + 'NotificationStore not found — call provideNotificationStore() in +layout.svelte' + ); + return store; } - -export function __resetForTest(): void { - eventSource?.close(); - eventSource = null; - refCount = 0; - errorCount = 0; - notifications = []; - unreadCount = 0; -} - -export function __setNavigateForTest(fn: (url: string) => void): void { - navigate = fn; -} - -export const notificationStore = { - get notifications() { - return notifications; - }, - get unreadCount() { - return unreadCount; - }, - fetchNotifications, - fetchUnreadCount, - optimisticMarkRead, - optimisticMarkAllRead, - init, - destroy -}; diff --git a/frontend/src/routes/+layout.svelte b/frontend/src/routes/+layout.svelte index 136eb060..1ca23440 100644 --- a/frontend/src/routes/+layout.svelte +++ b/frontend/src/routes/+layout.svelte @@ -10,6 +10,7 @@ import AppNav from './AppNav.svelte'; import UserMenu from './UserMenu.svelte'; import ConfirmDialog from '$lib/shared/primitives/ConfirmDialog.svelte'; import { provideConfirmService } from '$lib/shared/services/confirm.svelte'; +import { provideNotificationStore } from '$lib/notification/notifications.svelte'; import { bulkSelectionStore } from '$lib/document/bulkSelection.svelte'; let { children, data } = $props(); @@ -18,6 +19,11 @@ let { children, data } = $props(); // ConfirmDialog below reads it via getConfirmService() and renders the . provideConfirmService(); +// Provide the notification store to the tree. NotificationBell (header) and the +// Chronik page read it via getNotificationStore(); the bell drives the SSE +// lifecycle through init()/destroy() on mount. +provideNotificationStore(); + // Auto-clear the bulk-selection store when the user leaves the routes that // surface the BulkSelectionBar. Without this the selection silently follows // the user to /persons / /admin etc. and reappears as a stale 3-doc count diff --git a/frontend/src/routes/aktivitaeten/+page.svelte b/frontend/src/routes/aktivitaeten/+page.svelte index 7246e51e..06a0ff6b 100644 --- a/frontend/src/routes/aktivitaeten/+page.svelte +++ b/frontend/src/routes/aktivitaeten/+page.svelte @@ -3,7 +3,10 @@ import { onMount, onDestroy } from 'svelte'; import { goto } from '$app/navigation'; import { page, navigating } from '$app/state'; import * as m from '$lib/paraglide/messages.js'; -import { notificationStore, type NotificationItem } from '$lib/notification/notifications.svelte'; +import { + getNotificationStore, + type NotificationItem +} from '$lib/notification/notifications.svelte'; import ChronikFuerDichBox from '$lib/activity/ChronikFuerDichBox.svelte'; import ChronikFilterPills from '$lib/activity/ChronikFilterPills.svelte'; import ChronikTimeline from '$lib/activity/ChronikTimeline.svelte'; @@ -26,9 +29,11 @@ interface Props { const { data }: Props = $props(); -// Prefer the live SSE singleton for unread items so newly arriving mentions +const notificationStore = getNotificationStore(); + +// Prefer the live SSE store for unread items so newly arriving mentions // prepend without a reload. On first mount, seed from the server-loaded unread -// set if the singleton hasn't populated yet. +// set if the store hasn't populated yet. onMount(() => { notificationStore.init(); }); @@ -59,7 +64,7 @@ const seedUnread = $derived( })) ); -// If the singleton has any data (including zero after mark-all), trust it; +// If the store has any data (including zero after mark-all), trust it; // otherwise fall back to the SSR-seeded unread set. const unread = $derived( notificationStore.notifications.length > 0 ? liveUnread : seedUnread diff --git a/frontend/src/routes/aktivitaeten/page.svelte.test.ts b/frontend/src/routes/aktivitaeten/page.svelte.test.ts index def1980f..e9b90c4e 100644 --- a/frontend/src/routes/aktivitaeten/page.svelte.test.ts +++ b/frontend/src/routes/aktivitaeten/page.svelte.test.ts @@ -1,6 +1,7 @@ -import { describe, it, expect, vi, afterEach } from 'vitest'; +import { describe, it, expect, vi, afterEach, beforeEach } from 'vitest'; import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; +import { createNotificationStore, NOTIFICATION_KEY } from '$lib/notification/notifications.svelte'; const mockNavigating = { type: null }; const mockPage = { url: new URL('http://localhost/aktivitaeten') }; @@ -28,19 +29,35 @@ vi.mock('$app/navigation', () => ({ onNavigate: () => () => {} })); -vi.mock('$lib/notification/notifications.svelte', () => ({ - notificationStore: { - notifications: [], - init: vi.fn(), - destroy: vi.fn(), - markRead: vi.fn(), - markAllRead: vi.fn() - } -})); +// The Chronik page's onMount calls store.init(), opening an EventSource and +// fetching the unread count. Stub both so no real network / 401 → /login fires. +class NoopEventSource { + static CLOSED = 2; + readyState = 0; + onopen: (() => void) | null = null; + onerror: (() => void) | null = null; + addEventListener() {} + close() {} +} + +beforeEach(() => { + vi.stubGlobal('EventSource', NoopEventSource); + vi.stubGlobal( + 'fetch', + vi + .fn() + .mockResolvedValue(new Response(JSON.stringify({ count: 0, content: [] }), { status: 200 })) + ); +}); const { default: AktivitaetenPage } = await import('./+page.svelte'); -afterEach(cleanup); +afterEach(() => { + cleanup(); + vi.unstubAllGlobals(); +}); + +const notificationContext = () => new Map([[NOTIFICATION_KEY, createNotificationStore()]]); const baseData = (overrides: Record = {}) => ({ filter: 'alle' as const, @@ -52,13 +69,16 @@ const baseData = (overrides: Record = {}) => ({ describe('aktivitaeten page', () => { it('renders the page heading', async () => { - render(AktivitaetenPage, { props: { data: baseData() } }); + render(AktivitaetenPage, { context: notificationContext(), props: { data: baseData() } }); await expect.element(page.getByRole('heading', { name: /aktivitäten/i })).toBeVisible(); }); it('renders the error card when loadError is "activity"', async () => { - render(AktivitaetenPage, { props: { data: baseData({ loadError: 'activity' }) } }); + render(AktivitaetenPage, { + context: notificationContext(), + props: { data: baseData({ loadError: 'activity' }) } + }); // ChronikErrorCard renders some retry mechanism const main = document.querySelector('main'); @@ -69,7 +89,7 @@ describe('aktivitaeten page', () => { }); it('renders the FuerDichBox and FilterPills when loadError is null', async () => { - render(AktivitaetenPage, { props: { data: baseData() } }); + render(AktivitaetenPage, { context: notificationContext(), props: { data: baseData() } }); // FuerDichBox shows the inbox-zero state when no unread const fuerDich = document.querySelector('[data-testid="chronik-inbox-zero"]'); @@ -81,7 +101,7 @@ describe('aktivitaeten page', () => { }); it('renders the first-run empty state when activityFeed is empty', async () => { - render(AktivitaetenPage, { props: { data: baseData() } }); + render(AktivitaetenPage, { context: notificationContext(), props: { data: baseData() } }); const empty = document.querySelector('[data-testid="chronik-empty-state"]'); expect(empty?.getAttribute('data-variant')).toBe('first-run'); @@ -89,6 +109,7 @@ describe('aktivitaeten page', () => { it('renders the filter-empty empty state when feed has items but filter rules out all', async () => { render(AktivitaetenPage, { + context: notificationContext(), props: { data: baseData({ filter: 'fuer-dich' as const, @@ -114,6 +135,7 @@ describe('aktivitaeten page', () => { it('renders the timeline when displayFeed is non-empty', async () => { render(AktivitaetenPage, { + context: notificationContext(), props: { data: baseData({ filter: 'alle' as const, @@ -142,6 +164,7 @@ describe('aktivitaeten page', () => { it('renders without crashing when filter is set to a non-default value', async () => { render(AktivitaetenPage, { + context: notificationContext(), props: { data: baseData({ filter: 'transkription' as const }) } }); -- 2.49.1 From f13999d006cabec3d734a5ed1a8eebdb02d498f2 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 3 Jun 2026 07:52:37 +0200 Subject: [PATCH 15/17] fix(test): load shared mocks via vi.hoisted, not a static import MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts | 2 +- frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts | 2 +- frontend/src/lib/notification/NotificationBell.svelte.spec.ts | 3 ++- .../src/lib/notification/NotificationDropdown.svelte.test.ts | 2 +- frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts | 2 +- 5 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts index 41f1f277..c5c26cf7 100644 --- a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts +++ b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts @@ -4,7 +4,7 @@ import { page, userEvent } from 'vitest/browser'; import ChronikFuerDichBox from './ChronikFuerDichBox.svelte'; import type { NotificationItem } from '$lib/notification/notifications.svelte'; -import * as formsMock from '$mocks/$app/forms'; +const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); vi.mock('$app/forms', () => ({ ...formsMock })); diff --git a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts index 022f598c..f9386848 100644 --- a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts +++ b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts @@ -3,7 +3,7 @@ import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; import ChronikFuerDichBox from './ChronikFuerDichBox.svelte'; import type { NotificationItem } from '$lib/notification/notifications'; -import * as formsMock from '$mocks/$app/forms'; +const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); vi.mock('$app/forms', () => ({ ...formsMock })); diff --git a/frontend/src/lib/notification/NotificationBell.svelte.spec.ts b/frontend/src/lib/notification/NotificationBell.svelte.spec.ts index 256ee342..d325bb58 100644 --- a/frontend/src/lib/notification/NotificationBell.svelte.spec.ts +++ b/frontend/src/lib/notification/NotificationBell.svelte.spec.ts @@ -2,9 +2,10 @@ import { afterEach, beforeEach, describe, it, expect, vi } from 'vitest'; import { cleanup, render } from 'vitest-browser-svelte'; import { m } from '$lib/paraglide/messages.js'; import type { NotificationItem } from '$lib/notification/notifications'; -import * as formsMock from '$mocks/$app/forms'; import NotificationFixture from './notification.test-fixture.svelte'; +const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); + vi.mock('$app/navigation', () => ({ goto: vi.fn(), beforeNavigate: vi.fn() })); vi.mock('$app/forms', () => ({ ...formsMock })); diff --git a/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts b/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts index 7c302562..0ca8e79b 100644 --- a/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts +++ b/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts @@ -3,7 +3,7 @@ import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; import { goto } from '$app/navigation'; import NotificationDropdown from './NotificationDropdown.svelte'; -import * as formsMock from '$mocks/$app/forms'; +const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); vi.mock('$app/navigation', () => ({ goto: vi.fn() })); diff --git a/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts b/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts index 48cc1f08..2926d6e3 100644 --- a/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts +++ b/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import * as navMock from '$mocks/$app/navigation'; +const navMock = await vi.hoisted(() => import('$mocks/$app/navigation')); vi.mock('$app/navigation', () => ({ ...navMock })); -- 2.49.1 From cdfcb0290399acf952b455b2924f84197453d8ca Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 3 Jun 2026 08:21:02 +0200 Subject: [PATCH 16/17] =?UTF-8?q?revert(test):=20abandon=20shared-mock=20d?= =?UTF-8?q?edup=20=E2=80=94=20infeasible=20in=20vitest=20browser=20mode?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- docs/adr/012-browser-test-mocking-strategy.md | 17 ++++- frontend/src/__meta__/no-factory-ban.test.ts | 14 +++-- frontend/src/__mocks__/$app/forms.ts | 43 ------------- frontend/src/__mocks__/$app/navigation.ts | 62 ------------------- .../ChronikFuerDichBox.svelte.spec.ts | 30 ++++++++- .../ChronikFuerDichBox.svelte.test.ts | 30 ++++++++- .../NotificationBell.svelte.spec.ts | 13 +++- .../NotificationDropdown.svelte.test.ts | 34 ++++++++-- .../hooks/useUnsavedWarning.svelte.test.ts | 31 ++++++++-- frontend/svelte.config.js | 5 -- frontend/vite.config.ts | 7 --- frontend/vitest.client-coverage.config.ts | 7 --- 12 files changed, 143 insertions(+), 150 deletions(-) delete mode 100644 frontend/src/__mocks__/$app/forms.ts delete mode 100644 frontend/src/__mocks__/$app/navigation.ts diff --git a/docs/adr/012-browser-test-mocking-strategy.md b/docs/adr/012-browser-test-mocking-strategy.md index 05656bb9..029c1e77 100644 --- a/docs/adr/012-browser-test-mocking-strategy.md +++ b/docs/adr/012-browser-test-mocking-strategy.md @@ -145,16 +145,27 @@ PR #657 attempted to delete `vi.mock` factories entirely and rely on Vitest auto 7. **In-suite no-factory-ban meta-test** at `frontend/src/__meta__/no-factory-ban.test.ts` — same source-scan mechanism as the other meta-tests; fails if any browser spec contains a `vi.mock('mod')` with no second argument. -### Sanctioned dedup: shared mock body + per-spec sync factory +### Cross-file sharing of a virtual-module mock body is infeasible (the third false premise) -To remove duplicated factory bodies without removing the factory, keep one shared mock module per virtual module under `src/__mocks__/` and import it via the `$mocks` alias into a sync factory: +The original #560 plan ("Option A") proposed deduplicating the non-trivial interceptor factories by importing a shared body from `src/__mocks__/` into a sync factory: ```ts import * as formsMock from "$mocks/$app/forms"; vi.mock("$app/forms", () => ({ ...formsMock })); ``` -The shared module owns any non-trivial mock logic and embeds its own `beforeEach` reset of mutable state, so isolation is structural. The `$mocks` alias is declared in **both** `vite.config.ts` and `vitest.client-coverage.config.ts` so it resolves in the coverage job too. Only genuinely-shared logic is consolidated; the ~80 trivial inline factories (`enhance: () => () => {}`, `{ goto: vi.fn() }`) are left untouched. +**CI proved this does not work in `@vitest/browser-playwright` 4.1.6**, across two runs: + +1. The static-import form above fails at runtime — vitest hoists `vi.mock` _above_ the import, so the factory references an uninitialised binding: `vi.mock factory: make sure there are no top level variables inside, since this call is hoisted`. +2. The documented escape, loading the body through an async hoisted import, fails to even parse in browser mode — vitest's hoist transform mangles it: `SyntaxError: Unexpected identifier 'vi'`. + + ```ts + const formsMock = await vi.hoisted(() => import("$mocks/$app/forms")); // parse error in browser mode + ``` + +`vi.hoisted` has the _same_ constraint as `vi.mock` (its factory can't reference top-level imports either, since it too is hoisted above them), so there is no way to get an external module's body into the hoisted context here. **Therefore: do not share virtual-module mock bodies across spec files. Define each `vi.mock` factory inline, with a synchronous body.** Duplicating the handful of interceptor factories is the accepted cost — it is the only pattern that works. The `src/__mocks__/$app/*` modules and the `$mocks` alias added for Option A were removed. (Revisit on a newer `@vitest/browser-playwright` whose hoist transform handles async `vi.hoisted` imports.) + +The no-factory-ban above still stands: every `vi.mock` of a virtual module must pass an _inline_ sync factory — never no factory, never a spread of an imported binding. ### Rejected: Option C (config-level auto-resolve) diff --git a/frontend/src/__meta__/no-factory-ban.test.ts b/frontend/src/__meta__/no-factory-ban.test.ts index 10efcd64..235c8d46 100644 --- a/frontend/src/__meta__/no-factory-ban.test.ts +++ b/frontend/src/__meta__/no-factory-ban.test.ts @@ -11,9 +11,10 @@ import { fileURLToPath } from 'url'; // implementation (replaceState, which delegates through a getter). The result // is a partial mock that crashes when an unsubstituted export is hit. // -// The sanctioned dedup pattern keeps the factory and shares its body: -// import * as formsMock from '$mocks/$app/forms'; -// vi.mock('$app/forms', () => ({ ...formsMock })); +// The sanctioned form keeps an INLINE sync factory: +// vi.mock('$app/forms', () => ({ enhance(node, submit) { ... } })); +// (Sharing the body via a module imported into the factory is infeasible in +// browser mode — vitest hoists vi.mock above the import; see ADR-012.) // // ESLint and the CI grep guard catch the pattern earlier; this in-suite test // catches it at every vitest invocation — the layer hardest to disable. It @@ -60,9 +61,10 @@ describe('scan: hasNoFactoryViMock', () => { ); }); - it('does not flag a vi.mock with a shared-body spread factory', () => { - const fixture = `import * as formsMock from '$mocks/$app/forms'; - vi.mock('$app/forms', () => ({ ...formsMock }));`; + it('does not flag a vi.mock with a multi-line inline factory', () => { + const fixture = `vi.mock('$app/forms', () => ({ + enhance: (node, submit) => ({ destroy() {} }) + }));`; expect(hasNoFactoryViMock(fixture)).toBe(false); }); diff --git a/frontend/src/__mocks__/$app/forms.ts b/frontend/src/__mocks__/$app/forms.ts deleted file mode 100644 index 2c8915dc..00000000 --- a/frontend/src/__mocks__/$app/forms.ts +++ /dev/null @@ -1,43 +0,0 @@ -// Shared browser-test mock body for the SvelteKit `$app/forms` virtual module. -// -// Imported into a sync vi.mock factory via the $mocks alias: -// import * as formsMock from '$mocks/$app/forms'; -// vi.mock('$app/forms', () => ({ ...formsMock })); -// -// `enhance` intercepts the form's submit event, invokes the component's -// SubmitFunction, and — when that returns a post-submit callback — calls it -// with the configurable `_formResult`. Tests drive the success/failure branch -// with `setFormResult({ type: 'failure' })`. The embedded `beforeEach` resets -// the result before every test, so isolation is structural, not per-spec. -// See ADR-012. - -import { beforeEach } from 'vitest'; - -export type FormEnhanceResult = { type: string; data?: Record }; - -let _formResult: FormEnhanceResult = { type: 'success' }; - -export function setFormResult(result: FormEnhanceResult): void { - _formResult = result; -} - -export function enhance( - node: HTMLFormElement, - submit?: (opts: { - formData: FormData; - }) => (opts: { result: FormEnhanceResult; update: () => Promise }) => Promise -): { destroy: () => void } { - const handler = async (e: Event) => { - e.preventDefault(); - const callback = submit?.({ formData: new FormData(node) } as never); - if (typeof callback === 'function') { - await callback({ result: _formResult, update: async () => {} }); - } - }; - node.addEventListener('submit', handler); - return { destroy: () => node.removeEventListener('submit', handler) }; -} - -beforeEach(() => { - _formResult = { type: 'success' }; -}); diff --git a/frontend/src/__mocks__/$app/navigation.ts b/frontend/src/__mocks__/$app/navigation.ts deleted file mode 100644 index 2421289e..00000000 --- a/frontend/src/__mocks__/$app/navigation.ts +++ /dev/null @@ -1,62 +0,0 @@ -// Shared browser-test mock body for the SvelteKit `$app/navigation` virtual module. -// -// Imported into a sync vi.mock factory via the $mocks alias: -// import * as navMock from '$mocks/$app/navigation'; -// vi.mock('$app/navigation', () => ({ ...navMock })); -// -// All navigation functions are vi.fn() stubs. `beforeNavigate` additionally -// captures the registered callback so a test can drive it through the exported -// `simulateNavigate(href)` helper — the whole capture-and-fire pattern lives -// here, not the raw callback. The embedded `beforeEach` clears the captured -// callback and the mock call histories before every test, so isolation is -// structural. See ADR-012. - -import { beforeEach, vi } from 'vitest'; - -type BeforeNavigateCallback = (nav: { - cancel: () => void; - to: { url: { href: string } } | null; -}) => void; - -let _registeredBeforeNavigate: BeforeNavigateCallback | null = null; - -export const goto = vi.fn(); -export const invalidate = vi.fn(); -export const invalidateAll = vi.fn(); -export const beforeNavigate = vi.fn((fn: BeforeNavigateCallback) => { - _registeredBeforeNavigate = fn; -}); -export const afterNavigate = vi.fn(); -export const preloadCode = vi.fn(); -export const preloadData = vi.fn(); -export const pushState = vi.fn(); -export const replaceState = vi.fn(); -export const disableScrollHandling = vi.fn(); -export const onNavigate = vi.fn(); - -const _navMocks = [ - goto, - invalidate, - invalidateAll, - beforeNavigate, - afterNavigate, - preloadCode, - preloadData, - pushState, - replaceState, - disableScrollHandling, - onNavigate -]; - -// Fire the captured beforeNavigate callback as if navigating to `href`. -// Returns the cancel spy so the test can assert whether navigation was blocked. -export function simulateNavigate(href: string | null = '/somewhere'): ReturnType { - const cancel = vi.fn(); - _registeredBeforeNavigate?.({ cancel, to: href ? { url: { href } } : null }); - return cancel; -} - -beforeEach(() => { - _registeredBeforeNavigate = null; - _navMocks.forEach((mock) => mock.mockClear()); -}); diff --git a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts index c5c26cf7..8ffd6713 100644 --- a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts +++ b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.spec.ts @@ -4,12 +4,36 @@ import { page, userEvent } from 'vitest/browser'; import ChronikFuerDichBox from './ChronikFuerDichBox.svelte'; import type { NotificationItem } from '$lib/notification/notifications.svelte'; -const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); -vi.mock('$app/forms', () => ({ ...formsMock })); +const mockFormResult = vi.hoisted(() => ({ type: 'success' as string })); + +vi.mock('$app/forms', () => ({ + enhance( + node: HTMLFormElement, + submit?: (opts: { + formData: FormData; + }) => (opts: { + result: { type: string; data?: Record }; + update: () => Promise; + }) => Promise + ) { + const handler = async (e: Event) => { + e.preventDefault(); + const cb = submit?.({ formData: new FormData(node) } as never); + if (typeof cb === 'function') { + await ( + cb as (o: { result: typeof mockFormResult; update: () => Promise }) => Promise + )({ result: mockFormResult, update: async () => {} }); + } + }; + node.addEventListener('submit', handler); + return { destroy: () => node.removeEventListener('submit', handler) }; + } +})); afterEach(() => { cleanup(); + mockFormResult.type = 'success'; }); function notif(partial: Partial): NotificationItem { @@ -152,7 +176,7 @@ describe('ChronikFuerDichBox', () => { }); it('shows an accessible error banner when the dismiss action returns a failure', async () => { - formsMock.setFormResult({ type: 'failure' }); + mockFormResult.type = 'failure'; render(ChronikFuerDichBox, { unread: [notif({ id: 'err-1' })], optimisticMarkRead: vi.fn(), diff --git a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts index f9386848..2ee844e3 100644 --- a/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts +++ b/frontend/src/lib/activity/ChronikFuerDichBox.svelte.test.ts @@ -3,12 +3,36 @@ import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; import ChronikFuerDichBox from './ChronikFuerDichBox.svelte'; import type { NotificationItem } from '$lib/notification/notifications'; -const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); -vi.mock('$app/forms', () => ({ ...formsMock })); +const mockFormResult = vi.hoisted(() => ({ type: 'success' as string })); + +vi.mock('$app/forms', () => ({ + enhance( + node: HTMLFormElement, + submit?: (opts: { + formData: FormData; + }) => (opts: { + result: { type: string; data?: Record }; + update: () => Promise; + }) => Promise + ) { + const handler = async (e: Event) => { + e.preventDefault(); + const cb = submit?.({ formData: new FormData(node) } as never); + if (typeof cb === 'function') { + await ( + cb as (o: { result: typeof mockFormResult; update: () => Promise }) => Promise + )({ result: mockFormResult, update: async () => {} }); + } + }; + node.addEventListener('submit', handler); + return { destroy: () => node.removeEventListener('submit', handler) }; + } +})); afterEach(() => { cleanup(); + mockFormResult.type = 'success'; }); const mention = (overrides: Partial = {}): NotificationItem => ({ @@ -136,7 +160,7 @@ describe('ChronikFuerDichBox', () => { }); it('shows an accessible error banner when the dismiss action returns a failure', async () => { - formsMock.setFormResult({ type: 'failure' }); + mockFormResult.type = 'failure'; render(ChronikFuerDichBox, { props: { unread: [mention({ id: 'err-1' })], diff --git a/frontend/src/lib/notification/NotificationBell.svelte.spec.ts b/frontend/src/lib/notification/NotificationBell.svelte.spec.ts index d325bb58..28c09577 100644 --- a/frontend/src/lib/notification/NotificationBell.svelte.spec.ts +++ b/frontend/src/lib/notification/NotificationBell.svelte.spec.ts @@ -4,10 +4,17 @@ import { m } from '$lib/paraglide/messages.js'; import type { NotificationItem } from '$lib/notification/notifications'; import NotificationFixture from './notification.test-fixture.svelte'; -const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); - vi.mock('$app/navigation', () => ({ goto: vi.fn(), beforeNavigate: vi.fn() })); -vi.mock('$app/forms', () => ({ ...formsMock })); +vi.mock('$app/forms', () => ({ + enhance(node: HTMLFormElement, submit?: (opts: { formData: FormData }) => unknown) { + const handler = (e: Event) => { + e.preventDefault(); + submit?.({ formData: new FormData(node) } as never); + }; + node.addEventListener('submit', handler); + return { destroy: () => node.removeEventListener('submit', handler) }; + } +})); // NotificationBell.onMount calls store.init(), which opens an EventSource and // fetches the unread count. Stub both so no real network or 401 → /login diff --git a/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts b/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts index 0ca8e79b..bdbda7eb 100644 --- a/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts +++ b/frontend/src/lib/notification/NotificationDropdown.svelte.test.ts @@ -3,15 +3,41 @@ import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; import { goto } from '$app/navigation'; import NotificationDropdown from './NotificationDropdown.svelte'; -const formsMock = await vi.hoisted(() => import('$mocks/$app/forms')); vi.mock('$app/navigation', () => ({ goto: vi.fn() })); -vi.mock('$app/forms', () => ({ ...formsMock })); +// Configurable result for the enhance mock — tests that need failure set +// mockFormResult.type = 'failure' before clicking. +const mockFormResult = vi.hoisted(() => ({ type: 'success' as string })); + +// Invoke the SubmitFunction and always call the returned result callback with +// mockFormResult so tests can exercise both success and failure branches. +vi.mock('$app/forms', () => ({ + enhance( + node: HTMLFormElement, + submit?: (opts: { + formData: FormData; + }) => (opts: { + result: { type: string; data?: Record }; + update: () => Promise; + }) => Promise + ) { + const handler = async (e: Event) => { + e.preventDefault(); + const cb = submit?.({ formData: new FormData(node) } as never); + if (typeof cb === 'function') { + await cb({ result: mockFormResult, update: async () => {} } as never); + } + }; + node.addEventListener('submit', handler); + return { destroy: () => node.removeEventListener('submit', handler) }; + } +})); afterEach(() => { cleanup(); vi.clearAllMocks(); + mockFormResult.type = 'success'; // reset to default after each test }); const makeNotification = (overrides: Record = {}) => ({ @@ -209,7 +235,7 @@ describe('NotificationDropdown', () => { }); it('shows a role=alert error banner when mark-all-read returns a failure', async () => { - formsMock.setFormResult({ type: 'failure' }); + mockFormResult.type = 'failure'; render(NotificationDropdown, { props: { notifications: [makeNotification()], @@ -346,7 +372,7 @@ describe('NotificationDropdown', () => { }); it('does NOT call onClose or goto when the dismiss action returns a failure', async () => { - formsMock.setFormResult({ type: 'failure' }); + mockFormResult.type = 'failure'; const onClose = vi.fn(); const n = makeNotification({ id: 'n99', actorName: 'Bob' }); render(NotificationDropdown, { diff --git a/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts b/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts index 2926d6e3..d85dc4c3 100644 --- a/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts +++ b/frontend/src/lib/shared/hooks/useUnsavedWarning.svelte.test.ts @@ -1,12 +1,35 @@ -import { describe, it, expect, vi } from 'vitest'; -const navMock = await vi.hoisted(() => import('$mocks/$app/navigation')); +import { describe, it, expect, vi, beforeEach } from 'vitest'; -vi.mock('$app/navigation', () => ({ ...navMock })); +// Capture the beforeNavigate callback so tests can simulate navigation events +let registeredBeforeNavigate: + | ((nav: { cancel: () => void; to: { url: { href: string } } | null }) => void) + | null = null; -const { simulateNavigate, goto: mockGoto } = navMock; +const mockGoto = vi.fn(); + +vi.mock('$app/navigation', () => ({ + beforeNavigate: vi.fn((fn: typeof registeredBeforeNavigate) => { + registeredBeforeNavigate = fn; + }), + goto: mockGoto +})); const { createUnsavedWarning } = await import('./useUnsavedWarning.svelte'); +function simulateNavigate(href: string | null = '/somewhere') { + const cancel = vi.fn(); + registeredBeforeNavigate?.({ + cancel, + to: href ? { url: { href } } : null + }); + return cancel; +} + +beforeEach(() => { + registeredBeforeNavigate = null; + mockGoto.mockClear(); +}); + describe('createUnsavedWarning', () => { it('isDirty starts false', () => { const w = createUnsavedWarning(); diff --git a/frontend/svelte.config.js b/frontend/svelte.config.js index 3ca6f79a..e94e293c 100644 --- a/frontend/svelte.config.js +++ b/frontend/svelte.config.js @@ -8,11 +8,6 @@ const config = { preprocess: vitePreprocess(), kit: { adapter: adapter(), - // $mocks resolves shared browser-test mock bodies (src/__mocks__). Declared here - // so svelte-check/tsconfig and both vite configs resolve it. See ADR-012. - alias: { - $mocks: 'src/__mocks__' - }, prerender: { entries: ['/hilfe/transkription'], // Disable crawl: by default SvelteKit follows nav links from diff --git a/frontend/vite.config.ts b/frontend/vite.config.ts index 18240c74..a0d1e977 100644 --- a/frontend/vite.config.ts +++ b/frontend/vite.config.ts @@ -6,15 +6,8 @@ import { defineConfig } from 'vitest/config'; import { playwright } from '@vitest/browser-playwright'; import { sveltekit } from '@sveltejs/kit/vite'; import { viteStaticCopy } from 'vite-plugin-static-copy'; -import { fileURLToPath } from 'node:url'; export default defineConfig({ - resolve: { - alias: { - // Shared browser-test mock bodies, imported into sync vi.mock factories. See ADR-012. - $mocks: fileURLToPath(new URL('./src/__mocks__', import.meta.url)) - } - }, optimizeDeps: { include: ['pdfjs-dist', '@tiptap/core', '@tiptap/starter-kit', '@tiptap/extension-mention'] }, diff --git a/frontend/vitest.client-coverage.config.ts b/frontend/vitest.client-coverage.config.ts index b7b1ad57..dc822281 100644 --- a/frontend/vitest.client-coverage.config.ts +++ b/frontend/vitest.client-coverage.config.ts @@ -4,7 +4,6 @@ import tailwindcss from '@tailwindcss/vite'; import { defineConfig } from 'vitest/config'; import { playwright } from '@vitest/browser-playwright'; import { sveltekit } from '@sveltejs/kit/vite'; -import { fileURLToPath } from 'node:url'; // Standalone config for browser-project Istanbul coverage. // Uses a dedicated root-level coverage block because Vitest 4 ignores @@ -12,12 +11,6 @@ import { fileURLToPath } from 'node:url'; // Plugins mirrored from vite.config.ts: tailwindcss, sveltekit, devtoolsJson, paraglideVitePlugin // Update here whenever vite.config.ts plugins change. export default defineConfig({ - resolve: { - alias: { - // Shared browser-test mock bodies, imported into sync vi.mock factories. See ADR-012. - $mocks: fileURLToPath(new URL('./src/__mocks__', import.meta.url)) - } - }, optimizeDeps: { include: ['pdfjs-dist', '@tiptap/core', '@tiptap/starter-kit', '@tiptap/extension-mention'] }, -- 2.49.1 From 30a8e36563d489e9fe3fa18f4f4630502f038e54 Mon Sep 17 00:00:00 2001 From: Marcel Date: Wed, 3 Jun 2026 10:32:00 +0200 Subject: [PATCH 17/17] test(notification): make setNotifications authoritative in bell a11y tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- .../NotificationBell.svelte.spec.ts | 27 ++++++++++--------- 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/frontend/src/lib/notification/NotificationBell.svelte.spec.ts b/frontend/src/lib/notification/NotificationBell.svelte.spec.ts index 28c09577..6b406258 100644 --- a/frontend/src/lib/notification/NotificationBell.svelte.spec.ts +++ b/frontend/src/lib/notification/NotificationBell.svelte.spec.ts @@ -30,11 +30,12 @@ class NoopEventSource { beforeEach(() => { vi.stubGlobal('EventSource', NoopEventSource); + // init()'s fetchUnreadCount() never settles, so the bell's announced count is + // driven solely by setNotifications() — removes the init-fetch-vs-setNotifications + // race (the real fetch would resolve to {count:0} and clobber the seeded count). vi.stubGlobal( 'fetch', - vi - .fn() - .mockResolvedValue(new Response(JSON.stringify({ count: 0, content: [] }), { status: 200 })) + vi.fn(() => new Promise(() => {})) ); }); @@ -89,7 +90,6 @@ describe('NotificationBell — rendering', () => { describe('NotificationBell — announced unread count across a11y states', () => { it('empty: announces no unread count and hides the live badge', async () => { const { setNotifications } = renderBell(); - await tick(); setNotifications([]); await tick(); @@ -101,7 +101,6 @@ describe('NotificationBell — announced unread count across a11y states', () => it('single: announces one unread notification', async () => { const { setNotifications } = renderBell(); - await tick(); setNotifications([makeNotification({ id: 'n1', read: false })]); await tick(); @@ -114,7 +113,6 @@ describe('NotificationBell — announced unread count across a11y states', () => it('many: announces the exact unread count', async () => { const { setNotifications } = renderBell(); - await tick(); setNotifications([ makeNotification({ id: 'n1', read: false }), makeNotification({ id: 'n2', read: false }), @@ -128,16 +126,21 @@ describe('NotificationBell — announced unread count across a11y states', () => expect(unreadBadge().textContent?.trim()).toBe('3'); }); - it('error: degrades to a zero announced count when the unread-count load fails', async () => { + it('error: a failed unread-count load does not wipe the announced count', async () => { vi.stubGlobal('fetch', vi.fn().mockRejectedValue(new Error('network down'))); - renderBell(); + const { setNotifications } = renderBell(); + setNotifications([ + makeNotification({ id: 'n1', read: false }), + makeNotification({ id: 'n2', read: false }) + ]); await tick(); - // A failed load leaves the count at 0; the bell still announces a valid, - // non-urgent state rather than a broken count. + // init()'s fetchUnreadCount rejects; its catch must leave the already-set + // count intact rather than silently zeroing the live region. This is a + // distinct state from "empty" — the count survived a transient load failure. const btn = bellButton(); - expect(btn.getAttribute('title')).toBe(m.notification_bell_label()); + expect(btn.getAttribute('title')).toBe(m.notification_bell_unread_label({ count: 2 })); expect(btn.getAttribute('aria-label')).toBe(btn.getAttribute('title')); - expect(unreadBadge().classList.contains('hidden')).toBe(true); + expect(unreadBadge().textContent?.trim()).toBe('2'); }); }); -- 2.49.1