consolidate tests

Signed-off-by: Deluan <deluan@navidrome.org>

core/external_metadata_simple_test.go

@@ -1,474 +0,0 @@
-package core
-
-import (
-	"context"
-	"errors"
-	"log"
-	"reflect"
-	"strings"
-	"unsafe"
-
-	"github.com/Masterminds/squirrel"
-	"github.com/navidrome/navidrome/conf"
-	"github.com/navidrome/navidrome/core/agents"
-	"github.com/navidrome/navidrome/model"
-	"github.com/navidrome/navidrome/tests"
-	"github.com/navidrome/navidrome/utils/str"
-	. "github.com/onsi/ginkgo/v2"
-	. "github.com/onsi/gomega"
-)
-
-// Custom implementation of ArtistRepository for testing
-type customArtistRepo struct {
-	model.ArtistRepository
-	data map[string]*model.Artist
-	err  bool
-}
-
-func newCustomArtistRepo() *customArtistRepo {
-	return &customArtistRepo{
-		data: make(map[string]*model.Artist),
-	}
-}
-
-func (m *customArtistRepo) SetError(err bool) {
-	m.err = err
-}
-
-func (m *customArtistRepo) SetData(artists model.Artists) {
-	m.data = make(map[string]*model.Artist)
-	for i, a := range artists {
-		m.data[a.ID] = &artists[i]
-	}
-}
-
-// Key implementation needed for the test
-func (m *customArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, error) {
-	if m.err {
-		return nil, errors.New("error")
-	}
-
-	// No filters means return all
-	if len(options) == 0 || options[0].Filters == nil {
-		result := make(model.Artists, 0, len(m.data))
-		for _, a := range m.data {
-			result = append(result, *a)
-		}
-		return result, nil
-	}
-
-	// Handle filter by name (for findArtistByName)
-	if len(options) > 0 && options[0].Filters != nil {
-		switch filter := options[0].Filters.(type) {
-		case squirrel.Like:
-			if nameFilter, ok := filter["artist.name"]; ok {
-				name := strings.Trim(nameFilter.(string), "%")
-				log.Printf("ArtistRepo.GetAll: Looking for artist by name: %s", name)
-
-				for _, a := range m.data {
-					log.Printf("ArtistRepo.GetAll: Comparing with artist: %s", a.Name)
-					if a.Name == name {
-						log.Printf("ArtistRepo.GetAll: Found artist: %s (ID: %s)", a.Name, a.ID)
-						return model.Artists{*a}, nil
-					}
-				}
-			}
-		}
-	}
-
-	log.Println("ArtistRepo.GetAll: No matching artist found")
-	return model.Artists{}, nil
-}
-
-// Custom implementation of MediaFileRepository for testing
-type customMediaFileRepo struct {
-	model.MediaFileRepository
-	data map[string]*model.MediaFile
-	err  bool
-}
-
-func newCustomMediaFileRepo() *customMediaFileRepo {
-	return &customMediaFileRepo{
-		data: make(map[string]*model.MediaFile),
-	}
-}
-
-func (m *customMediaFileRepo) SetError(err bool) {
-	m.err = err
-}
-
-func (m *customMediaFileRepo) SetData(mediaFiles model.MediaFiles) {
-	m.data = make(map[string]*model.MediaFile)
-	for i, mf := range mediaFiles {
-		m.data[mf.ID] = &mediaFiles[i]
-	}
-}
-
-// Key implementation needed for the test
-func (m *customMediaFileRepo) GetAll(options ...model.QueryOptions) (model.MediaFiles, error) {
-	if m.err {
-		return nil, errors.New("error")
-	}
-
-	// No filters means return all
-	if len(options) == 0 || options[0].Filters == nil {
-		result := make(model.MediaFiles, 0, len(m.data))
-		for _, mf := range m.data {
-			result = append(result, *mf)
-		}
-		return result, nil
-	}
-
-	// Check if we're searching by MBID
-	if len(options) > 0 && options[0].Filters != nil {
-		// Log all filter types
-		log.Printf("MediaFileRepo.GetAll: Filter type: %T", options[0].Filters)
-
-		switch filter := options[0].Filters.(type) {
-		case squirrel.And:
-			log.Printf("MediaFileRepo.GetAll: Processing AND filter with %d conditions", len(filter))
-
-			// First check if there's a mbz_recording_id in one of the AND conditions
-			for i, cond := range filter {
-				log.Printf("MediaFileRepo.GetAll: AND condition %d is of type %T", i, cond)
-
-				if eq, ok := cond.(squirrel.Eq); ok {
-					log.Printf("MediaFileRepo.GetAll: Eq condition: %+v", eq)
-
-					if mbid, hasMbid := eq["mbz_recording_id"]; hasMbid {
-						log.Printf("MediaFileRepo.GetAll: Looking for MBID: %s", mbid)
-
-						for _, mf := range m.data {
-							if mf.MbzReleaseTrackID == mbid.(string) && !mf.Missing {
-								log.Printf("MediaFileRepo.GetAll: Found match by MBID: %s (Title: %s)", mf.ID, mf.Title)
-								return model.MediaFiles{*mf}, nil
-							}
-						}
-					}
-				}
-			}
-
-			// Otherwise, find by artist ID and title
-			var artistMatches model.MediaFiles
-			var titleMatches model.MediaFiles
-			var notMissingMatches model.MediaFiles
-
-			// Get non-missing files
-			for _, mf := range m.data {
-				if !mf.Missing {
-					notMissingMatches = append(notMissingMatches, *mf)
-				}
-			}
-
-			log.Printf("MediaFileRepo.GetAll: Found %d non-missing files", len(notMissingMatches))
-
-			for i, cond := range filter {
-				log.Printf("MediaFileRepo.GetAll: Processing condition %d of type %T", i, cond)
-
-				switch subFilter := cond.(type) {
-				case squirrel.Or:
-					log.Printf("MediaFileRepo.GetAll: Processing OR condition with %d subconditions", len(subFilter))
-
-					// Check for artist_id
-					for j, orCond := range subFilter {
-						log.Printf("MediaFileRepo.GetAll: OR subcondition %d is of type %T", j, orCond)
-
-						if eq, ok := orCond.(squirrel.Eq); ok {
-							log.Printf("MediaFileRepo.GetAll: Eq condition: %+v", eq)
-
-							if artistID, ok := eq["artist_id"]; ok {
-								log.Printf("MediaFileRepo.GetAll: Looking for artist_id: %s", artistID)
-
-								for _, mf := range notMissingMatches {
-									if mf.ArtistID == artistID.(string) {
-										log.Printf("MediaFileRepo.GetAll: Found match by artist_id: %s (Title: %s)", mf.ID, mf.Title)
-										artistMatches = append(artistMatches, mf)
-									}
-								}
-							}
-						}
-					}
-				case squirrel.Like:
-					log.Printf("MediaFileRepo.GetAll: Processing LIKE condition: %+v", subFilter)
-
-					// Check for title match
-					if orderTitle, ok := subFilter["order_title"]; ok {
-						normalizedTitle := str.SanitizeFieldForSorting(orderTitle.(string))
-						log.Printf("MediaFileRepo.GetAll: Looking for normalized title: %s", normalizedTitle)
-
-						for _, mf := range notMissingMatches {
-							normalizedMfTitle := str.SanitizeFieldForSorting(mf.Title)
-							log.Printf("MediaFileRepo.GetAll: Comparing with title: %s (normalized: %s)", mf.Title, normalizedMfTitle)
-
-							if normalizedTitle == normalizedMfTitle {
-								log.Printf("MediaFileRepo.GetAll: Found title match: %s", mf.ID)
-								titleMatches = append(titleMatches, mf)
-							}
-						}
-					}
-				}
-			}
-
-			log.Printf("MediaFileRepo.GetAll: Found %d artist matches and %d title matches", len(artistMatches), len(titleMatches))
-
-			// Find records that match both artist and title
-			var results model.MediaFiles
-			for _, am := range artistMatches {
-				for _, tm := range titleMatches {
-					if am.ID == tm.ID {
-						log.Printf("MediaFileRepo.GetAll: Found complete match: %s", am.ID)
-						results = append(results, am)
-					}
-				}
-			}
-
-			if len(results) > 0 {
-				// Apply Max if specified
-				if options[0].Max > 0 && len(results) > options[0].Max {
-					results = results[:options[0].Max]
-				}
-				log.Printf("MediaFileRepo.GetAll: Returning %d results", len(results))
-				return results, nil
-			}
-		case squirrel.Eq:
-			log.Printf("MediaFileRepo.GetAll: Processing Eq filter: %+v", filter)
-
-			// Handle direct MBID lookup
-			if mbid, ok := filter["mbz_recording_id"]; ok {
-				log.Printf("MediaFileRepo.GetAll: Looking for MBID: %s", mbid)
-
-				for _, mf := range m.data {
-					if mf.MbzReleaseTrackID == mbid.(string) && !mf.Missing {
-						log.Printf("MediaFileRepo.GetAll: Found match by MBID: %s (Title: %s)", mf.ID, mf.Title)
-						return model.MediaFiles{*mf}, nil
-					}
-				}
-			}
-		}
-	}
-
-	log.Println("MediaFileRepo.GetAll: No matches found")
-	return model.MediaFiles{}, nil
-}
-
-// Mock Agent implementation
-type MockAgent struct {
-	agents.Interface
-	topSongs []agents.Song
-	err      error
-}
-
-func (m *MockAgent) AgentName() string {
-	return "mock"
-}
-
-func (m *MockAgent) GetArtistTopSongs(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
-	log.Printf("MockAgent.GetArtistTopSongs called: id=%s, name=%s, mbid=%s, count=%d", id, artistName, mbid, count)
-
-	if m.err != nil {
-		log.Printf("MockAgent.GetArtistTopSongs returning error: %v", m.err)
-		return nil, m.err
-	}
-
-	log.Printf("MockAgent.GetArtistTopSongs returning %d songs", len(m.topSongs))
-	return m.topSongs, nil
-}
-
-// Ensure MockAgent implements the necessary interface
-var _ agents.ArtistTopSongsRetriever = (*MockAgent)(nil)
-
-// Sets unexported fields in a struct using reflection and unsafe package
-func setStructField(obj interface{}, fieldName string, value interface{}) {
-	v := reflect.ValueOf(obj).Elem()
-	f := v.FieldByName(fieldName)
-	rf := reflect.NewAt(f.Type(), unsafe.Pointer(f.UnsafeAddr())).Elem()
-	rf.Set(reflect.ValueOf(value))
-}
-
-var _ = Describe("CustomExternalMetadata", func() {
-	var ctx context.Context
-
-	BeforeEach(func() {
-		ctx = context.Background()
-	})
-
-	Describe("DirectTopSongs", func() {
-		var mockArtistRepo *customArtistRepo
-		var mockMediaFileRepo *customMediaFileRepo
-		var mockDS *tests.MockDataStore
-		var mockAgent *MockAgent
-		var agentsImpl *agents.Agents
-		var em ExternalMetadata
-
-		BeforeEach(func() {
-			// Create custom mock repositories
-			mockArtistRepo = newCustomArtistRepo()
-			mockMediaFileRepo = newCustomMediaFileRepo()
-
-			// Configure mock data
-			artist := model.Artist{ID: "artist-1", Name: "Artist One"}
-			mockArtistRepo.SetData(model.Artists{artist})
-
-			log.Printf("Test: Set up artist: %s (ID: %s)", artist.Name, artist.ID)
-
-			mediaFiles := []model.MediaFile{
-				{
-					ID:                "song-1",
-					Title:             "Song One",
-					Artist:            "Artist One",
-					ArtistID:          "artist-1",
-					MbzReleaseTrackID: "mbid-1",
-					Missing:           false,
-				},
-				{
-					ID:                "song-2",
-					Title:             "Song Two",
-					Artist:            "Artist One",
-					ArtistID:          "artist-1",
-					MbzReleaseTrackID: "mbid-2",
-					Missing:           false,
-				},
-			}
-			mockMediaFileRepo.SetData(model.MediaFiles(mediaFiles))
-
-			for _, mf := range mediaFiles {
-				log.Printf("Test: Set up media file: %s (ID: %s, MBID: %s, ArtistID: %s)",
-					mf.Title, mf.ID, mf.MbzReleaseTrackID, mf.ArtistID)
-			}
-
-			// Create mock datastore
-			mockDS = &tests.MockDataStore{
-				MockedArtist:    mockArtistRepo,
-				MockedMediaFile: mockMediaFileRepo,
-			}
-
-			// Create mock agent
-			mockAgent = &MockAgent{
-				topSongs: []agents.Song{
-					{Name: "Song One", MBID: "mbid-1"},
-					{Name: "Song Two", MBID: "mbid-2"},
-				},
-			}
-
-			// Use the real agents.Agents implementation but with our mock agent
-			agentsImpl = &agents.Agents{}
-
-			// Set unexported fields using reflection and unsafe
-			setStructField(agentsImpl, "ds", mockDS)
-			setStructField(agentsImpl, "agents", []agents.Interface{mockAgent})
-
-			// Create our service under test
-			em = NewExternalMetadata(mockDS, agentsImpl)
-		})
-
-		It("returns matching songs from the artist results", func() {
-			log.Printf("Test: Calling TopSongs for artist: %s", "Artist One")
-			songs, err := em.TopSongs(ctx, "Artist One", 5)
-
-			log.Printf("Test: Result: error=%v, songs=%v", err, songs)
-
-			Expect(err).ToNot(HaveOccurred())
-			Expect(songs).To(HaveLen(2))
-			Expect(songs[0].ID).To(Equal("song-1"))
-			Expect(songs[1].ID).To(Equal("song-2"))
-		})
-	})
-
-	Describe("TopSongs with agent registration", func() {
-		var mockArtistRepo *customArtistRepo
-		var mockMediaFileRepo *customMediaFileRepo
-		var mockDS *tests.MockDataStore
-		var mockAgent *MockAgent
-		var em ExternalMetadata
-		var originalAgentsConfig string
-
-		BeforeEach(func() {
-			// Store the original config to restore it later
-			originalAgentsConfig = conf.Server.Agents
-
-			// Set our mock agent as the only agent
-			conf.Server.Agents = "mock"
-
-			// Clear the agents map to prevent interference
-			agents.Map = nil
-
-			// Create custom mock repositories
-			mockArtistRepo = newCustomArtistRepo()
-			mockMediaFileRepo = newCustomMediaFileRepo()
-
-			// Configure mock data
-			artist := model.Artist{ID: "artist-1", Name: "Artist One"}
-			mockArtistRepo.SetData(model.Artists{artist})
-
-			log.Printf("Test: Set up artist: %s (ID: %s)", artist.Name, artist.ID)
-
-			mediaFiles := []model.MediaFile{
-				{
-					ID:                "song-1",
-					Title:             "Song One",
-					Artist:            "Artist One",
-					ArtistID:          "artist-1",
-					MbzReleaseTrackID: "mbid-1",
-					Missing:           false,
-				},
-				{
-					ID:                "song-2",
-					Title:             "Song Two",
-					Artist:            "Artist One",
-					ArtistID:          "artist-1",
-					MbzReleaseTrackID: "mbid-2",
-					Missing:           false,
-				},
-			}
-			mockMediaFileRepo.SetData(model.MediaFiles(mediaFiles))
-
-			for _, mf := range mediaFiles {
-				log.Printf("Test: Set up media file: %s (ID: %s, MBID: %s, ArtistID: %s)",
-					mf.Title, mf.ID, mf.MbzReleaseTrackID, mf.ArtistID)
-			}
-
-			// Create mock datastore
-			mockDS = &tests.MockDataStore{
-				MockedArtist:    mockArtistRepo,
-				MockedMediaFile: mockMediaFileRepo,
-			}
-
-			// Create and register a mock agent
-			mockAgent = &MockAgent{
-				topSongs: []agents.Song{
-					{Name: "Song One", MBID: "mbid-1"},
-					{Name: "Song Two", MBID: "mbid-2"},
-				},
-			}
-
-			// Register our mock agent - this is key to making it available
-			agents.Register("mock", func(model.DataStore) agents.Interface { return mockAgent })
-
-			// Dump the registered agents for debugging
-			log.Printf("Test: Registered agents:")
-			for name := range agents.Map {
-				log.Printf("  - %s", name)
-			}
-
-			// Create the service to test
-			log.Printf("Test: Creating ExternalMetadata with conf.Server.Agents=%s", conf.Server.Agents)
-			em = NewExternalMetadata(mockDS, agents.GetAgents(mockDS))
-		})
-
-		AfterEach(func() {
-			conf.Server.Agents = originalAgentsConfig
-		})
-
-		It("returns matching songs from the registered agent", func() {
-			log.Printf("Test: Calling TopSongs for artist: %s", "Artist One")
-			songs, err := em.TopSongs(ctx, "Artist One", 5)
-
-			log.Printf("Test: Result: error=%v, songs=%v", err, songs)
-
-			Expect(err).ToNot(HaveOccurred())
-			Expect(songs).To(HaveLen(2))
-			Expect(songs[0].ID).To(Equal("song-1"))
-			Expect(songs[1].ID).To(Equal("song-2"))
-		})
-	})
-})

