bvandeusen/minstrel
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 38 Wiki Activity

fix(ci): library_test Mount arg + 4 web test selectors + forgot-pw catch

Browse Source 
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).
This commit is contained in:
Bryan Van Deusen
2026-05-07 18:05:30 -04:00
parent 9ed576a029
commit 444144038a
5 changed files with 29 additions and 24 deletions
Download Patch File Download Diff File Expand all files Collapse all files
internal/api/library_test.go
+1 -1
View File
@@ -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",
web/src/routes/admin/users/users.test.ts
+11 -13
View File
@@ -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');
});
web/src/routes/forgot-password/+page.svelte
+5
View File
@@ -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;
web/src/routes/reset-password/[token]/reset-password.test.ts
+9 -9
View File
@@ -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<typeof vi.fn>).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<typeof vi.fn>).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));
});
});
web/src/routes/settings/settings.test.ts
+3 -1
View File
@@ -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()
}));