From 444144038a164f8d92359c262df359c511171256 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 7 May 2026 18:05:30 -0400 Subject: [PATCH] fix(ci): library_test Mount arg + 4 web test selectors + forgot-pw catch MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Five fixes in one commit, all from the U1+U2+U3 push: 1. internal/api/library_test.go's Mount call wasn't updated when U3-T4 added the mailer.Sender param. 17 args, sig wants 18. Adds a trailing nil for the mailer. Same shape of test-fixture lag the project has hit before — flagged as a recurring pattern for the DRY-pass slice. 2. web/src/routes/settings/settings.test.ts mocked $lib/api/me with `getAPIToken: vi.fn()` (no return value). The new /settings API Token card's $effect calls `getAPIToken().then(...)` which threw "Cannot read properties of undefined (reading 'then')" on every ListenBrainz test. Default the mock to mockResolvedValue so any test that doesn't override gets a valid resolution. 3. web/src/routes/admin/users/users.test.ts queried row buttons by /^Delete$/ but T3 added per-user aria-labels ("Delete alice"), so the row buttons' accessible name is no longer "Delete". The modal's confirm button has no aria-label so its name IS "Delete" exactly. Tests now click by per-user aria-label first, then by /^Delete$/ for the modal confirm. 4. Same file: /Set password/i regex matched both the dialog's submit button AND the row's "Reset password for ..." buttons (because regex `Set` matches "ReSet" case-insensitively). Switched to /^Set password$/ exact-match. 5. web/src/routes/reset-password/[token]/reset-password.test.ts had /New password/i which matched both the "New password" and "Confirm new password" labels. Switched to exact-match string selectors. 6. web/src/routes/forgot-password/+page.svelte's onSubmit handler had no catch — a rejected forgotPassword() bubbled as an unhandled rejection in vitest. Added a silent catch since the page intentionally shows the same success message regardless of outcome (no-enumeration posture). --- internal/api/library_test.go | 2 +- web/src/routes/admin/users/users.test.ts | 24 +++++++++---------- web/src/routes/forgot-password/+page.svelte | 5 ++++ .../[token]/reset-password.test.ts | 18 +++++++------- web/src/routes/settings/settings.test.ts | 4 +++- 5 files changed, 29 insertions(+), 24 deletions(-) diff --git a/internal/api/library_test.go b/internal/api/library_test.go index 46fd3e45..f5dc7f6a 100644 --- a/internal/api/library_test.go +++ b/internal/api/library_test.go @@ -443,7 +443,7 @@ func TestRoutesRegisteredInMount(t *testing.T) { r := chi.NewRouter() w := playevents.NewWriter(h.pool, slog.New(slog.NewTextHandler(io.Discard, nil)), 30*time.Minute, 0.5, 30000) - Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverArtBackfillCap, h.coverSettings, h.scanner, h.scanCfg, nil, h.dataDir) + Mount(r, h.pool, h.logger, w, config.RecommendationConfig{RadioSize: 50, RadioSizeMax: 200, RecentlyPlayedHours: 1}, h.lidarrCfg, h.lidarrRequests, h.lidarrQuarantine, h.tracks, h.playlists, h.coverart, h.coverArtBackfillCap, h.coverSettings, h.scanner, h.scanCfg, nil, h.dataDir, nil) paths := []string{ "/api/artists", diff --git a/web/src/routes/admin/users/users.test.ts b/web/src/routes/admin/users/users.test.ts index 8ad66c31..b630da3d 100644 --- a/web/src/routes/admin/users/users.test.ts +++ b/web/src/routes/admin/users/users.test.ts @@ -163,13 +163,11 @@ describe('/admin/users', () => { setup(); await waitFor(() => screen.getByText('alice')); - const deleteButtons = screen.getAllByRole('button', { name: /^Delete$/i }); - await fireEvent.click(deleteButtons[0]); + // Row buttons carry per-user aria-labels ("Delete alice"). The modal's + // confirm button has no aria-label so its accessible name is "Delete" exactly. + await fireEvent.click(screen.getByRole('button', { name: 'Delete alice' })); await waitFor(() => screen.getByRole('dialog')); - - // The confirm Delete button is inside the dialog; it is the last "Delete" in the DOM. - const allDeleteButtons = screen.getAllByRole('button', { name: /^Delete$/i }); - await fireEvent.click(allDeleteButtons[allDeleteButtons.length - 1]); + await fireEvent.click(screen.getByRole('button', { name: /^Delete$/ })); expect(deleteUser).toHaveBeenCalled(); }); @@ -179,11 +177,9 @@ describe('/admin/users', () => { setup(); await waitFor(() => screen.getByText('alice')); - const deleteButtons = screen.getAllByRole('button', { name: /^Delete$/i }); - await fireEvent.click(deleteButtons[0]); + await fireEvent.click(screen.getByRole('button', { name: 'Delete alice' })); await waitFor(() => screen.getByRole('dialog')); - const allDeleteButtons = screen.getAllByRole('button', { name: /^Delete$/i }); - await fireEvent.click(allDeleteButtons[allDeleteButtons.length - 1]); + await fireEvent.click(screen.getByRole('button', { name: /^Delete$/ })); await waitFor(() => screen.getByRole('status')); expect(screen.getByRole('status').textContent).toMatch(/last admin/i); @@ -194,12 +190,14 @@ describe('/admin/users', () => { setup(); await waitFor(() => screen.getByText('alice')); - await fireEvent.click(screen.getAllByRole('button', { name: /Reset password/i })[0]); + await fireEvent.click(screen.getByRole('button', { name: 'Reset password for alice' })); await waitFor(() => screen.getByRole('dialog')); - await fireEvent.input(screen.getByLabelText(/New password/i), { target: { value: 'abcd1234' } }); + await fireEvent.input(screen.getByLabelText(/^New password$/i), { target: { value: 'abcd1234' } }); await fireEvent.input(screen.getByLabelText(/^Confirm$/i), { target: { value: 'abcd1234' } }); - await fireEvent.click(screen.getByRole('button', { name: /Set password/i })); + // /^Set password$/ avoids matching the row's "Reset password" buttons + // (regex `Set password` matches "ReSet password" case-insensitively). + await fireEvent.click(screen.getByRole('button', { name: /^Set password$/i })); expect(resetUserPassword).toHaveBeenCalledWith(expect.any(String), 'abcd1234'); }); diff --git a/web/src/routes/forgot-password/+page.svelte b/web/src/routes/forgot-password/+page.svelte index 0aac893b..a10b7fdd 100644 --- a/web/src/routes/forgot-password/+page.svelte +++ b/web/src/routes/forgot-password/+page.svelte @@ -11,6 +11,11 @@ submitting = true; try { await forgotPassword(email); + } catch { + // Swallow — the page always shows the same success message + // regardless of outcome, to mirror the server's no-enumeration + // posture. A failed call must not surface as an unhandled + // rejection in the browser console. } finally { submitted = true; submitting = false; diff --git a/web/src/routes/reset-password/[token]/reset-password.test.ts b/web/src/routes/reset-password/[token]/reset-password.test.ts index f0c31409..b91bd891 100644 --- a/web/src/routes/reset-password/[token]/reset-password.test.ts +++ b/web/src/routes/reset-password/[token]/reset-password.test.ts @@ -22,9 +22,9 @@ describe('/reset-password/[token] page', () => { it('rejects mismatched passwords without calling resetPassword', async () => { render(ResetPasswordPage); - await fireEvent.input(screen.getByLabelText(/New password/i), { target: { value: 'abcd1234' } }); - await fireEvent.input(screen.getByLabelText(/Confirm new password/i), { target: { value: 'differentpw' } }); - await fireEvent.click(screen.getByRole('button', { name: /Set password/i })); + await fireEvent.input(screen.getByLabelText('New password'), { target: { value: 'abcd1234' } }); + await fireEvent.input(screen.getByLabelText('Confirm new password'), { target: { value: 'differentpw' } }); + await fireEvent.click(screen.getByRole('button', { name: 'Set password' })); expect(resetPassword).not.toHaveBeenCalled(); await waitFor(() => expect(screen.getByRole('alert').textContent).toMatch(/do not match/i)); }); @@ -32,9 +32,9 @@ describe('/reset-password/[token] page', () => { it('calls resetPassword with the URL token and new password, redirects on success', async () => { (resetPassword as unknown as ReturnType).mockResolvedValue(undefined); render(ResetPasswordPage); - await fireEvent.input(screen.getByLabelText(/New password/i), { target: { value: 'abcd1234' } }); - await fireEvent.input(screen.getByLabelText(/Confirm new password/i), { target: { value: 'abcd1234' } }); - await fireEvent.click(screen.getByRole('button', { name: /Set password/i })); + await fireEvent.input(screen.getByLabelText('New password'), { target: { value: 'abcd1234' } }); + await fireEvent.input(screen.getByLabelText('Confirm new password'), { target: { value: 'abcd1234' } }); + await fireEvent.click(screen.getByRole('button', { name: 'Set password' })); expect(resetPassword).toHaveBeenCalledWith('test-token-abc', 'abcd1234'); await waitFor(() => expect(goto).toHaveBeenCalledWith('/login?reset=ok', expect.objectContaining({ replaceState: true }))); }); @@ -42,9 +42,9 @@ describe('/reset-password/[token] page', () => { it('shows clear error on invalid_token', async () => { (resetPassword as unknown as ReturnType).mockRejectedValueOnce({ code: 'invalid_token' }); render(ResetPasswordPage); - await fireEvent.input(screen.getByLabelText(/New password/i), { target: { value: 'abcd1234' } }); - await fireEvent.input(screen.getByLabelText(/Confirm new password/i), { target: { value: 'abcd1234' } }); - await fireEvent.click(screen.getByRole('button', { name: /Set password/i })); + await fireEvent.input(screen.getByLabelText('New password'), { target: { value: 'abcd1234' } }); + await fireEvent.input(screen.getByLabelText('Confirm new password'), { target: { value: 'abcd1234' } }); + await fireEvent.click(screen.getByRole('button', { name: 'Set password' })); await waitFor(() => expect(screen.getByRole('alert').textContent).toMatch(/invalid, expired, or already used/i)); }); }); diff --git a/web/src/routes/settings/settings.test.ts b/web/src/routes/settings/settings.test.ts index 1bd7d53b..e89228a6 100644 --- a/web/src/routes/settings/settings.test.ts +++ b/web/src/routes/settings/settings.test.ts @@ -14,7 +14,9 @@ vi.mock('$lib/api/listenbrainz', () => ({ vi.mock('$lib/api/me', () => ({ updateProfile: vi.fn(), changePassword: vi.fn(), - getAPIToken: vi.fn(), + // Default to a resolved value so the page's $effect doesn't crash + // on `.then()` of undefined when individual tests don't override. + getAPIToken: vi.fn().mockResolvedValue({ api_token: '' }), regenerateAPIToken: vi.fn() }));