From 23cbb6be224f74d55cd640a122acb44147542362 Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 13:01:50 +0200 Subject: [PATCH 1/2] =?UTF-8?q?test(pdf-renderer):=20eliminate=20real=20pd?= =?UTF-8?q?fjs-dist=20loading=20from=20browser=20tests=20=E2=80=94=20use?= =?UTF-8?q?=20fake=20libLoader=20for=20all=20init()=20calls?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five tests in usePdfRenderer.svelte.test.ts called createPdfRenderer() without a libLoader, causing init() to dynamically import pdfjs-dist in the browser. Every dynamic import goes through Playwright's route handler, which calls resolveManualMock via birpc to check for mocks. If the RPC closes during teardown while one of these imports is in flight, the birpc race fires — even though pdfjs-dist was never explicitly vi.mock()-ed. Replace all bare createPdfRenderer() calls that invoke init() with createPdfRenderer(makeFakeLibLoader()), identical to the pattern already used in PdfViewer.svelte.test.ts. No real module loads, no route-handler calls, no birpc exposure. Co-Authored-By: Claude Sonnet 4.6 --- .../viewer/usePdfRenderer.svelte.test.ts | 40 +++++++++++++------ 1 file changed, 28 insertions(+), 12 deletions(-) diff --git a/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts b/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts index 7bdf387d..2a1c9f90 100644 --- a/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts +++ b/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts @@ -1,6 +1,25 @@ import { describe, it, expect, vi } from 'vitest'; +import type { LibLoader } from './usePdfRenderer.svelte'; import { createPdfRenderer } from './usePdfRenderer.svelte'; +function makeFakeLibLoader(): LibLoader { + return vi.fn().mockResolvedValue([ + { + GlobalWorkerOptions: { workerSrc: '' }, + getDocument: vi.fn().mockReturnValue({ + promise: Promise.resolve({ numPages: 1, getPage: vi.fn() }) + }), + TextLayer: class { + render() { + return Promise.resolve(); + } + cancel() {} + } + } as unknown as typeof import('pdfjs-dist'), + { default: '' } + ] as const); +} + // Note: init() and loadDocument() require pdfjsLib (browser module). // These tests cover pure state logic only — bounds clamping and zoom limits. @@ -122,39 +141,36 @@ describe('createPdfRenderer', () => { expect(r.scale).toBe(before); }); - it('init() is callable and resolves without throwing in browser env', async () => { - const r = createPdfRenderer(); + it('init() sets pdfjsReady to true when loader resolves', async () => { + const r = createPdfRenderer(makeFakeLibLoader()); await expect(r.init()).resolves.toBeUndefined(); - // pdfjsReady is now true expect(r.pdfjsReady).toBe(true); }); - it('after init, loadDocument with a bogus URL sets error', async () => { - const r = createPdfRenderer(); + it('after init, loadDocument completes and loading returns to false', async () => { + const r = createPdfRenderer(makeFakeLibLoader()); await r.init(); - await r.loadDocument('about:invalid-pdf'); - // Either error is set or loading flips back to false — both are acceptable + await r.loadDocument('/some/path'); expect(r.loading).toBe(false); }); it('renderCurrentPage is a no-op when canvasEl is null but pdfjsLib is initialized', async () => { - const r = createPdfRenderer(); + const r = createPdfRenderer(makeFakeLibLoader()); await r.init(); // Without setElements, canvasEl is null — early return await expect(r.renderCurrentPage()).resolves.toBeUndefined(); }); it('renderCurrentPage is a no-op when textLayerEl is null', async () => { - const r = createPdfRenderer(); + const r = createPdfRenderer(makeFakeLibLoader()); await r.init(); - // Set only canvas, leave textLayer unset is not directly testable; - // confirm calling without elements wired returns early. + // Without setElements, textLayerEl is null — early return await expect(r.renderCurrentPage()).resolves.toBeUndefined(); }); it('init() can be called multiple times safely', async () => { - const r = createPdfRenderer(); + const r = createPdfRenderer(makeFakeLibLoader()); await r.init(); await r.init(); expect(r.pdfjsReady).toBe(true); -- 2.49.1 From d21ba8fed244b2bc80086d7f7948f3db2a204b3e Mon Sep 17 00:00:00 2001 From: Marcel Date: Tue, 12 May 2026 15:19:37 +0200 Subject: [PATCH 2/2] refactor(pdf-viewer-tests): extract shared fake, add loadDocument error path, fix assertions MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Extract makeFakePdfjsLib / makeFakeLibLoader to testHelpers.ts — single source of truth used by both PdfViewer.svelte.test.ts and usePdfRenderer.svelte.test.ts; removes the diverging-fidelity DRY violation flagged by @felixbrandt and @saraholt in the PR review - Add 'loadDocument sets error and loading=false when getDocument().promise rejects' test to usePdfRenderer.svelte.test.ts — closes the error-path gap flagged by @felixbrandt and @saraholt - Replace toBeInTheDocument() with toBeVisible() in the three absorbed spec-file tests — uniform assertion style across the loaded-state describe block, as flagged by @felixbrandt Co-Authored-By: Claude Sonnet 4.6 --- .../document/viewer/PdfViewer.svelte.test.ts | 41 +++---------------- .../src/lib/document/viewer/testHelpers.ts | 31 ++++++++++++++ .../viewer/usePdfRenderer.svelte.test.ts | 40 +++++++++--------- 3 files changed, 58 insertions(+), 54 deletions(-) create mode 100644 frontend/src/lib/document/viewer/testHelpers.ts diff --git a/frontend/src/lib/document/viewer/PdfViewer.svelte.test.ts b/frontend/src/lib/document/viewer/PdfViewer.svelte.test.ts index a4b49ba9..17d1827c 100644 --- a/frontend/src/lib/document/viewer/PdfViewer.svelte.test.ts +++ b/frontend/src/lib/document/viewer/PdfViewer.svelte.test.ts @@ -1,40 +1,11 @@ import { describe, it, expect, vi, afterEach } from 'vitest'; import { cleanup, render } from 'vitest-browser-svelte'; import { page } from 'vitest/browser'; -import type { LibLoader } from '$lib/document/viewer/usePdfRenderer.svelte'; import PdfViewer from './PdfViewer.svelte'; +import { makeFakeLibLoader } from './testHelpers'; afterEach(cleanup); -function makeFakePdfjsLib() { - class TextLayerMock { - render() { - return Promise.resolve(); - } - cancel() {} - } - - return { - GlobalWorkerOptions: { workerSrc: '' }, - getDocument: vi.fn().mockReturnValue({ - promise: Promise.resolve({ - numPages: 2, - getPage: vi.fn().mockResolvedValue({ - getViewport: vi.fn().mockReturnValue({ width: 595, height: 842 }), - render: vi.fn().mockReturnValue({ promise: Promise.resolve() }), - streamTextContent: vi.fn().mockReturnValue(new ReadableStream()) - }) - }) - }), - TextLayer: TextLayerMock - } as unknown as typeof import('pdfjs-dist'); -} - -function makeFakeLibLoader(): LibLoader { - const fakePdfjs = makeFakePdfjsLib(); - return vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const); -} - describe('PdfViewer — empty / error states', () => { it('renders the no-file placeholder when url is empty', async () => { render(PdfViewer, { url: '', libLoader: makeFakeLibLoader() }); @@ -293,18 +264,18 @@ describe('PdfViewer — loaded state', () => { it('shows previous and next page navigation buttons', async () => { render(PdfViewer, { url: '/api/documents/test-id/file', libLoader: makeFakeLibLoader() }); - await expect.element(page.getByRole('button', { name: /zurück/i })).toBeInTheDocument(); - await expect.element(page.getByRole('button', { name: /weiter/i })).toBeInTheDocument(); + await expect.element(page.getByRole('button', { name: /zurück/i })).toBeVisible(); + await expect.element(page.getByRole('button', { name: /weiter/i })).toBeVisible(); }); it('shows zoom controls', async () => { render(PdfViewer, { url: '/api/documents/test-id/file', libLoader: makeFakeLibLoader() }); - await expect.element(page.getByRole('button', { name: /vergrößern/i })).toBeInTheDocument(); - await expect.element(page.getByRole('button', { name: /verkleinern/i })).toBeInTheDocument(); + await expect.element(page.getByRole('button', { name: /vergrößern/i })).toBeVisible(); + await expect.element(page.getByRole('button', { name: /verkleinern/i })).toBeVisible(); }); it('displays the page counter once the PDF has loaded', async () => { render(PdfViewer, { url: '/api/documents/test-id/file', libLoader: makeFakeLibLoader() }); - await expect.element(page.getByText(/1\s*\/\s*2/)).toBeInTheDocument(); + await expect.element(page.getByText(/1\s*\/\s*2/)).toBeVisible(); }); }); diff --git a/frontend/src/lib/document/viewer/testHelpers.ts b/frontend/src/lib/document/viewer/testHelpers.ts new file mode 100644 index 00000000..f7e5c295 --- /dev/null +++ b/frontend/src/lib/document/viewer/testHelpers.ts @@ -0,0 +1,31 @@ +import { vi } from 'vitest'; +import type { LibLoader } from './usePdfRenderer.svelte'; + +export function makeFakePdfjsLib() { + class TextLayerMock { + render() { + return Promise.resolve(); + } + cancel() {} + } + + return { + GlobalWorkerOptions: { workerSrc: '' }, + getDocument: vi.fn().mockReturnValue({ + promise: Promise.resolve({ + numPages: 2, + getPage: vi.fn().mockResolvedValue({ + getViewport: vi.fn().mockReturnValue({ width: 595, height: 842 }), + render: vi.fn().mockReturnValue({ promise: Promise.resolve() }), + streamTextContent: vi.fn().mockReturnValue(new ReadableStream()) + }) + }) + }), + TextLayer: TextLayerMock + } as unknown as typeof import('pdfjs-dist'); +} + +export function makeFakeLibLoader(): LibLoader { + const fakePdfjs = makeFakePdfjsLib(); + return vi.fn().mockResolvedValue([fakePdfjs, { default: '' }] as const); +} diff --git a/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts b/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts index 2a1c9f90..6b48c3f8 100644 --- a/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts +++ b/frontend/src/lib/document/viewer/usePdfRenderer.svelte.test.ts @@ -1,24 +1,6 @@ import { describe, it, expect, vi } from 'vitest'; -import type { LibLoader } from './usePdfRenderer.svelte'; import { createPdfRenderer } from './usePdfRenderer.svelte'; - -function makeFakeLibLoader(): LibLoader { - return vi.fn().mockResolvedValue([ - { - GlobalWorkerOptions: { workerSrc: '' }, - getDocument: vi.fn().mockReturnValue({ - promise: Promise.resolve({ numPages: 1, getPage: vi.fn() }) - }), - TextLayer: class { - render() { - return Promise.resolve(); - } - cancel() {} - } - } as unknown as typeof import('pdfjs-dist'), - { default: '' } - ] as const); -} +import { makeFakeLibLoader } from './testHelpers'; // Note: init() and loadDocument() require pdfjsLib (browser module). // These tests cover pure state logic only — bounds clamping and zoom limits. @@ -222,4 +204,24 @@ describe('createPdfRenderer', () => { await r.init(); expect(fakeLoader).toHaveBeenCalledOnce(); }); + + it('loadDocument sets error and loading=false when getDocument().promise rejects', async () => { + const failingLib = { + GlobalWorkerOptions: { workerSrc: '' }, + getDocument: vi.fn().mockReturnValue({ + promise: Promise.reject(new Error('PDF not found')) + }), + TextLayer: class { + render() { + return Promise.resolve(); + } + cancel() {} + } + } as unknown as typeof import('pdfjs-dist'); + const r = createPdfRenderer(vi.fn().mockResolvedValue([failingLib, { default: '' }] as const)); + await r.init(); + await r.loadDocument('/bad/path'); + expect(r.loading).toBe(false); + expect(r.error).toBe('PDF not found'); + }); }); -- 2.49.1