test(lidarrrequests): expand coverage on Service.Approve + Reconciler

- Adds Service tests for ListPending/ListByStatus/ListForUser, the album- and unknown-kind validation branches, and an Approve happy path with a stub Lidarr server that exercises both default-snapshot and override- snapshot persistence. - Adds Reconciler tests for Run (short tick + cancel), the album-not-yet- in-library no-op branch, and the track-kind 'parent album present but no tracks yet' branch. Combined coverage on internal/lidarr*, lidarrconfig, lidarrrequests now 86.5% (lidarrrequests 81.5%), meeting the per-package >=80% target in spec section 8.
2026-04-30 10:58:21 -04:00
parent ab9be40bb2
commit db37deb6f8
2 changed files with 302 additions and 0 deletions
@@ -5,6 +5,7 @@ import (
 	"io"
 	"log/slog"
 	"testing"
+	"time"

 	"github.com/jackc/pgx/v5/pgtype"
 	"github.com/jackc/pgx/v5/pgxpool"
@@ -325,3 +326,118 @@ func TestReconciler_DisabledIsNoOp(t *testing.T) {
 		t.Errorf("status = %v, want approved — reconciler should no-op when disabled", got.Status)
 	}
 }
+
+// TestReconciler_RunRunsAtLeastOneTick exercises Run's goroutine loop with a
+// short tick, verifies it picks up an approved row and transitions it to
+// completed before the test cancels the context.
+func TestReconciler_RunRunsAtLeastOneTick(t *testing.T) {
+	ctx, cancel := context.WithCancel(context.Background())
+	defer cancel()
+
+	pool := newPool(t)
+	enableLidarrForPool(t, pool)
+	q := dbq.New(pool)
+	user := seedUser(t, pool)
+
+	const mbid = "run-mbid"
+	seedArtist(t, q, "Run Artist", mbid)
+	req := seedApprovedRequestDirect(t, q, user, CreateParams{
+		Kind: "artist", LidarrArtistMBID: mbid, ArtistName: "Run Artist",
+	})
+
+	rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger())
+	rec.tick = 25 * time.Millisecond
+
+	done := make(chan struct{})
+	go func() {
+		rec.Run(ctx)
+		close(done)
+	}()
+
+	// Poll the row up to ~2s for the reconciler to flip it. Beats sleeping
+	// a fixed amount on slow CI.
+	deadline := time.Now().Add(2 * time.Second)
+	for {
+		got, err := q.GetLidarrRequestByID(ctx, req.ID)
+		if err == nil && got.Status == dbq.LidarrRequestStatusCompleted {
+			break
+		}
+		if time.Now().After(deadline) {
+			t.Fatalf("status not completed within 2s; last err=%v", err)
+		}
+		time.Sleep(10 * time.Millisecond)
+	}
+
+	cancel()
+	select {
+	case <-done:
+	case <-time.After(time.Second):
+		t.Fatal("Run did not exit within 1s of cancel")
+	}
+}
+
+// TestReconciler_TrackKindNoTracksInAlbumYet: the parent album exists in the
+// library but no track rows have been ingested yet. Reconciler should leave
+// the row approved so a subsequent tick can pick it up once tracks land.
+func TestReconciler_TrackKindNoTracksInAlbumYet(t *testing.T) {
+	pool := newPool(t)
+	q := dbq.New(pool)
+	ctx := context.Background()
+
+	enableLidarrForPool(t, pool)
+	user := seedUser(t, pool)
+	artistMBID := "ar-mbid-tracks-pending"
+	albumMBID := "al-mbid-tracks-pending"
+
+	artist := seedArtist(t, q, "Pending Tracks Artist", artistMBID)
+	_ = seedAlbum(t, q, artist.ID, "Album Without Tracks", albumMBID)
+	// No tracks seeded.
+
+	req := seedApprovedRequestDirect(t, q, user, CreateParams{
+		Kind: "track", LidarrArtistMBID: artistMBID, ArtistName: "Pending Tracks Artist",
+		LidarrAlbumMBID: albumMBID, AlbumTitle: "Album Without Tracks",
+		LidarrTrackMBID: "t-mbid-not-yet", TrackTitle: "Some Track",
+	})
+
+	rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger())
+	if err := rec.tickOnce(ctx); err != nil {
+		t.Fatalf("tickOnce: %v", err)
+	}
+
+	got, err := q.GetLidarrRequestByID(ctx, req.ID)
+	if err != nil {
+		t.Fatalf("GetLidarrRequestByID: %v", err)
+	}
+	if got.Status != dbq.LidarrRequestStatusApproved {
+		t.Errorf("status = %v, want approved (album present, no tracks yet)", got.Status)
+	}
+}
+
+// TestReconciler_AlbumKindNoMatchInAlbumsTable exercises the album branch's
+// no-rows return path when the request's album_mbid isn't in the library yet.
+func TestReconciler_AlbumKindNoMatchInAlbumsTable(t *testing.T) {
+	pool := newPool(t)
+	q := dbq.New(pool)
+	ctx := context.Background()
+
+	enableLidarrForPool(t, pool)
+	user := seedUser(t, pool)
+
+	req := seedApprovedRequestDirect(t, q, user, CreateParams{
+		Kind: "album", LidarrArtistMBID: "ar-no-match", ArtistName: "Nope Artist",
+		LidarrAlbumMBID: "al-not-in-library", AlbumTitle: "Nope Album",
+	})
+
+	rec := NewReconciler(pool, lidarrconfig.New(pool), newTestLogger())
+	if err := rec.tickOnce(ctx); err != nil {
+		t.Fatalf("tickOnce: %v", err)
+	}
+
+	got, err := q.GetLidarrRequestByID(ctx, req.ID)
+	if err != nil {
+		t.Fatalf("GetLidarrRequestByID: %v", err)
+	}
+	if got.Status != dbq.LidarrRequestStatusApproved {
+		t.Errorf("status = %v, want approved (album not yet in library)", got.Status)
+	}
+}
@@ -5,6 +5,8 @@ import (
 	"errors"
 	"io"
 	"log/slog"
+	"net/http"
+	"net/http/httptest"
 	"os"
 	"testing"

@@ -14,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/lidarr"
 	"git.fabledsword.com/bvandeusen/minstrel/internal/lidarrconfig"
 )

