From 62222d84388cad7528a8d20547bb8e6070ac26d6 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 6 May 2026 13:55:35 -0400 Subject: [PATCH] fix(server,web/m7-cover-sources): forward-fix CI round 2 Three issues from the previous push: 1. internal/api/admin_covers_test.go used t.Context() (Go 1.24+) but go.mod declares go 1.23.0. Replace with context.Background() in the testHandlersWithCovers helper; add "context" import. 2. internal/server/server_test.go's New() call missed the *coverart.SettingsService argument added in T11. The new param sits between the cover-art-backfill cap (int) and the *library.Scanner pointer in the function signature; the test was using 12 args instead of 13. Insert nil at the right position in all six call sites. 3. integrations.test.ts had 11 failing tests after T13 added the cover-art-providers section. Both panels render "Save changes", "Test connection", and "API key" controls, so the existing Lidarr tests' role/text queries matched multiple elements. Add helper functions (lidarrSection, coverProvidersSection, providerCard) and scope every relevant query with within() so each panel's tests interact only with their own controls. Co-Authored-By: Claude Sonnet 4.6 --- internal/api/admin_covers_test.go | 3 +- internal/server/server_test.go | 12 ++-- .../admin/integrations/integrations.test.ts | 57 ++++++++++++------- 3 files changed, 45 insertions(+), 27 deletions(-) diff --git a/internal/api/admin_covers_test.go b/internal/api/admin_covers_test.go index 62f65a12..49aa736b 100644 --- a/internal/api/admin_covers_test.go +++ b/internal/api/admin_covers_test.go @@ -1,6 +1,7 @@ package api import ( + "context" "encoding/json" "log/slog" "net/http" @@ -46,7 +47,7 @@ func doAdminCoversReq(t *testing.T, h *handlers, method, path string, user dbq.U func testHandlersWithCovers(t *testing.T) (*handlers, *coverart.Enricher) { t.Helper() h, pool := testHandlers(t) - settings, err := coverart.NewSettingsService(t.Context(), pool, slog.Default()) + settings, err := coverart.NewSettingsService(context.Background(), pool, slog.Default()) if err != nil { t.Fatalf("NewSettingsService: %v", err) } diff --git a/internal/server/server_test.go b/internal/server/server_test.go index 97b14134..a36db94f 100644 --- a/internal/server/server_test.go +++ b/internal/server/server_test.go @@ -24,7 +24,7 @@ import ( ) func TestHealthz(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, library.RunScanConfig{}) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -73,7 +73,7 @@ func TestHealthz_IncludesMinClientVersion(t *testing.T) { } func TestRouter_ServesSPAAtRoot(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, library.RunScanConfig{}) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -92,7 +92,7 @@ func TestRouter_ServesSPAAtRoot(t *testing.T) { } func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, library.RunScanConfig{}) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -111,7 +111,7 @@ func TestRouter_DeepLinkFallbackReturnsSPA(t *testing.T) { } func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, library.RunScanConfig{}) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -130,7 +130,7 @@ func TestRouter_APIPathNotSwallowedBySPA(t *testing.T) { } func TestRouter_RestPathNotSwallowedBySPA(t *testing.T) { - s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, library.RunScanConfig{}) + s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), nil, nil, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}) ts := httptest.NewServer(s.Router()) defer ts.Close() @@ -206,7 +206,7 @@ func TestRouter_AdminSubtreeNotShadowed(t *testing.T) { } s := New(slog.New(slog.NewTextHandler(io.Discard, nil)), pool, - stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, library.RunScanConfig{}) + stubScanner{}, subsonic.Config{}, config.EventsConfig{}, config.RecommendationConfig{}, "", config.BrandingConfig{}, nil, 0, nil, nil, library.RunScanConfig{}) ts := httptest.NewServer(s.Router()) defer ts.Close() diff --git a/web/src/routes/admin/integrations/integrations.test.ts b/web/src/routes/admin/integrations/integrations.test.ts index ed68e43c..e8fc3a3e 100644 --- a/web/src/routes/admin/integrations/integrations.test.ts +++ b/web/src/routes/admin/integrations/integrations.test.ts @@ -1,5 +1,5 @@ import { afterEach, describe, expect, test, vi } from 'vitest'; -import { render, screen, fireEvent, waitFor } from '@testing-library/svelte'; +import { render, screen, fireEvent, waitFor, within } from '@testing-library/svelte'; import { mockQuery } from '../../../test-utils/query'; import type { LidarrConfig } from '$lib/api/types'; import type { CoverProvider } from '$lib/api/admin'; @@ -133,11 +133,30 @@ function modalConfirmButton(): HTMLButtonElement { return inDialog as HTMLButtonElement; } +// Section-scoping helpers. Both Lidarr and the cover-art-providers panel render +// "Save changes", "Test connection", and "API key" controls, so unscoped queries +// match multiple elements. These helpers narrow every query to its own
. + +function lidarrSection(): HTMLElement { + return screen.getByRole('heading', { name: /^lidarr$/i, level: 3 }).closest('section') as HTMLElement; +} + +function coverProvidersSection(): HTMLElement { + return screen.getByRole('heading', { name: /cover art providers/i, level: 3 }).closest('section') as HTMLElement; +} + +// Each provider is rendered inside an
. Provider headings are h4. +function providerCard(providerName: string): HTMLElement { + return within(coverProvidersSection()) + .getByRole('heading', { name: providerName, level: 4 }) + .closest('article') as HTMLElement; +} + describe('/admin/integrations', () => { test('Save calls putLidarrConfig with empty api_key when input is blank', async () => { setup(); (putLidarrConfig as ReturnType).mockResolvedValueOnce(cfgConnected); - await fireEvent.click(screen.getByRole('button', { name: /save changes/i })); + await fireEvent.click(within(lidarrSection()).getByRole('button', { name: /save changes/i })); expect(putLidarrConfig).toHaveBeenCalledWith( expect.objectContaining({ enabled: true, @@ -154,7 +173,7 @@ describe('/admin/integrations', () => { const apiInput = screen.getByPlaceholderText(/saved — leave empty to keep/i); await fireEvent.input(apiInput, { target: { value: 'newkey' } }); (putLidarrConfig as ReturnType).mockResolvedValueOnce(cfgConnected); - await fireEvent.click(screen.getByRole('button', { name: /save changes/i })); + await fireEvent.click(within(lidarrSection()).getByRole('button', { name: /save changes/i })); expect(putLidarrConfig).toHaveBeenCalledWith( expect.objectContaining({ api_key: 'newkey' }) ); @@ -167,7 +186,7 @@ describe('/admin/integrations', () => { version: '2.0.0' }); await fireEvent.click( - screen.getByRole('button', { name: /test connection/i }) + within(lidarrSection()).getByRole('button', { name: /test connection/i }) ); await waitFor(() => expect(screen.getByText(/lidarr 2\.0\.0/i)).toBeInTheDocument() @@ -181,7 +200,7 @@ describe('/admin/integrations', () => { error: 'lidarr_unreachable' }); await fireEvent.click( - screen.getByRole('button', { name: /test connection/i }) + within(lidarrSection()).getByRole('button', { name: /test connection/i }) ); await waitFor(() => expect(screen.getByText(/lidarr_unreachable/i)).toBeInTheDocument() @@ -274,7 +293,8 @@ describe('/admin/integrations — cover art providers', () => { test('Save button is disabled initially (no changes)', () => { setup(); - const saveButtons = screen.getAllByRole('button', { name: /save changes/i }); + const cpSection = coverProvidersSection(); + const saveButtons = within(cpSection).getAllByRole('button', { name: /save changes/i }); // Both provider cards render Save buttons, both should be disabled initially for (const btn of saveButtons) { expect((btn as HTMLButtonElement).disabled).toBe(true); @@ -283,11 +303,11 @@ describe('/admin/integrations — cover art providers', () => { test('Save button enables after typing in API key field', async () => { setup(); - const keyInput = screen.getByLabelText(/api key/i); + const theAudioDbCard = providerCard('TheAudioDB'); + const keyInput = within(theAudioDbCard).getByLabelText(/api key/i); await fireEvent.input(keyInput, { target: { value: 'myapikey' } }); // The TheAudioDB save button should now be enabled - const saveButtons = screen.getAllByRole('button', { name: /save changes/i }); - const theAudioDbSave = saveButtons.find((b) => !b.closest('article')?.textContent?.includes('MusicBrainz')); + const theAudioDbSave = within(theAudioDbCard).getByRole('button', { name: /save changes/i }); expect(theAudioDbSave).toBeDefined(); expect((theAudioDbSave as HTMLButtonElement).disabled).toBe(false); }); @@ -298,14 +318,11 @@ describe('/admin/integrations — cover art providers', () => { ...providerTheAudioDB, version_bumped: false }); - const keyInput = screen.getByLabelText(/api key/i); + const theAudioDbCard = providerCard('TheAudioDB'); + const keyInput = within(theAudioDbCard).getByLabelText(/api key/i); await fireEvent.input(keyInput, { target: { value: 'newkey123' } }); - const saveButtons = screen.getAllByRole('button', { name: /save changes/i }); - const theAudioDbCard = screen.getByText('TheAudioDB').closest('article'); - const theAudioDbSave = theAudioDbCard?.querySelector('button[type="button"]'); - // Find the save button for TheAudioDB specifically - const btn = saveButtons.find((b) => b.closest('article')?.contains(keyInput)); - await fireEvent.click(btn!); + const btn = within(theAudioDbCard).getByRole('button', { name: /save changes/i }); + await fireEvent.click(btn); expect(updateCoverProvider).toHaveBeenCalledWith( 'theaudiodb', expect.objectContaining({ api_key: 'newkey123' }) @@ -364,8 +381,8 @@ describe('/admin/integrations — cover art providers', () => { ok: true, duration_ms: 120 }); - const testButtons = screen.getAllByRole('button', { name: /test connection/i }); - await fireEvent.click(testButtons[0]); + const mbcaaCard = providerCard('MusicBrainz CAA'); + await fireEvent.click(within(mbcaaCard).getByRole('button', { name: /test connection/i })); await waitFor(() => expect(screen.getByText(/ok.*120ms/i)).toBeInTheDocument() ); @@ -377,8 +394,8 @@ describe('/admin/integrations — cover art providers', () => { ok: false, error: 'mbcaa_unreachable' }); - const testButtons = screen.getAllByRole('button', { name: /test connection/i }); - await fireEvent.click(testButtons[0]); + const mbcaaCard = providerCard('MusicBrainz CAA'); + await fireEvent.click(within(mbcaaCard).getByRole('button', { name: /test connection/i })); await waitFor(() => expect(screen.getByText(/mbcaa_unreachable/i)).toBeInTheDocument() );