core/external_metadata_test.go

@@ -9,7 +9,11 @@ import (
 	"unsafe"
 
 	"github.com/Masterminds/squirrel"
+	"github.com/navidrome/navidrome/conf"
 	"github.com/navidrome/navidrome/core/agents"
+	_ "github.com/navidrome/navidrome/core/agents/lastfm"
+	_ "github.com/navidrome/navidrome/core/agents/listenbrainz"
+	_ "github.com/navidrome/navidrome/core/agents/spotify"
 	"github.com/navidrome/navidrome/log"
 	"github.com/navidrome/navidrome/model"
 	"github.com/navidrome/navidrome/tests"
@@ -40,14 +44,19 @@ func (m *mockArtistTopSongsAgent) GetArtistTopSongs(ctx context.Context, id, art
 	m.lastArtistName = artistName
 	m.lastMBID = mbid
 
+	log.Debug(ctx, "MockAgent.GetArtistTopSongs called", "id", id, "name", artistName, "mbid", mbid, "count", count)
+
 	// Use the custom function if available
 	if m.getArtistTopSongsFn != nil {
 		return m.getArtistTopSongsFn(ctx, id, artistName, mbid, count)
 	}
 
 	if m.err != nil {
+		log.Debug(ctx, "MockAgent.GetArtistTopSongs returning error", "err", m.err)
 		return nil, m.err
 	}
+
+	log.Debug(ctx, "MockAgent.GetArtistTopSongs returning songs", "count", len(m.topSongs))
 	return m.topSongs, nil
 }
 
@@ -92,6 +101,11 @@ func (m *testArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e
 		return nil, errors.New("error")
 	}
 
+	// No filters means return all
+	if len(options) == 0 || options[0].Filters == nil {
+		return m.artists, nil
+	}
+
 	// Basic implementation that returns artists matching name filter
 	if len(options) > 0 && options[0].Filters != nil {
 		switch f := options[0].Filters.(type) {
@@ -108,6 +122,18 @@ func (m *testArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e
 					}
 				}
 			}
+		case squirrel.Eq:
+			if ids, ok := f["artist.id"]; ok {
+				var result model.Artists
+				for _, a := range m.artists {
+					for _, id := range ids.([]string) {
+						if a.ID == id {
+							result = append(result, a)
+						}
+					}
+				}
+				return result, nil
+			}
 		}
 	}
 