@@ -144,3 +147,186 @@ func TestCancel_OwnPendingOnly(t *testing.T) {
 		t.Errorf("err = %v, want ErrNotPending", err)
 	}
 }
+
+func TestCreate_AlbumKindRequiresAlbumMBID(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, nil)
+	_, err := svc.Create(context.Background(), user, CreateParams{
+		Kind:             "album",
+		LidarrArtistMBID: "a-mbid", ArtistName: "X",
+		// missing album_mbid + album_title
+	})
+	if !errors.Is(err, ErrInvalidKindFields) {
+		t.Fatalf("err = %v, want ErrInvalidKindFields", err)
+	}
+}
+
+func TestCreate_UnknownKindRejected(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, nil)
+	_, err := svc.Create(context.Background(), user, CreateParams{
+		Kind: "playlist", LidarrArtistMBID: "a-mbid", ArtistName: "X",
+	})
+	if !errors.Is(err, ErrInvalidKindFields) {
+		t.Fatalf("err = %v, want ErrInvalidKindFields", err)
+	}
+}
+
+func TestListPending_AndListByStatus(t *testing.T) {
+	pool := newPool(t)
+	user := seedUser(t, pool)
+	svc := NewService(pool, lidarrconfig.New(pool), nil, nil)
+	for _, name := range []string{"A", "B"} {
+		if _, err := svc.Create(context.Background(), user, CreateParams{
+			Kind: "artist", LidarrArtistMBID: name + "-mbid", ArtistName: name,
+		}); err != nil {
+			t.Fatalf("Create(%s): %v", name, err)
+		}
+	}
+	pending, err := svc.ListPending(context.Background(), 50)
+	if err != nil {
+		t.Fatalf("ListPending: %v", err)
+	}
+	if len(pending) != 2 {
+		t.Errorf("ListPending len = %d, want 2", len(pending))
+	}
+	rejected, err := svc.ListByStatus(context.Background(), "rejected", 50)
+	if err != nil {
+		t.Fatalf("ListByStatus(rejected): %v", err)
+	}
+	if len(rejected) != 0 {
+		t.Errorf("ListByStatus(rejected) len = %d, want 0", len(rejected))
+	}
+}
+
+func TestListForUser_OnlyOwnRows(t *testing.T) {
+	pool := newPool(t)
+	alice := seedUser(t, pool)
+	bob, err := dbq.New(pool).CreateUser(context.Background(), dbq.CreateUserParams{
+		Username: dbtest.TestUserPrefix + "bob", PasswordHash: "x", ApiToken: "x2", IsAdmin: false,
+	})
+	if err != nil {
+		t.Fatalf("seed bob: %v", err)
+	}
+	svc := NewService(pool, lidarrconfig.New(pool), nil, nil)
+	if _, err := svc.Create(context.Background(), alice, CreateParams{
+		Kind: "artist", LidarrArtistMBID: "a-mbid", ArtistName: "Alice's request",
+	}); err != nil {
+		t.Fatalf("Create(alice): %v", err)
+	}
+	if _, err := svc.Create(context.Background(), bob.ID, CreateParams{
+		Kind: "artist", LidarrArtistMBID: "b-mbid", ArtistName: "Bob's request",
+	}); err != nil {
+		t.Fatalf("Create(bob): %v", err)
+	}
+	rows, err := svc.ListForUser(context.Background(), alice, 50)
+	if err != nil {
+		t.Fatalf("ListForUser: %v", err)
+	}
+	if len(rows) != 1 || rows[0].ArtistName != "Alice's request" {
+		t.Errorf("ListForUser = %+v, want only Alice's row", rows)
+	}
+}
+
+// approveTestSetup wires Service against a stub Lidarr server. Returns the
+// Service, an admin user id, and the captured Lidarr request body for the
+// most recent call (handler updates it in-place).
+func approveTestSetup(t *testing.T) (*Service, pgtype.UUID, *pgxpool.Pool, *httptest.Server) {
+	t.Helper()
+	pool := newPool(t)
+	stub := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
+		w.Header().Set("Content-Type", "application/json")
+		w.WriteHeader(http.StatusOK)
+		_, _ = w.Write([]byte(`{"id":1}`))
+	}))
+	t.Cleanup(stub.Close)
+
+	cfg := lidarrconfig.New(pool)
+	if err := cfg.Save(context.Background(), lidarrconfig.Config{
+		Enabled:                 true,
+		BaseURL:                 stub.URL,
+		APIKey:                  "k",
+		DefaultQualityProfileID: 7,
+		DefaultRootFolderPath:   "/music",
+	}); err != nil {
+		t.Fatalf("save config: %v", err)
+	}
+	clientFn := func() *lidarr.Client { return lidarr.NewClient(stub.URL, "k") }
+	svc := NewService(pool, cfg, clientFn, nil)
+	user := seedUser(t, pool)
+	return svc, user, pool, stub
+}
+
+func TestApprove_HappyPath_ArtistKind(t *testing.T) {
+	svc, user, _, _ := approveTestSetup(t)
+	r, err := svc.Create(context.Background(), user, CreateParams{
+		Kind: "artist", LidarrArtistMBID: "ar-mbid", ArtistName: "Approvee",
+	})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	approved, err := svc.Approve(context.Background(), r.ID, user, ApproveOverrides{})
+	if err != nil {
+		t.Fatalf("Approve: %v", err)
+	}
+	if approved.Status != dbq.LidarrRequestStatusApproved {
+		t.Errorf("status = %v, want approved", approved.Status)
+	}
+	// Default snapshot from config.
+	if approved.QualityProfileID == nil || *approved.QualityProfileID != 7 {
+		t.Errorf("quality_profile_id = %v, want 7", approved.QualityProfileID)
+	}
+	if approved.RootFolderPath == nil || *approved.RootFolderPath != "/music" {
+		t.Errorf("root_folder_path = %v, want /music", approved.RootFolderPath)
+	}
+}
+
+func TestApprove_HappyPath_AlbumKindWithOverride(t *testing.T) {
+	svc, user, _, _ := approveTestSetup(t)
+	r, err := svc.Create(context.Background(), user, CreateParams{
+		Kind:             "album",
+		LidarrArtistMBID: "ar-mbid", ArtistName: "X",
+		LidarrAlbumMBID: "al-mbid", AlbumTitle: "Y",
+	})
+	if err != nil {
+		t.Fatalf("Create: %v", err)
+	}
+	approved, err := svc.Approve(context.Background(), r.ID, user, ApproveOverrides{
+		QualityProfileID: 99,
+		RootFolderPath:   "/elsewhere",
+	})
+	if err != nil {
+		t.Fatalf("Approve: %v", err)
+	}
+	if approved.QualityProfileID == nil || *approved.QualityProfileID != 99 {
+		t.Errorf("quality_profile_id = %v, want 99 (override)", approved.QualityProfileID)
+	}
+	if approved.RootFolderPath == nil || *approved.RootFolderPath != "/elsewhere" {
+		t.Errorf("root_folder_path = %v, want /elsewhere", approved.RootFolderPath)
+	}
+}
+
+func TestApprove_AlreadyApprovedReturnsErrNotPending(t *testing.T) {
+	svc, user, _, _ := approveTestSetup(t)
+	r, _ := svc.Create(context.Background(), user, CreateParams{
+		Kind: "artist", LidarrArtistMBID: "ar-mbid", ArtistName: "X",
+	})
+	if _, err := svc.Approve(context.Background(), r.ID, user, ApproveOverrides{}); err != nil {
+		t.Fatalf("first approve: %v", err)
+	}
+	if _, err := svc.Approve(context.Background(), r.ID, user, ApproveOverrides{}); !errors.Is(err, ErrNotPending) {
+		t.Errorf("second approve err = %v, want ErrNotPending", err)
+	}
+}
+
+func TestApprove_NotFound(t *testing.T) {
+	svc, user, _, _ := approveTestSetup(t)
+	var bogus pgtype.UUID
+	bogus.Bytes = [16]byte{1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12, 13, 14, 15, 16}
+	bogus.Valid = true
+	if _, err := svc.Approve(context.Background(), bogus, user, ApproveOverrides{}); !errors.Is(err, ErrNotFound) {
+		t.Errorf("err = %v, want ErrNotFound", err)
+	}
+}