From d212621eaab16800f9dbca0ce32755d46c5f0fd2 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Sun, 17 May 2026 13:32:42 -0400 Subject: [PATCH] test: fix 4 integration-suite root causes surfaced by CI (C+A+B+F) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Latent failures exposed now that integration tests run in CI (#339). All test/test-harness only — no production code changes. A (dbtest.ResetDB): also truncate cover_art_provider_settings + cover_art_sources_meta. The monotonic source-version counter (seeded 1 by 0018) accumulated across internal/coverart tests → CurrentVersion=4 want 1, key-only-bump assertions, enricher source skips. SettingsService re-seeds both at boot, so a truncated start is the correct fresh state. B (playlists system_test.seedPlayEvent): inserted play_events with a random session_id → play_events_session_id_fkey violation (7 tests). Create the parent play_sessions row in the same statement (CTE). C (similarity worker_integration_test.newTestWorker): built the client with only BaseURL. Post-4fca0e6 similarity hits the Labs API via LabsBaseURL, so an unset LabsBaseURL fell through to the real labs.api and the stub never ran → 0 similarity rows. Set LabsBaseURL to the stub. F (library scanner_test): NOT a flake — deterministic. Synthetic ID3-only MP3s have no decodable duration; the scanner intentionally won't skip duration_ms=0 rows (retries duration backfill), so every re-scan reported Updated not Skipped. Seed duration_ms>0 before the second scan so the mtime-based incremental-skip path under test is actually exercised. Production scanner behavior unchanged. Co-Authored-By: Claude Opus 4.7 (1M context) --- internal/dbtest/reset.go | 8 ++++++++ internal/library/scanner_test.go | 11 +++++++++++ internal/playlists/system_test.go | 9 ++++++++- internal/similarity/worker_integration_test.go | 7 +++++-- 4 files changed, 32 insertions(+), 3 deletions(-) diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go index 5d6dbbda..cd42e423 100644 --- a/internal/dbtest/reset.go +++ b/internal/dbtest/reset.go @@ -55,6 +55,14 @@ var dataTables = []string{ "playlist_tracks", "playlists", "library_changes", // M7 #357 — must reset to keep cursor isolated per test + // Cover-art provider settings + the monotonic source-version + // counter (cover_art_sources_meta, seeded at 1 by migration 0018). + // Not resetting these let the version accumulate across tests in + // internal/coverart (CurrentVersion=4 want 1, key-only-bump + // assertions, enricher source skips). SettingsService re-seeds both + // at boot, so a truncated start is the correct fresh state. + "cover_art_provider_settings", + "cover_art_sources_meta", "tracks", "albums", "artists", diff --git a/internal/library/scanner_test.go b/internal/library/scanner_test.go index a9dc5250..80f8df54 100644 --- a/internal/library/scanner_test.go +++ b/internal/library/scanner_test.go @@ -103,6 +103,17 @@ func TestScanner_Integration(t *testing.T) { t.Errorf("artist sort = [%q, %q], want [Artist X, Artist Y]", artists[0].SortName, artists[1].SortName) } + // The synthetic MP3s carry only an ID3 tag — no decodable audio — + // so ffprobe yields duration 0. The scanner deliberately refuses to + // skip zero-duration rows (it re-runs them so a later scan can + // backfill duration once probing works), which is orthogonal to the + // mtime-based incremental-skip this test covers. Simulate a + // normally-probed library so the skip path is actually exercised; + // updated_at is left untouched (still ≥ file mtime). + if _, err := pool.Exec(ctx, "UPDATE tracks SET duration_ms = 1000 WHERE duration_ms = 0"); err != nil { + t.Fatalf("seed durations: %v", err) + } + stats2, err := scanner.Scan(ctx, nil) if err != nil { t.Fatalf("second scan: %v", err) diff --git a/internal/playlists/system_test.go b/internal/playlists/system_test.go index 64d7c456..6d5fea50 100644 --- a/internal/playlists/system_test.go +++ b/internal/playlists/system_test.go @@ -24,9 +24,16 @@ func discardLogger() *slog.Logger { // `InsertPlayEvent` doesn't accept was_skipped (that's set by an UPDATE). func seedPlayEvent(t *testing.T, pool *pgxpool.Pool, userID, trackID pgtype.UUID, startedAt time.Time, wasSkipped bool) { t.Helper() + // play_events.session_id has a FK to play_sessions; create a parent + // session in the same statement (a random UUID violates the FK). _, err := pool.Exec(context.Background(), ` + WITH s AS ( + INSERT INTO play_sessions (user_id, started_at, last_event_at) + VALUES ($1, $3, $3) + RETURNING id + ) INSERT INTO play_events (user_id, track_id, session_id, started_at, was_skipped) - VALUES ($1, $2, gen_random_uuid(), $3, $4) + SELECT $1, $2, s.id, $3, $4 FROM s `, userID, trackID, startedAt, wasSkipped) if err != nil { t.Fatalf("seed play_event: %v", err) diff --git a/internal/similarity/worker_integration_test.go b/internal/similarity/worker_integration_test.go index e79863ca..c8fa0fde 100644 --- a/internal/similarity/worker_integration_test.go +++ b/internal/similarity/worker_integration_test.go @@ -114,8 +114,11 @@ func markPlayed(t *testing.T, f fixture, trackID pgtype.UUID) { func newTestWorker(f fixture, lbBaseURL string) *Worker { logger := slog.New(slog.NewTextHandler(io.Discard, nil)) return &Worker{ - pool: f.pool, - client: &listenbrainz.Client{BaseURL: lbBaseURL, HTTP: http.DefaultClient}, + pool: f.pool, + // LabsBaseURL must point at the stub too: similarity calls hit + // the Labs API (commit 4fca0e6), so an unset LabsBaseURL would + // fall through to the real labs.api and the stub never runs. + client: &listenbrainz.Client{BaseURL: lbBaseURL, LabsBaseURL: lbBaseURL, HTTP: http.DefaultClient}, logger: logger, tick: 1 * time.Hour, batch: 5,