@@ -208,6 +234,15 @@ func (m *testMediaFileRepo) handleAndFilter(andFilter squirrel.And, option model
 							}
 						}
 					}
+					if albumArtistID, ok := eqCond["album_artist_id"]; ok {
+						log.Debug("MediaFileRepo.handleAndFilter: Looking for album_artist_id", "albumArtistID", albumArtistID)
+						for _, mf := range notMissingMatches {
+							if mf.AlbumArtistID == albumArtistID.(string) {
+								log.Debug("MediaFileRepo.handleAndFilter: Found match by album_artist_id", "id", mf.ID, "title", mf.Title)
+								artistMatches = append(artistMatches, mf)
+							}
+						}
+					}
 				}
 			}
 		case squirrel.Like:
@@ -225,6 +260,12 @@ func (m *testMediaFileRepo) handleAndFilter(andFilter squirrel.And, option model
 					}
 				}
 			}
+		case squirrel.Eq:
+			// Handle missing check
+			if missingFlag, ok := filter["missing"]; ok && !missingFlag.(bool) {
+				// This is already handled above when we build notMissingMatches
+				continue
+			}
 		}
 	}
 
@@ -257,10 +298,14 @@ var _ = Describe("ExternalMetadata", func() {
 	var mockArtistRepo *testArtistRepo
 	var mockMediaFileRepo *testMediaFileRepo
 	var ctx context.Context
+	var originalAgentsConfig string
 
 	BeforeEach(func() {
 		ctx = context.Background()
 
+		// Store the original agents config to restore it later
+		originalAgentsConfig = conf.Server.Agents
+
 		// Setup mocks
 		mockArtistRepo = newTestArtistRepo()
 		mockMediaFileRepo = newTestMediaFileRepo()
@@ -275,31 +320,21 @@ var _ = Describe("ExternalMetadata", func() {
 
 		// Create a mock agent
 		mockAgent = &mockArtistTopSongsAgent{}
-		log.Debug("Creating mock agent", "agent", mockAgent)
+		log.Debug(ctx, "Creating mock agent", "agent", mockAgent)
-		agents.Register("mock", func(model.DataStore) agents.Interface { return mockAgent })
-
-		// Create a custom agents instance directly with our mock agent
-		agentsImpl := &agents.Agents{}
-
-		// Use reflection to set the unexported fields
-		setAgentField(agentsImpl, "ds", ds)
-		setAgentField(agentsImpl, "agents", []agents.Interface{mockAgent})
-
-		// Create the externalMetadata instance with our custom Agents implementation
-		em = NewExternalMetadata(ds, agentsImpl)
-
-		// Verify that the agent is available
-		log.Debug("ExternalMetadata created", "em", em)
 	})
 
-	Describe("TopSongs", func() {
+	AfterEach(func() {
+		// Restore original config
+		conf.Server.Agents = originalAgentsConfig
+	})
+
+	Describe("TopSongs with direct agent injection", func() {
 		BeforeEach(func() {
 			// Set up artists data
 			mockArtistRepo.SetData(model.Artists{
 				{ID: "artist-1", Name: "Artist One"},
 				{ID: "artist-2", Name: "Artist Two"},
 			})
-			log.Debug("Artist data set up", "count", 2)
 
 			// Set up mediafiles data with all necessary fields for matching
 			mockMediaFileRepo.SetData(model.MediaFiles{
@@ -331,23 +366,27 @@ var _ = Describe("ExternalMetadata", func() {
 					Missing:           false,
 				},
 			})
-			log.Debug("Media files data set up", "count", 3)
 
 			// Configure the mockAgent to return some top songs
 			mockAgent.topSongs = []agents.Song{
 				{Name: "Song One", MBID: "mbid-1"},
 				{Name: "Song Two", MBID: "mbid-2"},
 			}
-			log.Debug("Mock agent configured with top songs", "count", len(mockAgent.topSongs))
+
+			// Create a custom agents instance directly with our mock agent
+			agentsImpl := &agents.Agents{}
+
+			// Use reflection to set the unexported fields
+			setAgentField(agentsImpl, "ds", ds)
+			setAgentField(agentsImpl, "agents", []agents.Interface{mockAgent})
+
+			// Create the externalMetadata instance with our custom Agents implementation
+			em = NewExternalMetadata(ds, agentsImpl)
 		})
 
 		It("returns matching songs from the agent results", func() {
-			log.Debug("Running test: returns matching songs from the agent results")
-
 			songs, err := em.TopSongs(ctx, "Artist One", 5)
 
-			log.Debug("Test results", "err", err, "songs", songs)
-
 			Expect(err).ToNot(HaveOccurred())
 			Expect(songs).To(HaveLen(2))
 			Expect(songs[0].ID).To(Equal("song-1"))
@@ -360,13 +399,11 @@ var _ = Describe("ExternalMetadata", func() {
 		})
 
 		It("returns nil when artist is not found", func() {
-			// Empty mockArtistRepo to simulate artist not found
+			// Set an error for mockArtistRepo to simulate artist not found
-			mockArtistRepo.err = errors.New("artist repo error")
+			mockArtistRepo.SetError(true)
 
 			songs, err := em.TopSongs(ctx, "Unknown Artist", 5)
 
-			log.Debug("Test results after TopSongs call with unknown artist", "err", err, "songs", songs)
-
 			Expect(err).To(BeNil())
 			Expect(songs).To(BeNil())
 		})
@@ -379,111 +416,21 @@ var _ = Describe("ExternalMetadata", func() {
 
 			songs, err := em.TopSongs(ctx, "Artist One", 5)
 
-			log.Debug("Test results for non-matching songs", "err", err, "songs", songs)
-
 			Expect(err).ToNot(HaveOccurred())
 			Expect(songs).To(HaveLen(0))
 		})
 
-		It("returns nil when agent returns other errors", func() {
+		It("returns nil when agent returns errors", func() {
-			// We need to ensure artist is found first
+			// Reset the agent error state
-			mockArtistRepo.err = nil
+			mockArtistRepo.SetError(false)
 
-			// Create the error right away
+			// Set the error
 			testError := errors.New("some agent error")
-
-			// Reset the default mock agent
 			mockAgent.err = testError
-			mockAgent.topSongs = nil // Make sure no songs are returned with the error
 
-			log.Debug("==================== TEST SETUP ====================")
+			songs, err := em.TopSongs(ctx, "Artist One", 5)
-			log.Debug("Using default mock agent for this test", "agent", mockAgent, "err", mockAgent.err)
 
-			// Directly test the mock agent's GetArtistTopSongs function
+			// Current behavior returns nil for both error and songs
-			songs, err := mockAgent.GetArtistTopSongs(ctx, "1", "Artist One", "mbz-1", 5)
-			log.Debug("Direct GetArtistTopSongs result", "songs", songs, "err", err)
-			Expect(err).To(Equal(testError))
-
-			// Directly test the agents.Agents implementation to check how it handles errors
-			agentsObj := &agents.Agents{}
-			setAgentField(agentsObj, "ds", ds)
-			setAgentField(agentsObj, "agents", []agents.Interface{mockAgent})
-
-			// Test the wrapped agent directly
-			songs, err = agentsObj.GetArtistTopSongs(ctx, "1", "Artist One", "mbz-1", 5)
-			log.Debug("Agents.GetArtistTopSongs result", "songs", songs, "err", err)
-			// If Agents.GetArtistTopSongs swallows errors and returns ErrNotFound, that's an issue with agents
-			// but we're still testing the current behavior
-
-			// Create a direct agent that returns the error directly from GetArtistTopSongs
-			directAgent := &mockArtistTopSongsAgent{
-				getArtistTopSongsFn: func(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
-					return nil, testError
-				},
-			}
-
-			// Create the ExternalMetadata instance with a direct pipeline to our agent
-			directAgentsObj := &agents.Agents{}
-			setAgentField(directAgentsObj, "ds", ds)
-			setAgentField(directAgentsObj, "agents", []agents.Interface{directAgent})
-
-			// Create the externalMetadata instance to test
-			directEM := NewExternalMetadata(ds, directAgentsObj)
-
-			// Call the method we're testing with our direct agent setup
-			songs2, err := directEM.TopSongs(ctx, "Artist One", 5)
-
-			log.Debug("Direct TopSongs result", "err", err, "songs", songs2)
-
-			// With our improved code, the error should now be propagated if it's passed directly from the agent
-			// But we keep this test in its original form to ensure the current behavior works
-			// A new test will be added that tests the improved error propagation
-			Expect(err).To(BeNil())
-			Expect(songs2).To(BeNil())
-		})
-
-		It("propagates errors with direct agent implementation", func() {
-			// We need to ensure artist is found first
-			mockArtistRepo.err = nil
-
-			// Create the error right away
-			testError := errors.New("direct agent error")
-
-			// Create a direct agent that bypasses agents.Agents
-			// This simulates a case where the error would be properly propagated if agents.Agents
-			// wasn't silently swallowing errors
-			directAgent := &mockArtistTopSongsAgent{
-				getArtistTopSongsFn: func(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
-					log.Debug("Direct agent GetArtistTopSongs called", "id", id, "name", artistName, "count", count)
-					return nil, testError
-				},
-			}
-
-			// Check that our direct agent works as expected
-			songsTest, errTest := directAgent.GetArtistTopSongs(ctx, "1", "Artist One", "mbz-1", 5)
-			log.Debug("Testing direct agent", "songs", songsTest, "err", errTest)
-			Expect(errTest).To(Equal(testError))
-
-			// Create a custom implementation of agents.Agents that will return our error
-			directAgentsImpl := &agents.Agents{}
-			setAgentField(directAgentsImpl, "ds", ds)
-			setAgentField(directAgentsImpl, "agents", []agents.Interface{directAgent})
-
-			// Test the agents implementation directly
-			songsAgents, errAgents := directAgentsImpl.GetArtistTopSongs(ctx, "1", "Artist One", "mbz-1", 5)
-			log.Debug("Direct agents result", "songs", songsAgents, "err", errAgents)
-
-			// Create a new external metadata instance
-			directEM := NewExternalMetadata(ds, directAgentsImpl)
-
-			// Call the method we're testing with our direct implementation
-			songs, err := directEM.TopSongs(ctx, "Artist One", 5)
-
-			log.Debug("Direct TopSongs result with propagation", "err", err, "songs", songs)
-
-			// In theory this would pass with the improved implementation, but in practice
-			// the root issue is the agents.Agents implementation that swallows non-ErrNotFound errors
-			// For now we'll expect nil, which matches the current behavior
 			Expect(err).To(BeNil())
 			Expect(songs).To(BeNil())
 		})
@@ -497,11 +444,95 @@ var _ = Describe("ExternalMetadata", func() {
 
 			songs, err := em.TopSongs(ctx, "Artist One", 1)
 
-			log.Debug("Test results for count parameter", "err", err, "songs", songs, "count", len(songs))
-
 			Expect(err).ToNot(HaveOccurred())
 			Expect(songs).To(HaveLen(1))
 			Expect(songs[0].ID).To(Equal("song-1"))
 		})
 	})
+
+	Describe("TopSongs with agent registration", func() {
+		BeforeEach(func() {
+			// Set our mock agent as the only agent
+			conf.Server.Agents = "mock"
+
+			// Set up artists data
+			mockArtistRepo.SetData(model.Artists{
+				{ID: "artist-1", Name: "Artist One"},
+			})
+
+			// Set up mediafiles data
+			mockMediaFileRepo.SetData(model.MediaFiles{
+				{
+					ID:                "song-1",
+					Title:             "Song One",
+					Artist:            "Artist One",
+					ArtistID:          "artist-1",
+					MbzReleaseTrackID: "mbid-1",
+					Missing:           false,
+				},
+				{
+					ID:                "song-2",
+					Title:             "Song Two",
+					Artist:            "Artist One",
+					ArtistID:          "artist-1",
+					MbzReleaseTrackID: "mbid-2",
+					Missing:           false,
+				},
+			})
+
+			// Configure and register the agent
+			mockAgent.topSongs = []agents.Song{
+				{Name: "Song One", MBID: "mbid-1"},
+				{Name: "Song Two", MBID: "mbid-2"},
+			}
+
+			// Register our mock agent
+			agents.Register("mock", func(model.DataStore) agents.Interface { return mockAgent })
+
+			// Create the externalMetadata instance with registered agents
+			em = NewExternalMetadata(ds, agents.GetAgents(ds))
+		})
+
+		It("returns matching songs from the registered agent", func() {
+			songs, err := em.TopSongs(ctx, "Artist One", 5)
+
+			Expect(err).ToNot(HaveOccurred())
+			Expect(songs).To(HaveLen(2))
+			Expect(songs[0].ID).To(Equal("song-1"))
+			Expect(songs[1].ID).To(Equal("song-2"))
+		})
+	})
+
+	Describe("Error propagation from agents", func() {
+		BeforeEach(func() {
+			// Set up artists data
+			mockArtistRepo.SetData(model.Artists{
+				{ID: "artist-1", Name: "Artist One"},
+			})
+
+			// Create a direct agent that returns an error
+			testError := errors.New("direct agent error")
+			directAgent := &mockArtistTopSongsAgent{
+				getArtistTopSongsFn: func(ctx context.Context, id, artistName, mbid string, count int) ([]agents.Song, error) {
+					return nil, testError
+				},
+			}
+
+			// Create a custom implementation of agents.Agents that will return our error
+			directAgentsImpl := &agents.Agents{}
+			setAgentField(directAgentsImpl, "ds", ds)
+			setAgentField(directAgentsImpl, "agents", []agents.Interface{directAgent})
+
+			// Create a new external metadata instance
+			em = NewExternalMetadata(ds, directAgentsImpl)
+		})
+
+		It("handles errors from the agent according to current behavior", func() {
+			songs, err := em.TopSongs(ctx, "Artist One", 5)
+
+			// Current behavior returns nil for both error and songs
+			Expect(err).To(BeNil())
+			Expect(songs).To(BeNil())
+		})
+	})
 })