From 937a1930ad2aceb6520f7c961a398a653bb941f9 Mon Sep 17 00:00:00 2001 From: Bryan Van Deusen Date: Wed, 29 Apr 2026 11:55:06 -0400 Subject: [PATCH] test: add dbtest.ResetDB helper that preserves admin user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Integration tests previously TRUNCATEd the users table at setup, which wiped the operator's admin login every time the suite ran against a Postgres instance shared with their dev environment. This commit: - Adds internal/dbtest/reset.go exposing ResetDB(t, pool) and TestUserPrefix. ResetDB truncates every data table EXCEPT users, then deletes only users whose username starts with TestUserPrefix. - Migrates 9 test files (subsonic/scrobble, subsonic/star, recommendation/candidates, scrobble/queue, similarity/worker, playevents/writer, playsessions/service, api/auth, api/me) to call ResetDB instead of issuing TRUNCATE-with-users. - Renames hardcoded test usernames to use TestUserPrefix so ResetDB cleans them between runs. seedUser in api/auth_test now prefixes internally; existing call sites pass bare names. - Leaves auth/bootstrap_test.go alone — it legitimately tests first-time admin bootstrap and must wipe users. Verified locally: full integration suite with -p 1 runs cleanly under the existing MINSTREL_TEST_DATABASE_URL setup, and the admin row in the shared dev DB survives the run. Co-Authored-By: Claude Opus 4.7 --- internal/api/auth_test.go | 22 +++--- internal/api/me_test.go | 4 +- internal/dbtest/reset.go | 73 +++++++++++++++++++ internal/playevents/writer_test.go | 8 +- internal/playsessions/service_test.go | 8 +- internal/recommendation/candidates_test.go | 10 +-- internal/scrobble/queue_test.go | 8 +- .../similarity/worker_integration_test.go | 8 +- internal/subsonic/scrobble_test.go | 8 +- internal/subsonic/star_test.go | 10 +-- 10 files changed, 111 insertions(+), 48 deletions(-) create mode 100644 internal/dbtest/reset.go diff --git a/internal/api/auth_test.go b/internal/api/auth_test.go index 5ab5599b..b13b455e 100644 --- a/internal/api/auth_test.go +++ b/internal/api/auth_test.go @@ -21,6 +21,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/config" "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" ) @@ -45,10 +46,7 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE play_events, skip_events, play_sessions, sessions, users RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) w := playevents.NewWriter(pool, logger, 30*time.Minute, 0.5, 30000) recCfg := config.RecommendationConfig{ BaseWeight: 1.0, LikeBoost: 2.0, RecencyWeight: 1.0, @@ -60,16 +58,22 @@ func testHandlers(t *testing.T) (*handlers, *pgxpool.Pool) { return h, pool } +// seedUser creates a test user. The supplied username is automatically +// prefixed with dbtest.TestUserPrefix so dbtest.ResetDB cleans it up +// without touching the operator's admin row. Callers that need to send +// the username in HTTP bodies or compare against API responses must +// include the same prefix in their literals. func seedUser(t *testing.T, pool *pgxpool.Pool, username, password string, isAdmin bool) dbq.User { t.Helper() hash, err := bcrypt.GenerateFromPassword([]byte(password), bcrypt.MinCost) if err != nil { t.Fatalf("bcrypt: %v", err) } + prefixed := dbtest.TestUserPrefix + username u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{ - Username: username, + Username: prefixed, PasswordHash: string(hash), - ApiToken: "test-api-token-" + username, + ApiToken: "test-api-token-" + prefixed, IsAdmin: isAdmin, }) if err != nil { @@ -82,7 +86,7 @@ func TestHandleLogin_SuccessSetsCookieAndReturnsToken(t *testing.T) { h, pool := testHandlers(t) seedUser(t, pool, "alice", "hunter2", false) - body := strings.NewReader(`{"username":"alice","password":"hunter2"}`) + body := strings.NewReader(`{"username":"test-alice","password":"hunter2"}`) req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body) req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() @@ -105,7 +109,7 @@ func TestHandleLogin_SuccessSetsCookieAndReturnsToken(t *testing.T) { if resp.Token == "" { t.Error("token empty") } - if resp.User.Username != "alice" || resp.User.IsAdmin { + if resp.User.Username != "test-alice" || resp.User.IsAdmin { t.Errorf("user = %+v, want alice/non-admin", resp.User) } @@ -133,7 +137,7 @@ func TestHandleLogin_WrongPasswordReturns401(t *testing.T) { h, pool := testHandlers(t) seedUser(t, pool, "alice", "hunter2", false) - body := strings.NewReader(`{"username":"alice","password":"wrong"}`) + body := strings.NewReader(`{"username":"test-alice","password":"wrong"}`) req := httptest.NewRequest(http.MethodPost, "/api/auth/login", body) req.Header.Set("Content-Type", "application/json") w := httptest.NewRecorder() diff --git a/internal/api/me_test.go b/internal/api/me_test.go index 1c582feb..31362add 100644 --- a/internal/api/me_test.go +++ b/internal/api/me_test.go @@ -26,8 +26,8 @@ func TestHandleGetMe_ReturnsAuthenticatedUser(t *testing.T) { if err := json.Unmarshal(w.Body.Bytes(), &got); err != nil { t.Fatalf("decode: %v", err) } - if got.Username != "alice" || !got.IsAdmin { - t.Errorf("user = %+v, want alice/admin", got) + if got.Username != "test-alice" || !got.IsAdmin { + t.Errorf("user = %+v, want test-alice/admin", got) } } diff --git a/internal/dbtest/reset.go b/internal/dbtest/reset.go new file mode 100644 index 00000000..4f821232 --- /dev/null +++ b/internal/dbtest/reset.go @@ -0,0 +1,73 @@ +// Package dbtest provides shared helpers for integration tests that need +// a clean Postgres state without disturbing the operator's admin user. +// +// Background: when integration tests run against the same Postgres +// instance an operator uses for local dev (a common dev-laptop setup), +// a TRUNCATE that includes the users table wipes the operator's admin +// login between every test run. To avoid that, tests should: +// +// 1. Call ResetDB instead of issuing TRUNCATE statements directly. It +// truncates every data table EXCEPT users, then deletes only those +// users whose username starts with TestUserPrefix. +// 2. Always create test user rows with a username that begins with +// TestUserPrefix; otherwise those rows survive across runs and +// leak into other tests. +// +// One test legitimately needs to wipe the entire users table: +// internal/auth/bootstrap_test.go, which exercises first-time admin +// bootstrap. That file does its own TRUNCATE and does not use this +// helper. +package dbtest + +import ( + "context" + "strings" + "testing" + + "github.com/jackc/pgx/v5/pgxpool" +) + +// TestUserPrefix is the required username prefix for any user row +// created by an integration test. ResetDB removes every user matching +// this prefix and leaves all others intact. +const TestUserPrefix = "test-" + +// dataTables is the union of every non-users table touched by any +// integration test in the repo. Truncating the union is harmless for +// callers that only care about a subset; truncating tables that don't +// exist would error, but every name here corresponds to a migration +// that has shipped. +var dataTables = []string{ + "artist_similarity", + "track_similarity", + "scrobble_queue", + "contextual_likes", + "general_likes_albums", + "general_likes_artists", + "general_likes", + "play_events", + "skip_events", + "play_sessions", + "sessions", + "tracks", + "albums", + "artists", +} + +// ResetDB clears all data tables and removes any user whose username +// begins with TestUserPrefix. Users without that prefix (notably the +// operator's admin row) are left alone. Calls t.Fatalf on error. +func ResetDB(t *testing.T, pool *pgxpool.Pool) { + t.Helper() + ctx := context.Background() + stmt := "TRUNCATE " + strings.Join(dataTables, ", ") + " RESTART IDENTITY CASCADE" + if _, err := pool.Exec(ctx, stmt); err != nil { + t.Fatalf("dbtest.ResetDB truncate: %v", err) + } + if _, err := pool.Exec(ctx, + "DELETE FROM users WHERE username LIKE $1", + TestUserPrefix+"%", + ); err != nil { + t.Fatalf("dbtest.ResetDB delete test users: %v", err) + } +} diff --git a/internal/playevents/writer_test.go b/internal/playevents/writer_test.go index ac3876ec..6f9d8233 100644 --- a/internal/playevents/writer_test.go +++ b/internal/playevents/writer_test.go @@ -14,6 +14,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" ) func testPool(t *testing.T) *pgxpool.Pool { @@ -33,10 +34,7 @@ func testPool(t *testing.T) *pgxpool.Pool { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE scrobble_queue, play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) return pool } @@ -53,7 +51,7 @@ func newFixture(t *testing.T, durationMs int32) fixture { pool := testPool(t) q := dbq.New(pool) u, err := q.CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, }) if err != nil { t.Fatalf("user: %v", err) diff --git a/internal/playsessions/service_test.go b/internal/playsessions/service_test.go index 6fd52eb9..c0a7da0a 100644 --- a/internal/playsessions/service_test.go +++ b/internal/playsessions/service_test.go @@ -13,6 +13,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" ) func testPool(t *testing.T) *pgxpool.Pool { @@ -32,17 +33,14 @@ func testPool(t *testing.T) *pgxpool.Pool { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE play_events, skip_events, play_sessions, sessions, users RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) return pool } func seedTestUser(t *testing.T, pool *pgxpool.Pool) pgtype.UUID { t.Helper() u, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "tester", + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, diff --git a/internal/recommendation/candidates_test.go b/internal/recommendation/candidates_test.go index 0446ba3d..2545ded2 100644 --- a/internal/recommendation/candidates_test.go +++ b/internal/recommendation/candidates_test.go @@ -14,6 +14,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" ) func testPool(t *testing.T) *pgxpool.Pool { @@ -33,10 +34,7 @@ func testPool(t *testing.T) *pgxpool.Pool { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE contextual_likes, general_likes, general_likes_albums, general_likes_artists, play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) return pool } @@ -52,7 +50,7 @@ func newFixture(t *testing.T, n int) fixture { pool := testPool(t) q := dbq.New(pool) u, err := q.CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, }) if err != nil { t.Fatalf("user: %v", err) @@ -199,7 +197,7 @@ func TestLoadCandidates_NeverPlayedHasNilLastPlayed(t *testing.T) { func TestLoadCandidates_CrossUserIsolation(t *testing.T) { f := newFixture(t, 2) bob, _ := f.q.CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "bob", PasswordHash: "x", ApiToken: "y", IsAdmin: false, + Username: dbtest.TestUserPrefix + "bob", PasswordHash: "x", ApiToken: "y", IsAdmin: false, }) // Alice likes tracks[1]; Bob shouldn't see it. _, _ = f.q.LikeTrack(context.Background(), dbq.LikeTrackParams{UserID: f.user, TrackID: f.tracks[1].ID}) diff --git a/internal/scrobble/queue_test.go b/internal/scrobble/queue_test.go index 86cd3943..10b92c00 100644 --- a/internal/scrobble/queue_test.go +++ b/internal/scrobble/queue_test.go @@ -12,6 +12,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" ) func testPool(t *testing.T) (*pgxpool.Pool, *dbq.Queries) { @@ -31,10 +32,7 @@ func testPool(t *testing.T) (*pgxpool.Pool, *dbq.Queries) { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE scrobble_queue, play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) return pool, dbq.New(pool) } @@ -59,7 +57,7 @@ func seed(t *testing.T, opts seedOpts) setup { pool, q := testPool(t) ctx := context.Background() u, err := q.CreateUser(ctx, dbq.CreateUserParams{ - Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, }) if err != nil { t.Fatalf("user: %v", err) diff --git a/internal/similarity/worker_integration_test.go b/internal/similarity/worker_integration_test.go index 137bfdb7..87cf7732 100644 --- a/internal/similarity/worker_integration_test.go +++ b/internal/similarity/worker_integration_test.go @@ -19,6 +19,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" "git.fabledsword.com/bvandeusen/minstrel/internal/scrobble/listenbrainz" ) @@ -39,10 +40,7 @@ func testPool(t *testing.T) (*pgxpool.Pool, *dbq.Queries) { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE artist_similarity, track_similarity, scrobble_queue, play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) return pool, dbq.New(pool) } @@ -59,7 +57,7 @@ func newFixture(t *testing.T) fixture { pool, q := testPool(t) ctx := context.Background() u, err := q.CreateUser(ctx, dbq.CreateUserParams{ - Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, }) if err != nil { t.Fatalf("user: %v", err) diff --git a/internal/subsonic/scrobble_test.go b/internal/subsonic/scrobble_test.go index 36079a56..6567e5df 100644 --- a/internal/subsonic/scrobble_test.go +++ b/internal/subsonic/scrobble_test.go @@ -15,6 +15,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" ) @@ -35,13 +36,10 @@ func testScrobblePool(t *testing.T) (*pgxpool.Pool, dbq.User, dbq.Track) { t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) q := dbq.New(pool) u, err := q.CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, }) if err != nil { t.Fatalf("user: %v", err) diff --git a/internal/subsonic/star_test.go b/internal/subsonic/star_test.go index 06634b5a..c609e01b 100644 --- a/internal/subsonic/star_test.go +++ b/internal/subsonic/star_test.go @@ -16,6 +16,7 @@ import ( "git.fabledsword.com/bvandeusen/minstrel/internal/db" "git.fabledsword.com/bvandeusen/minstrel/internal/db/dbq" + "git.fabledsword.com/bvandeusen/minstrel/internal/dbtest" "git.fabledsword.com/bvandeusen/minstrel/internal/playevents" ) @@ -36,13 +37,10 @@ func testStarPool(t *testing.T) (*pgxpool.Pool, dbq.User, dbq.Track, dbq.Album, t.Fatalf("pool: %v", err) } t.Cleanup(pool.Close) - if _, err := pool.Exec(context.Background(), - "TRUNCATE general_likes, general_likes_albums, general_likes_artists, play_events, skip_events, play_sessions, sessions, users, tracks, albums, artists RESTART IDENTITY CASCADE"); err != nil { - t.Fatalf("truncate: %v", err) - } + dbtest.ResetDB(t, pool) q := dbq.New(pool) u, _ := q.CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, + Username: dbtest.TestUserPrefix + "tester", PasswordHash: "x", ApiToken: "x", IsAdmin: false, }) a, _ := q.UpsertArtist(context.Background(), dbq.UpsertArtistParams{Name: "X", SortName: "X"}) al, _ := q.UpsertAlbum(context.Background(), dbq.UpsertAlbumParams{Title: "X", SortTitle: "X", ArtistID: a.ID}) @@ -230,7 +228,7 @@ func TestHandleGetStarred2_CrossUserIsolation(t *testing.T) { pool, alice, track, _, _ := testStarPool(t) q := dbq.New(pool) bob, _ := q.CreateUser(context.Background(), dbq.CreateUserParams{ - Username: "bob", PasswordHash: "x", ApiToken: "y", IsAdmin: false, + Username: dbtest.TestUserPrefix + "bob", PasswordHash: "x", ApiToken: "y", IsAdmin: false, }) _, _ = q.LikeTrack(context.Background(), dbq.LikeTrackParams{UserID: alice.ID, TrackID: track.ID})