From f5dd4462de19a3a601dde51283717716eb29d67c Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Thu, 10 Sep 2026 21:25:49 -0400 Subject: [PATCH] test(playlists): the same-artist guard needed a fixture that has same artists MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Fixes the integration failure from 31190657. The test was wrong, not the code: it could not have passed whatever produceSeedMixes did. seedActiveLibrary builds its tracks through seedTrack, whose own comment says "artist and album are not deduplicated across calls (mbid-less upsert)". So every track gets a fresh artist row despite sharing a name — 4 artists x 5 tracks is really 20 artists with one track each. A seed artist's only track IS the seed, which is excluded from its own mix, so "does this mix contain a track by its seed artist" was structurally answerable only as no. That is the failure mode worth naming: the assertion was measuring the fixture, not the behaviour, and it reported the behaviour as broken. seedSharedArtistLibrary upserts each artist ONCE and reuses the id across its tracks, so a seed artist genuinely owns five others. Albums are still not deduplicated, which suits this test — the per-album cap never binds, so the per-artist cap (3) is unambiguously what is under test. Noted in the fixture, because "tidying" the album titles into something shared would silently change which cap the assertion measures. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH --- internal/playlists/system_test.go | 66 ++++++++++++++++++++++++++++++- 1 file changed, 65 insertions(+), 1 deletion(-) diff --git a/internal/playlists/system_test.go b/internal/playlists/system_test.go index 18effa26..48a96719 100644 --- a/internal/playlists/system_test.go +++ b/internal/playlists/system_test.go @@ -2,8 +2,10 @@ package playlists_test import ( "context" + "fmt" "io" "log/slog" + "path/filepath" "sync" "testing" "time" @@ -287,6 +289,68 @@ func TestBuildSystemPlaylists_Concurrency(t *testing.T) { } } +// seedSharedArtistLibrary seeds artists that genuinely OWN several tracks. +// +// seedActiveLibrary cannot be used for this: its helper documents that +// "artist and album are not deduplicated across calls (mbid-less upsert)", +// so every track gets its own artist row and a seed artist always has +// exactly one track — the seed itself, which is excluded. A same-artist +// assertion against that fixture can never pass no matter what the code +// does, which is how the first version of this test failed. +// +// Albums are not deduplicated either, for the same mbid-less reason, so each +// track ends up under its own album row however the titles are written. That +// is convenient here rather than a problem: it means the per-ALBUM cap (2) +// never binds, and the per-ARTIST cap (3) is unambiguously the thing under +// test. Do not "fix" the album titles into something shared without checking +// which cap you are then measuring. +func seedSharedArtistLibrary( + t *testing.T, pool *pgxpool.Pool, name string, numArtists, tracksPerArtist int, +) (dbq.User, []pgtype.UUID) { + t.Helper() + q := dbq.New(pool) + ctx := context.Background() + u := seedUser(t, pool, name) + now := time.Now().UTC() + dir := t.TempDir() + + artistIDs := make([]pgtype.UUID, 0, numArtists) + for a := 0; a < numArtists; a++ { + artistName := name + "-shared-" + string(rune('A'+a)) + ar, err := q.UpsertArtist(ctx, dbq.UpsertArtistParams{Name: artistName, SortName: artistName}) + if err != nil { + t.Fatalf("seed artist: %v", err) + } + artistIDs = append(artistIDs, ar.ID) + + for k := 0; k < tracksPerArtist; k++ { + albumTitle := fmt.Sprintf("%s - Album %d", artistName, k/2) + al, err := q.UpsertAlbum(ctx, dbq.UpsertAlbumParams{ + Title: albumTitle, SortTitle: albumTitle, ArtistID: ar.ID, + }) + if err != nil { + t.Fatalf("seed album: %v", err) + } + tk, err := q.UpsertTrack(ctx, dbq.UpsertTrackParams{ + Title: fmt.Sprintf("%s-t%d", artistName, k), AlbumID: al.ID, ArtistID: ar.ID, + DurationMs: 1000, + FilePath: filepath.Join(dir, fmt.Sprintf("%s-%d-%d.mp3", name, a, k)), + FileSize: 100, FileFormat: "mp3", + }) + if err != nil { + t.Fatalf("seed track: %v", err) + } + // Well clear of the recently-played exclusion window, which uses + // the DATABASE clock rather than the build's `now`. + for pl := 0; pl < 3; pl++ { + seedPlayEvent(t, pool, u.ID, tk.ID, + now.Add(-time.Duration(24+a*10+k+pl)*time.Hour), false) + } + } + } + return u, artistIDs +} + // The seed artist's own tracks are ELIGIBLE for its "Songs like" mix, and are // bounded by the diversity cap rather than excluded outright (#3881). // @@ -301,7 +365,7 @@ func TestBuildSystemPlaylists_Concurrency(t *testing.T) { func TestBuildSystemPlaylists_SongsLikeIncludesItsSeedArtist(t *testing.T) { pool := newPool(t) logger := discardLogger() - u, _ := seedActiveLibrary(t, pool, "seedartist", 4, 5) + u, _ := seedSharedArtistLibrary(t, pool, "seedartist", 4, 6) ctx := context.Background() now := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)