diff --git a/core/extdata/provider_helper_test.go b/core/extdata/provider_helper_test.go index 2034da8fa..ef4d0de62 100644 --- a/core/extdata/provider_helper_test.go +++ b/core/extdata/provider_helper_test.go @@ -186,15 +186,15 @@ func (m *mockSimilarArtistAgent) GetSimilarArtists(ctx context.Context, id, name // mockAgents mocks the main Agents interface used by Provider type mockAgents struct { - mock.Mock // Embed testify mock - topSongsAgent agents.ArtistTopSongsRetriever - similarAgent agents.ArtistSimilarRetriever - imageAgent agents.ArtistImageRetriever - albumInfoAgent agents.AlbumInfoRetriever - bioAgent agents.ArtistBiographyRetriever // Added field for clarity - mbidAgent agents.ArtistMBIDRetriever // Added field for clarity - urlAgent agents.ArtistURLRetriever // Added field for clarity - agents.Interface // Embed to satisfy non-overridden methods + mock.Mock // Embed testify mock + topSongsAgent agents.ArtistTopSongsRetriever + similarAgent agents.ArtistSimilarRetriever + imageAgent agents.ArtistImageRetriever + albumInfoAgent agents.AlbumInfoRetriever + bioAgent agents.ArtistBiographyRetriever + mbidAgent agents.ArtistMBIDRetriever + urlAgent agents.ArtistURLRetriever + agents.Interface } func (m *mockAgents) AgentName() string { @@ -205,7 +205,6 @@ func (m *mockAgents) GetSimilarArtists(ctx context.Context, id, name, mbid strin if m.similarAgent != nil { return m.similarAgent.GetSimilarArtists(ctx, id, name, mbid, limit) } - // Fallback to testify mock args := m.Called(ctx, id, name, mbid, limit) if args.Get(0) != nil { return args.Get(0).([]agents.Artist), args.Error(1) @@ -217,7 +216,6 @@ func (m *mockAgents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid if m.topSongsAgent != nil { return m.topSongsAgent.GetArtistTopSongs(ctx, id, artistName, mbid, count) } - // Fallback to testify mock args := m.Called(ctx, id, artistName, mbid, count) if args.Get(0) != nil { return args.Get(0).([]agents.Song), args.Error(1) @@ -225,13 +223,10 @@ func (m *mockAgents) GetArtistTopSongs(ctx context.Context, id, artistName, mbid return nil, args.Error(1) } -// --- Stubs for other Agents interface methods --- - func (m *mockAgents) GetAlbumInfo(ctx context.Context, name, artist, mbid string) (*agents.AlbumInfo, error) { if m.albumInfoAgent != nil { return m.albumInfoAgent.GetAlbumInfo(ctx, name, artist, mbid) } - // Fallback to testify mock args := m.Called(ctx, name, artist, mbid) if args.Get(0) != nil { return args.Get(0).(*agents.AlbumInfo), args.Error(1) @@ -243,7 +238,6 @@ func (m *mockAgents) GetArtistMBID(ctx context.Context, id string, name string) if m.mbidAgent != nil { return m.mbidAgent.GetArtistMBID(ctx, id, name) } - // Fallback to testify mock args := m.Called(ctx, id, name) return args.String(0), args.Error(1) } @@ -252,7 +246,6 @@ func (m *mockAgents) GetArtistURL(ctx context.Context, id, name, mbid string) (s if m.urlAgent != nil { return m.urlAgent.GetArtistURL(ctx, id, name, mbid) } - // Fallback to testify mock args := m.Called(ctx, id, name, mbid) return args.String(0), args.Error(1) } @@ -261,7 +254,6 @@ func (m *mockAgents) GetArtistBiography(ctx context.Context, id, name, mbid stri if m.bioAgent != nil { return m.bioAgent.GetArtistBiography(ctx, id, name, mbid) } - // Fallback to testify mock args := m.Called(ctx, id, name, mbid) return args.String(0), args.Error(1) } @@ -270,7 +262,6 @@ func (m *mockAgents) GetArtistImages(ctx context.Context, id, name, mbid string) if m.imageAgent != nil { return m.imageAgent.GetArtistImages(ctx, id, name, mbid) } - // Fallback to testify mock args := m.Called(ctx, id, name, mbid) if args.Get(0) != nil { return args.Get(0).([]agents.ExternalImage), args.Error(1) diff --git a/core/extdata/provider_updatealbuminfo_test.go b/core/extdata/provider_updatealbuminfo_test.go index fd694fcf8..bfa7a5345 100644 --- a/core/extdata/provider_updatealbuminfo_test.go +++ b/core/extdata/provider_updatealbuminfo_test.go @@ -23,10 +23,11 @@ func init() { var _ = Describe("Provider UpdateAlbumInfo", func() { var ( - ctx context.Context - p extdata.Provider - ds *tests.MockDataStore - ag *extdata.MockAgents + ctx context.Context + p extdata.Provider + ds *tests.MockDataStore + ag *extdata.MockAgents + mockAlbumRepo *tests.MockAlbumRepo ) BeforeEach(func() { @@ -34,35 +35,27 @@ var _ = Describe("Provider UpdateAlbumInfo", func() { ds = new(tests.MockDataStore) ag = new(extdata.MockAgents) p = extdata.NewProvider(ds, ag) - - // Default config + mockAlbumRepo = ds.Album(ctx).(*tests.MockAlbumRepo) conf.Server.DevAlbumInfoTimeToLive = 1 * time.Hour }) It("returns error when album is not found", func() { - // MockDataStore will return a MockAlbumRepo with an empty Data map by default - _ = ds.Album(ctx).(*tests.MockAlbumRepo) // Ensure MockAlbumRepo is initialized - album, err := p.UpdateAlbumInfo(ctx, "al-not-found") Expect(err).To(MatchError(model.ErrNotFound)) Expect(album).To(BeNil()) - // No AssertExpectations needed for custom mock ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) It("populates info when album exists but has no external info", func() { - // Setup: Album exists in DS, ExternalInfoUpdatedAt is nil originalAlbum := &model.Album{ ID: "al-existing", Name: "Test Album", AlbumArtist: "Test Artist", MbzAlbumID: "mbid-album", } - mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) mockAlbumRepo.SetData(model.Albums{*originalAlbum}) - // Mock agent response expectedInfo := &agents.AlbumInfo{ URL: "http://example.com/album", Description: "Album Description", @@ -74,10 +67,8 @@ var _ = Describe("Provider UpdateAlbumInfo", func() { } ag.On("GetAlbumInfo", ctx, "Test Album", "Test Artist", "mbid-album").Return(expectedInfo, nil) - // Call the method under test updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-existing") - // Assertions Expect(err).NotTo(HaveOccurred()) Expect(updatedAlbum).NotTo(BeNil()) Expect(updatedAlbum.ID).To(Equal("al-existing")) @@ -87,13 +78,12 @@ var _ = Describe("Provider UpdateAlbumInfo", func() { Expect(updatedAlbum.MediumImageUrl).To(Equal("http://example.com/medium.jpg")) Expect(updatedAlbum.SmallImageUrl).To(Equal("http://example.com/small.jpg")) Expect(updatedAlbum.ExternalInfoUpdatedAt).NotTo(BeNil()) - Expect(*updatedAlbum.ExternalInfoUpdatedAt).To(BeTemporally("~", time.Now(), time.Second)) // Check timestamp was set + Expect(*updatedAlbum.ExternalInfoUpdatedAt).To(BeTemporally("~", time.Now(), time.Second)) ag.AssertExpectations(GinkgoT()) }) It("returns cached info when album exists and info is not expired", func() { - // Setup: Album exists in DS, ExternalInfoUpdatedAt is recent now := time.Now() originalAlbum := &model.Album{ ID: "al-cached", @@ -102,26 +92,22 @@ var _ = Describe("Provider UpdateAlbumInfo", func() { ExternalUrl: "http://cached.com/album", Description: "Cached Desc", LargeImageUrl: "http://cached.com/large.jpg", - ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevAlbumInfoTimeToLive / 2)), // Not expired + ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevAlbumInfoTimeToLive / 2)), } - mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) mockAlbumRepo.SetData(model.Albums{*originalAlbum}) - // Call the method under test updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-cached") - // Assertions Expect(err).NotTo(HaveOccurred()) Expect(updatedAlbum).NotTo(BeNil()) - Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return the exact cached data + Expect(*updatedAlbum).To(Equal(*originalAlbum)) ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) It("returns cached info and triggers background refresh when info is expired", func() { - // Setup: Album exists in DS, ExternalInfoUpdatedAt is old now := time.Now() - expiredTime := now.Add(-conf.Server.DevAlbumInfoTimeToLive * 2) // Definitely expired + expiredTime := now.Add(-conf.Server.DevAlbumInfoTimeToLive * 2) originalAlbum := &model.Album{ ID: "al-expired", Name: "Expired Album", @@ -131,75 +117,54 @@ var _ = Describe("Provider UpdateAlbumInfo", func() { LargeImageUrl: "http://expired.com/large.jpg", ExternalInfoUpdatedAt: gg.P(expiredTime), } - mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) mockAlbumRepo.SetData(model.Albums{*originalAlbum}) - // Call the method under test - // Note: We are not testing the background refresh directly here, only the immediate return updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-expired") - // Assertions: Should return the stale data immediately Expect(err).NotTo(HaveOccurred()) Expect(updatedAlbum).NotTo(BeNil()) - Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return the exact cached (stale) data + Expect(*updatedAlbum).To(Equal(*originalAlbum)) - // Assertions: Agent should NOT be called synchronously ag.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) It("returns error when agent fails to get album info", func() { - // Setup: Album exists in DS, ExternalInfoUpdatedAt is nil originalAlbum := &model.Album{ ID: "al-agent-error", Name: "Agent Error Album", AlbumArtist: "Agent Error Artist", MbzAlbumID: "mbid-agent-error", } - mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) mockAlbumRepo.SetData(model.Albums{*originalAlbum}) - // Mock agent response with an error expectedErr := errors.New("agent communication failed") ag.On("GetAlbumInfo", ctx, "Agent Error Album", "Agent Error Artist", "mbid-agent-error").Return(nil, expectedErr) - // Call the method under test updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-agent-error") - // Assertions Expect(err).To(MatchError(expectedErr)) Expect(updatedAlbum).To(BeNil()) ag.AssertExpectations(GinkgoT()) }) It("returns original album when agent returns ErrNotFound", func() { - // Setup: Album exists in DS, ExternalInfoUpdatedAt is nil originalAlbum := &model.Album{ ID: "al-agent-notfound", Name: "Agent NotFound Album", AlbumArtist: "Agent NotFound Artist", MbzAlbumID: "mbid-agent-notfound", - // ExternalInfoUpdatedAt is nil } - mockAlbumRepo := ds.Album(ctx).(*tests.MockAlbumRepo) mockAlbumRepo.SetData(model.Albums{*originalAlbum}) - // Mock agent response with ErrNotFound ag.On("GetAlbumInfo", ctx, "Agent NotFound Album", "Agent NotFound Artist", "mbid-agent-notfound").Return(nil, agents.ErrNotFound) - // Call the method under test updatedAlbum, err := p.UpdateAlbumInfo(ctx, "al-agent-notfound") - // Assertions - Expect(err).NotTo(HaveOccurred()) // No error should be returned + Expect(err).NotTo(HaveOccurred()) Expect(updatedAlbum).NotTo(BeNil()) - Expect(*updatedAlbum).To(Equal(*originalAlbum)) // Should return original data - Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil()) // Timestamp should not be set + Expect(*updatedAlbum).To(Equal(*originalAlbum)) + Expect(updatedAlbum.ExternalInfoUpdatedAt).To(BeNil()) - // Agent was called, but UpdateExternalInfo should not have been ag.AssertExpectations(GinkgoT()) - // We can't assert mockAlbumRepo.AssertNotCalled for UpdateExternalInfo because it's not a testify mock method }) - - // Test cases will go here - }) diff --git a/core/extdata/provider_updateartistinfo_test.go b/core/extdata/provider_updateartistinfo_test.go index 7d8624127..afda3852c 100644 --- a/core/extdata/provider_updateartistinfo_test.go +++ b/core/extdata/provider_updateartistinfo_test.go @@ -27,7 +27,7 @@ var _ = Describe("Provider UpdateArtistInfo", func() { p extdata.Provider ds *tests.MockDataStore ag *extdata.MockAgents - mockArtistRepo *tests.MockArtistRepo // Convenience variable + mockArtistRepo *tests.MockArtistRepo ) BeforeEach(func() { @@ -35,16 +35,11 @@ var _ = Describe("Provider UpdateArtistInfo", func() { ds = new(tests.MockDataStore) ag = new(extdata.MockAgents) p = extdata.NewProvider(ds, ag) - mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo) - - // Default config conf.Server.DevArtistInfoTimeToLive = 1 * time.Hour }) It("returns error when artist is not found", func() { - // MockArtistRepo.Get returns ErrNotFound by default if data is empty - artist, err := p.UpdateArtistInfo(ctx, "ar-not-found", 10, false) Expect(err).To(MatchError(model.ErrNotFound)) @@ -57,18 +52,14 @@ var _ = Describe("Provider UpdateArtistInfo", func() { }) It("populates info when artist exists but has no external info", func() { - // Setup: Artist exists in DS, ExternalInfoUpdatedAt is nil originalArtist := &model.Artist{ ID: "ar-existing", Name: "Test Artist", - // MbzArtistID is empty initially - // ExternalInfoUpdatedAt is nil } mockArtistRepo.SetData(model.Artists{*originalArtist}) - // Mock agent responses expectedMBID := "mbid-artist-123" - expectedBio := "Artist Bio" // Raw bio from agent + expectedBio := "Artist Bio" expectedURL := "http://artist.url" expectedImages := []agents.ExternalImage{ {URL: "http://large.jpg", Size: 300}, @@ -77,8 +68,8 @@ var _ = Describe("Provider UpdateArtistInfo", func() { } rawSimilar := []agents.Artist{ {Name: "Similar Artist 1", MBID: "mbid-similar-1"}, - {Name: "Similar Artist 2", MBID: "mbid-similar-2"}, // This one exists in DS - {Name: "Similar Artist 3", MBID: "mbid-similar-3"}, // This one doesn't exist + {Name: "Similar Artist 2", MBID: "mbid-similar-2"}, + {Name: "Similar Artist 3", MBID: "mbid-similar-3"}, } similarInDS := model.Artist{ID: "ar-similar-2", Name: "Similar Artist 2"} @@ -88,18 +79,14 @@ var _ = Describe("Provider UpdateArtistInfo", func() { ag.On("GetArtistURL", ctx, "ar-existing", "Test Artist", expectedMBID).Return(expectedURL, nil).Once() ag.On("GetSimilarArtists", ctx, "ar-existing", "Test Artist", expectedMBID, 100).Return(rawSimilar, nil).Once() - // Mock loading similar artists from DS (only finds ar-similar-2) - mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) // Update repo data to include similar + mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) - // Call the method under test (includeNotPresent = false for this part) - updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-existing", 10, false) // 10 similar requested, includeNotPresent=false + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-existing", 10, false) - // Assertions Expect(err).NotTo(HaveOccurred()) Expect(updatedArtist).NotTo(BeNil()) Expect(updatedArtist.ID).To(Equal("ar-existing")) Expect(updatedArtist.MbzArtistID).To(Equal(expectedMBID)) - // Biography gets sanitized and space-replaced in populateArtistInfo Expect(updatedArtist.Biography).To(Equal("Artist Bio")) Expect(updatedArtist.ExternalUrl).To(Equal(expectedURL)) Expect(updatedArtist.LargeImageUrl).To(Equal("http://large.jpg")) @@ -108,17 +95,14 @@ var _ = Describe("Provider UpdateArtistInfo", func() { Expect(updatedArtist.ExternalInfoUpdatedAt).NotTo(BeNil()) Expect(*updatedArtist.ExternalInfoUpdatedAt).To(BeTemporally("~", time.Now(), time.Second)) - // Assert similar artists loaded (only the one present in DS) Expect(updatedArtist.SimilarArtists).To(HaveLen(1)) Expect(updatedArtist.SimilarArtists[0].ID).To(Equal("ar-similar-2")) Expect(updatedArtist.SimilarArtists[0].Name).To(Equal("Similar Artist 2")) ag.AssertExpectations(GinkgoT()) - // Cannot assert UpdateExternalInfo on mockArtistRepo }) It("returns cached info when artist exists and info is not expired", func() { - // Setup: Artist exists in DS, ExternalInfoUpdatedAt is recent now := time.Now() originalArtist := &model.Artist{ ID: "ar-cached", @@ -127,20 +111,17 @@ var _ = Describe("Provider UpdateArtistInfo", func() { ExternalUrl: "http://cached.url", Biography: "Cached Bio", LargeImageUrl: "http://cached_large.jpg", - ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), // Not expired - // Pre-populate with some similar artist IDs for the loading part + ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), SimilarArtists: model.Artists{ {ID: "ar-similar-present", Name: "Similar Present"}, - {ID: "ar-similar-absent", Name: "Similar Absent"}, // This one won't be found in DS + {ID: "ar-similar-absent", Name: "Similar Absent"}, }, } similarInDS := model.Artist{ID: "ar-similar-present", Name: "Similar Present Updated"} mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) - // Call the method under test - updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-cached", 5, false) // 5 similar requested, includeNotPresent=false + updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-cached", 5, false) - // Assertions: Should return the cached data (except similar) Expect(err).NotTo(HaveOccurred()) Expect(updatedArtist).NotTo(BeNil()) Expect(updatedArtist.ID).To(Equal(originalArtist.ID)) @@ -151,28 +132,23 @@ var _ = Describe("Provider UpdateArtistInfo", func() { Expect(updatedArtist.LargeImageUrl).To(Equal(originalArtist.LargeImageUrl)) Expect(updatedArtist.ExternalInfoUpdatedAt).To(Equal(originalArtist.ExternalInfoUpdatedAt)) - // Assert similar artists loaded (only the one found in DS) Expect(updatedArtist.SimilarArtists).To(HaveLen(1)) Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID)) - Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) // Check if it loaded the updated name + Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) - // Assertions: Core info agents should NOT be called ag.AssertNotCalled(GinkgoT(), "GetArtistMBID") ag.AssertNotCalled(GinkgoT(), "GetArtistImages") ag.AssertNotCalled(GinkgoT(), "GetArtistBiography") ag.AssertNotCalled(GinkgoT(), "GetArtistURL") - // GetSimilarArtists *might* be called if similar artists weren't cached, but we check the main ones }) It("returns cached info and triggers background refresh when info is expired", func() { - // Setup: Artist exists in DS, ExternalInfoUpdatedAt is old now := time.Now() - expiredTime := now.Add(-conf.Server.DevArtistInfoTimeToLive * 2) // Definitely expired + expiredTime := now.Add(-conf.Server.DevArtistInfoTimeToLive * 2) originalArtist := &model.Artist{ ID: "ar-expired", Name: "Expired Artist", ExternalInfoUpdatedAt: gg.P(expiredTime), - // Include some similar artists to test loading SimilarArtists: model.Artists{ {ID: "ar-exp-similar", Name: "Expired Similar"}, }, @@ -180,22 +156,18 @@ var _ = Describe("Provider UpdateArtistInfo", func() { similarInDS := model.Artist{ID: "ar-exp-similar", Name: "Expired Similar Updated"} mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) - // Call the method under test updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-expired", 5, false) - // Assertions: Should return the stale cached data Expect(err).NotTo(HaveOccurred()) Expect(updatedArtist).NotTo(BeNil()) Expect(updatedArtist.ID).To(Equal(originalArtist.ID)) Expect(updatedArtist.Name).To(Equal(originalArtist.Name)) Expect(updatedArtist.ExternalInfoUpdatedAt).To(Equal(originalArtist.ExternalInfoUpdatedAt)) - // Assert similar artists loaded Expect(updatedArtist.SimilarArtists).To(HaveLen(1)) Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID)) Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) - // Assertions: Core info agents should NOT be called synchronously ag.AssertNotCalled(GinkgoT(), "GetArtistMBID") ag.AssertNotCalled(GinkgoT(), "GetArtistImages") ag.AssertNotCalled(GinkgoT(), "GetArtistBiography") @@ -203,73 +175,53 @@ var _ = Describe("Provider UpdateArtistInfo", func() { }) It("includes non-present similar artists when includeNotPresent is true", func() { - // Setup: Artist exists with cached similar artists (some present, some not) now := time.Now() originalArtist := &model.Artist{ ID: "ar-similar-test", Name: "Similar Test Artist", - ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), // Not expired + ExternalInfoUpdatedAt: gg.P(now.Add(-conf.Server.DevArtistInfoTimeToLive / 2)), SimilarArtists: model.Artists{ {ID: "ar-sim-present", Name: "Similar Present"}, - {ID: "", Name: "Similar Absent Raw"}, // Already marked as absent in cache - {ID: "ar-sim-absent-lookup", Name: "Similar Absent Lookup"}, // Will fail DS lookup + {ID: "", Name: "Similar Absent Raw"}, + {ID: "ar-sim-absent-lookup", Name: "Similar Absent Lookup"}, }, } similarInDS := model.Artist{ID: "ar-sim-present", Name: "Similar Present Updated"} mockArtistRepo.SetData(model.Artists{*originalArtist, similarInDS}) - // Call the method under test with includeNotPresent = true updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-similar-test", 5, true) - // Assertions Expect(err).NotTo(HaveOccurred()) Expect(updatedArtist).NotTo(BeNil()) - // Assert similar artists loaded (should include present and absent ones) Expect(updatedArtist.SimilarArtists).To(HaveLen(3)) - // Check present artist (updated from DS) Expect(updatedArtist.SimilarArtists[0].ID).To(Equal(similarInDS.ID)) Expect(updatedArtist.SimilarArtists[0].Name).To(Equal(similarInDS.Name)) - // Check already absent artist (ID remains empty) Expect(updatedArtist.SimilarArtists[1].ID).To(BeEmpty()) Expect(updatedArtist.SimilarArtists[1].Name).To(Equal("Similar Absent Raw")) - // Check artist that became absent after DS lookup (ID should be empty) Expect(updatedArtist.SimilarArtists[2].ID).To(BeEmpty()) Expect(updatedArtist.SimilarArtists[2].Name).To(Equal("Similar Absent Lookup")) }) It("returns error when an agent fails during info population", func() { - // Setup: Artist exists, ExternalInfoUpdatedAt is nil originalArtist := &model.Artist{ ID: "ar-agent-fail", Name: "Agent Fail Artist", } mockArtistRepo.SetData(model.Artists{*originalArtist}) - // Mock agent response (GetArtistMBID fails) expectedErr := errors.New("agent MBID failed") ag.On("GetArtistMBID", ctx, "ar-agent-fail", "Agent Fail Artist").Return("", expectedErr).Once() - // Add non-strict expectations for other concurrent agent calls that might happen before error propagation ag.On("GetArtistImages", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything).Return(nil, nil).Maybe() ag.On("GetArtistBiography", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything).Return("", nil).Maybe() ag.On("GetArtistURL", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything).Return("", nil).Maybe() ag.On("GetSimilarArtists", ctx, "ar-agent-fail", "Agent Fail Artist", mock.Anything, 100).Return(nil, nil).Maybe() - // Call the method under test updatedArtist, err := p.UpdateArtistInfo(ctx, "ar-agent-fail", 10, false) - // Assertions - Expect(err).NotTo(HaveOccurred()) // Expect no error based on current implementation - Expect(updatedArtist).NotTo(BeNil()) // The original artist data should still be returned after refresh attempt + Expect(err).NotTo(HaveOccurred()) + Expect(updatedArtist).NotTo(BeNil()) Expect(updatedArtist.ID).To(Equal("ar-agent-fail")) - ag.AssertExpectations(GinkgoT()) // Verifies that GetArtistMBID was called as expected - // Ensure other agent calls were not made after the first failure - Not strictly true due to concurrency - // ag.AssertNotCalled(GinkgoT(), "GetArtistImages") - // ag.AssertNotCalled(GinkgoT(), "GetArtistBiography") - // ag.AssertNotCalled(GinkgoT(), "GetArtistURL") - // ag.AssertNotCalled(GinkgoT(), "GetSimilarArtists") + ag.AssertExpectations(GinkgoT()) }) - - // Test cases will go here - }) diff --git a/tests/mock_album_repo.go b/tests/mock_album_repo.go index ab632b4d4..58c33c97f 100644 --- a/tests/mock_album_repo.go +++ b/tests/mock_album_repo.go @@ -114,8 +114,6 @@ func (m *MockAlbumRepo) UpdateExternalInfo(album *model.Album) error { if m.Err { return errors.New("unexpected error") } - // Simple implementation to prevent nil pointer dereference in tests. - // We could optionally update m.Data here if needed for specific tests. return nil } diff --git a/tests/mock_artist_repo.go b/tests/mock_artist_repo.go index 5674d5b80..7058cead0 100644 --- a/tests/mock_artist_repo.go +++ b/tests/mock_artist_repo.go @@ -76,7 +76,6 @@ func (m *MockArtistRepo) GetAll(options ...model.QueryOptions) (model.Artists, e if m.Err { return nil, errors.New("mock repo error") } - // Convert map back to slice for GetAll simulation var allArtists model.Artists for _, artist := range m.Data { allArtists = append(allArtists, *artist) @@ -92,8 +91,6 @@ func (m *MockArtistRepo) UpdateExternalInfo(artist *model.Artist) error { if m.Err { return errors.New("mock repo error") } - // Simple implementation to prevent nil pointer dereference - // Could update m.Data if needed for specific tests return nil }