test(playlists): the same-artist guard needed a fixture that has same artists
test-go / test (push) Successful in 1m25s
test-go / integration (push) Successful in 4m46s
release / Build signed APK (releases and dev) (push) Successful in 5m45s
release / Build + push container image (push) Successful in 17s
release / Verify release artifacts (tag releases only) (push) Skipped
test-go / test (push) Successful in 1m25s
test-go / integration (push) Successful in 4m46s
release / Build signed APK (releases and dev) (push) Successful in 5m45s
release / Build + push container image (push) Successful in 17s
release / Verify release artifacts (tag releases only) (push) Skipped
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) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SQ31KQpYbStyK5y58UmPLH
This commit is contained in:
@@ -2,8 +2,10 @@ package playlists_test
|
|||||||
|
|
||||||
import (
|
import (
|
||||||
"context"
|
"context"
|
||||||
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"log/slog"
|
"log/slog"
|
||||||
|
"path/filepath"
|
||||||
"sync"
|
"sync"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"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
|
// 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).
|
// 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) {
|
func TestBuildSystemPlaylists_SongsLikeIncludesItsSeedArtist(t *testing.T) {
|
||||||
pool := newPool(t)
|
pool := newPool(t)
|
||||||
logger := discardLogger()
|
logger := discardLogger()
|
||||||
u, _ := seedActiveLibrary(t, pool, "seedartist", 4, 5)
|
u, _ := seedSharedArtistLibrary(t, pool, "seedartist", 4, 6)
|
||||||
ctx := context.Background()
|
ctx := context.Background()
|
||||||
now := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)
|
now := time.Date(2026, 5, 4, 12, 0, 0, 0, time.UTC)
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user