diff --git a/core/extdata/provider_albumimage_test.go b/core/extdata/provider_albumimage_test.go index 2a7f9611b..6f0d9d388 100644 --- a/core/extdata/provider_albumimage_test.go +++ b/core/extdata/provider_albumimage_test.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/extdata" "github.com/navidrome/navidrome/model" @@ -24,12 +25,10 @@ var _ = Describe("Provider - AlbumImage", func() { var mockAlbumAgent *mockAlbumInfoAgent var agentsCombined *mockAgents var ctx context.Context - var cancel context.CancelFunc - var originalAgentsConfig string BeforeEach(func() { - ctx, cancel = context.WithCancel(context.Background()) - originalAgentsConfig = conf.Server.Agents + ctx = GinkgoT().Context() + DeferCleanup(configtest.SetupConfig()) conf.Server.Agents = "mockAlbum" // Configure mock agent mockArtistRepo = newMockArtistRepo() @@ -51,9 +50,6 @@ var _ = Describe("Provider - AlbumImage", func() { provider = NewProvider(ds, agentsCombined) // Default mocks - // Removed: mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Maybe() - // Removed: mockMediaFileRepo.On("Get", "mf-1").Return(&model.MediaFile{ID: "mf-1", Title: "Track One", ArtistID: "artist-1", AlbumID: "album-1"}, nil).Maybe() - // Mocks for GetEntityByID sequence (initial failed lookups) mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() @@ -63,240 +59,218 @@ var _ = Describe("Provider - AlbumImage", func() { mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() - - // Default agent response removed - tests will define their own expectations - // mockAlbumAgent.On("GetAlbumInfo", mock.Anything, "Album One", "", ""). - // Return(&agents.AlbumInfo{ - // Images: []agents.ExternalImage{ - // {URL: "http://example.com/large.jpg", Size: 1000}, - // {URL: "http://example.com/medium.jpg", Size: 500}, - // {URL: "http://example.com/small.jpg", Size: 200}, - // }, - // }, nil).Maybe() }) - AfterEach(func() { - cancel() // Restore context cancellation - conf.Server.Agents = originalAgentsConfig // Restore original agent config - // Removed mock assertions - rely on Once() in tests and outcome validation - // mockArtistRepo.AssertExpectations(GinkgoT()) - // mockAlbumRepo.AssertExpectations(GinkgoT()) - // mockMediaFileRepo.AssertExpectations(GinkgoT()) - // mockAgent.AssertExpectations(GinkgoT()) + It("returns the largest image URL when successful", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + {URL: "http://example.com/small.jpg", Size: 200}, + }, + }, nil).Once() + + expectedURL, _ := url.Parse("http://example.com/large.jpg") + imgURL, err := provider.AlbumImage(ctx, "album-1") + + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") // From GetEntityByID + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") // Artist lookup no longer happens in getAlbum + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist name }) - Describe("AlbumImage", func() { - It("returns the largest image URL when successful", func() { - // Arrange - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). - Return(&agents.AlbumInfo{ - Images: []agents.ExternalImage{ - {URL: "http://example.com/large.jpg", Size: 1000}, - {URL: "http://example.com/medium.jpg", Size: 500}, - {URL: "http://example.com/small.jpg", Size: 200}, - }, - }, nil).Once() + It("returns ErrNotFound if the album is not found in the DB", func() { + // Arrange: Explicitly expect the full GetEntityByID sequence for "not-found" + mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - expectedURL, _ := url.Parse("http://example.com/large.jpg") - imgURL, err := provider.AlbumImage(ctx, "album-1") + imgURL, err := provider.AlbumImage(ctx, "not-found") - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") // From GetEntityByID - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") // Artist lookup no longer happens in getAlbum - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist name - }) + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) - It("returns ErrNotFound if the album is not found in the DB", func() { - // Arrange: Explicitly expect the full GetEntityByID sequence for "not-found" - mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + It("returns the agent error if the agent fails", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - imgURL, err := provider.AlbumImage(ctx, "not-found") + agentErr := errors.New("agent failure") + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", "").Return(nil, agentErr).Once() // Expect empty artist - Expect(err).To(MatchError("data not found")) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) - }) + imgURL, err := provider.AlbumImage(ctx, "album-1") - It("returns the agent error if the agent fails", func() { - // Arrange - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + Expect(err).To(MatchError("agent failure")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist + }) - agentErr := errors.New("agent failure") - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", "").Return(nil, agentErr).Once() // Expect empty artist + It("returns ErrNotFound if the agent returns ErrNotFound", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - imgURL, err := provider.AlbumImage(ctx, "album-1") + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", "").Return(nil, agents.ErrNotFound).Once() // Expect empty artist - Expect(err).To(MatchError("agent failure")) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist - }) + imgURL, err := provider.AlbumImage(ctx, "album-1") - It("returns ErrNotFound if the agent returns ErrNotFound", func() { - // Arrange - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist + }) - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", "").Return(nil, agents.ErrNotFound).Once() // Expect empty artist + It("returns ErrNotFound if the agent returns no images", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - imgURL, err := provider.AlbumImage(ctx, "album-1") + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{Images: []agents.ExternalImage{}}, nil).Once() // Expect empty artist - Expect(err).To(MatchError("data not found")) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist - }) + imgURL, err := provider.AlbumImage(ctx, "album-1") - It("returns ErrNotFound if the agent returns no images", func() { - // Arrange - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist + }) - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). - Return(&agents.AlbumInfo{Images: []agents.ExternalImage{}}, nil).Once() // Expect empty artist + It("returns context error if context is canceled", func() { + // Arrange + cctx, cancelCtx := context.WithCancel(ctx) + // Mock the necessary DB calls *before* canceling the context + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Expect the agent call even if context is cancelled, returning the context error + mockAlbumAgent.On("GetAlbumInfo", cctx, "Album One", "", "").Return(nil, context.Canceled).Once() + // Cancel the context *before* calling the function under test + cancelCtx() - imgURL, err := provider.AlbumImage(ctx, "album-1") + imgURL, err := provider.AlbumImage(cctx, "album-1") - Expect(err).To(MatchError("data not found")) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") // Expect empty artist - }) + Expect(err).To(MatchError("context canceled")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + // Agent should now be called, verify this expectation + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", cctx, "Album One", "", "") + }) - It("returns context error if context is canceled", func() { - // Arrange - cctx, cancelCtx := context.WithCancel(context.Background()) - // Mock the necessary DB calls *before* canceling the context - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - // Expect the agent call even if context is cancelled, returning the context error - mockAlbumAgent.On("GetAlbumInfo", cctx, "Album One", "", "").Return(nil, context.Canceled).Once() - // Cancel the context *before* calling the function under test - cancelCtx() + It("derives album ID from MediaFile ID", func() { + // Arrange: Mock full GetEntityByID for "mf-1" and recursive "album-1" + mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "mf-1").Return(&model.MediaFile{ID: "mf-1", Title: "Track One", ArtistID: "artist-1", AlbumID: "album-1"}, nil).Once() + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - imgURL, err := provider.AlbumImage(cctx, "album-1") + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + {URL: "http://example.com/small.jpg", Size: 200}, + }, + }, nil).Once() - Expect(err).To(MatchError("context canceled")) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - // Agent should now be called, verify this expectation - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", cctx, "Album One", "", "") - }) + expectedURL, _ := url.Parse("http://example.com/large.jpg") + imgURL, err := provider.AlbumImage(ctx, "mf-1") - It("derives album ID from MediaFile ID", func() { - // Arrange: Mock full GetEntityByID for "mf-1" and recursive "album-1" - mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() - mockMediaFileRepo.On("Get", "mf-1").Return(&model.MediaFile{ID: "mf-1", Title: "Track One", ArtistID: "artist-1", AlbumID: "album-1"}, nil).Once() - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") + }) - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). - Return(&agents.AlbumInfo{ - Images: []agents.ExternalImage{ - {URL: "http://example.com/large.jpg", Size: 1000}, - {URL: "http://example.com/medium.jpg", Size: 500}, - {URL: "http://example.com/small.jpg", Size: 200}, - }, - }, nil).Once() + It("handles different image orders from agent", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/small.jpg", Size: 200}, + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + }, + }, nil).Once() - expectedURL, _ := url.Parse("http://example.com/large.jpg") - imgURL, err := provider.AlbumImage(ctx, "mf-1") + expectedURL, _ := url.Parse("http://example.com/large.jpg") + imgURL, err := provider.AlbumImage(ctx, "album-1") - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-1") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-1") - mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-1") - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockArtistRepo.AssertNotCalled(GinkgoT(), "Get", "artist-1") - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") - }) + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) // Should still pick the largest + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") + }) - It("handles different image orders from agent", func() { - // Arrange - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). - Return(&agents.AlbumInfo{ - Images: []agents.ExternalImage{ - {URL: "http://example.com/small.jpg", Size: 200}, - {URL: "http://example.com/large.jpg", Size: 1000}, - {URL: "http://example.com/medium.jpg", Size: 500}, - }, - }, nil).Once() + It("handles agent returning only one image", func() { + // Arrange + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence + mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() + // Explicitly mock agent call for this test + mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). + Return(&agents.AlbumInfo{ + Images: []agents.ExternalImage{ + {URL: "http://example.com/single.jpg", Size: 700}, + }, + }, nil).Once() - expectedURL, _ := url.Parse("http://example.com/large.jpg") - imgURL, err := provider.AlbumImage(ctx, "album-1") + expectedURL, _ := url.Parse("http://example.com/single.jpg") + imgURL, err := provider.AlbumImage(ctx, "album-1") - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) // Should still pick the largest - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") - }) + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") + }) - It("handles agent returning only one image", func() { - // Arrange - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() // Expect GetEntityByID sequence - mockAlbumRepo.On("Get", "album-1").Return(&model.Album{ID: "album-1", Name: "Album One", AlbumArtistID: "artist-1"}, nil).Once() - // Explicitly mock agent call for this test - mockAlbumAgent.On("GetAlbumInfo", ctx, "Album One", "", ""). - Return(&agents.AlbumInfo{ - Images: []agents.ExternalImage{ - {URL: "http://example.com/single.jpg", Size: 700}, - }, - }, nil).Once() + It("returns ErrNotFound if deriving album ID fails", func() { + // Arrange: Mock full GetEntityByID for "mf-no-album" and recursive "not-found" + mockArtistRepo.On("Get", "mf-no-album").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-no-album").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "mf-no-album").Return(&model.MediaFile{ID: "mf-no-album", Title: "Track No Album", ArtistID: "artist-1", AlbumID: "not-found"}, nil).Once() + mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - expectedURL, _ := url.Parse("http://example.com/single.jpg") - imgURL, err := provider.AlbumImage(ctx, "album-1") + imgURL, err := provider.AlbumImage(ctx, "mf-no-album") - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockAlbumAgent.AssertCalled(GinkgoT(), "GetAlbumInfo", ctx, "Album One", "", "") - }) - - It("returns ErrNotFound if deriving album ID fails", func() { - // Arrange: Mock full GetEntityByID for "mf-no-album" and recursive "not-found" - mockArtistRepo.On("Get", "mf-no-album").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "mf-no-album").Return(nil, model.ErrNotFound).Once() - mockMediaFileRepo.On("Get", "mf-no-album").Return(&model.MediaFile{ID: "mf-no-album", Title: "Track No Album", ArtistID: "artist-1", AlbumID: "not-found"}, nil).Once() - mockArtistRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Once() - - imgURL, err := provider.AlbumImage(ctx, "mf-no-album") - - Expect(err).To(MatchError("data not found")) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") - mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) - }) + Expect(err).To(MatchError("data not found")) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-no-album") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockAlbumAgent.AssertNotCalled(GinkgoT(), "GetAlbumInfo", mock.Anything, mock.Anything, mock.Anything, mock.Anything) }) }) diff --git a/core/extdata/provider_artistimage_test.go b/core/extdata/provider_artistimage_test.go index 26a63f7fe..5d073f77c 100644 --- a/core/extdata/provider_artistimage_test.go +++ b/core/extdata/provider_artistimage_test.go @@ -6,6 +6,7 @@ import ( "net/url" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/extdata" "github.com/navidrome/navidrome/model" @@ -24,13 +25,11 @@ var _ = Describe("Provider - ArtistImage", func() { var mockImageAgent *mockArtistImageAgent var agentsCombined *mockAgents var ctx context.Context - var cancel context.CancelFunc - var originalAgentsConfig string BeforeEach(func() { - ctx, cancel = context.WithCancel(context.Background()) - originalAgentsConfig = conf.Server.Agents + DeferCleanup(configtest.SetupConfig()) conf.Server.Agents = "mockImage" // Configure only the mock agent + ctx = GinkgoT().Context() mockArtistRepo = newMockArtistRepo() mockAlbumRepo = newMockAlbumRepo() @@ -70,205 +69,201 @@ var _ = Describe("Provider - ArtistImage", func() { }) AfterEach(func() { - cancel() // Ensure context is cancelled - conf.Server.Agents = originalAgentsConfig mockArtistRepo.AssertExpectations(GinkgoT()) mockAlbumRepo.AssertExpectations(GinkgoT()) mockMediaFileRepo.AssertExpectations(GinkgoT()) mockImageAgent.AssertExpectations(GinkgoT()) }) - Describe("ArtistImage", func() { - It("returns the largest image URL when successful", func() { - // Arrange - expectedURL, _ := url.Parse("http://example.com/large.jpg") + It("returns the largest image URL when successful", func() { + // Arrange + expectedURL, _ := url.Parse("http://example.com/large.jpg") - // Act - imgURL, err := provider.ArtistImage(ctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") - // Assert - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("returns ErrNotFound if the artist is not found in the DB", func() { - // Arrange + It("returns ErrNotFound if the artist is not found in the DB", func() { + // Arrange - // Act - imgURL, err := provider.ArtistImage(ctx, "not-found") + // Act + imgURL, err := provider.ArtistImage(ctx, "not-found") - // Assert - Expect(err).To(MatchError(model.ErrNotFound)) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything) - }) + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) - It("returns the agent error if the agent fails", func() { - // Arrange - agentErr := errors.New("agent failure") - mockImageAgent.Mock = mock.Mock{} // Reset default expectation - mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return(nil, agentErr).Once() + It("returns the agent error if the agent fails", func() { + // Arrange + agentErr := errors.New("agent failure") + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return(nil, agentErr).Once() - // Act - imgURL, err := provider.ArtistImage(ctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") - // Assert - Expect(err).To(MatchError(model.ErrNotFound)) // Corrected Expectation: The provider maps agent errors (other than canceled) to ErrNotFound if no image was found/populated - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) // Corrected Expectation: The provider maps agent errors (other than canceled) to ErrNotFound if no image was found/populated + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("returns ErrNotFound if the agent returns ErrNotFound", func() { - // Arrange - mockImageAgent.Mock = mock.Mock{} // Reset default expectation - mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return(nil, agents.ErrNotFound).Once() + It("returns ErrNotFound if the agent returns ErrNotFound", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return(nil, agents.ErrNotFound).Once() - // Act - imgURL, err := provider.ArtistImage(ctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") - // Assert - Expect(err).To(MatchError(model.ErrNotFound)) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("returns ErrNotFound if the agent returns no images", func() { - // Arrange - mockImageAgent.Mock = mock.Mock{} // Reset default expectation - mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return([]agents.ExternalImage{}, nil).Once() + It("returns ErrNotFound if the agent returns no images", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", "").Return([]agents.ExternalImage{}, nil).Once() - // Act - imgURL, err := provider.ArtistImage(ctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") - // Assert - Expect(err).To(MatchError(model.ErrNotFound)) // Implementation maps empty result to ErrNotFound - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) // Implementation maps empty result to ErrNotFound + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("returns context error if context is canceled before agent call", func() { - // Arrange - cctx, cancelCtx := context.WithCancel(context.Background()) - mockArtistRepo.Mock = mock.Mock{} // Reset default expectation for artist repo as well - mockArtistRepo.On("Get", "artist-1").Return(&model.Artist{ID: "artist-1", Name: "Artist One"}, nil).Run(func(args mock.Arguments) { - cancelCtx() // Cancel context *during* the DB call simulation - }).Once() + It("returns context error if context is canceled before agent call", func() { + // Arrange + cctx, cancelCtx := context.WithCancel(context.Background()) + mockArtistRepo.Mock = mock.Mock{} // Reset default expectation for artist repo as well + mockArtistRepo.On("Get", "artist-1").Return(&model.Artist{ID: "artist-1", Name: "Artist One"}, nil).Run(func(args mock.Arguments) { + cancelCtx() // Cancel context *during* the DB call simulation + }).Once() - // Act - imgURL, err := provider.ArtistImage(cctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(cctx, "artist-1") - // Assert - Expect(err).To(MatchError(context.Canceled)) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - }) + // Assert + Expect(err).To(MatchError(context.Canceled)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + }) - It("derives artist ID from MediaFile ID", func() { - // Arrange: Add mocks for the initial GetEntityByID lookups - mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() - // Default mocks for MediaFileRepo.Get("mf-1") and ArtistRepo.Get("artist-1") handle the rest - expectedURL, _ := url.Parse("http://example.com/large.jpg") + It("derives artist ID from MediaFile ID", func() { + // Arrange: Add mocks for the initial GetEntityByID lookups + mockArtistRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-1").Return(nil, model.ErrNotFound).Once() + // Default mocks for MediaFileRepo.Get("mf-1") and ArtistRepo.Get("artist-1") handle the rest + expectedURL, _ := url.Parse("http://example.com/large.jpg") - // Act - imgURL, err := provider.ArtistImage(ctx, "mf-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "mf-1") - // Assert - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-1") // GetEntityByID sequence - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-1") // GetEntityByID sequence - mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-1") - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") // Should be called after getting MF - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-1") // GetEntityByID sequence + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-1") // GetEntityByID sequence + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-1") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") // Should be called after getting MF + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("derives artist ID from Album ID", func() { - // Arrange: Add mock for the initial GetEntityByID lookup - mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() - // Default mocks for AlbumRepo.Get("album-1") and ArtistRepo.Get("artist-1") handle the rest - expectedURL, _ := url.Parse("http://example.com/large.jpg") + It("derives artist ID from Album ID", func() { + // Arrange: Add mock for the initial GetEntityByID lookup + mockArtistRepo.On("Get", "album-1").Return(nil, model.ErrNotFound).Once() + // Default mocks for AlbumRepo.Get("album-1") and ArtistRepo.Get("artist-1") handle the rest + expectedURL, _ := url.Parse("http://example.com/large.jpg") - // Act - imgURL, err := provider.ArtistImage(ctx, "album-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "album-1") - // Assert - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") // GetEntityByID sequence - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") // Should be called after getting Album - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "album-1") // GetEntityByID sequence + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "album-1") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") // Should be called after getting Album + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("returns ErrNotFound if derived artist is not found", func() { - // Arrange - // Add mocks for the initial GetEntityByID lookups - mockArtistRepo.On("Get", "mf-bad-artist").Return(nil, model.ErrNotFound).Once() - mockAlbumRepo.On("Get", "mf-bad-artist").Return(nil, model.ErrNotFound).Once() - mockMediaFileRepo.On("Get", "mf-bad-artist").Return(&model.MediaFile{ID: "mf-bad-artist", ArtistID: "not-found"}, nil).Once() - // Add expectation for the recursive GetEntityByID call for the MediaFileRepo - mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() - // The default mocks for ArtistRepo/AlbumRepo handle the final "not-found" lookups + It("returns ErrNotFound if derived artist is not found", func() { + // Arrange + // Add mocks for the initial GetEntityByID lookups + mockArtistRepo.On("Get", "mf-bad-artist").Return(nil, model.ErrNotFound).Once() + mockAlbumRepo.On("Get", "mf-bad-artist").Return(nil, model.ErrNotFound).Once() + mockMediaFileRepo.On("Get", "mf-bad-artist").Return(&model.MediaFile{ID: "mf-bad-artist", ArtistID: "not-found"}, nil).Once() + // Add expectation for the recursive GetEntityByID call for the MediaFileRepo + mockMediaFileRepo.On("Get", "not-found").Return(nil, model.ErrNotFound).Maybe() + // The default mocks for ArtistRepo/AlbumRepo handle the final "not-found" lookups - // Act - imgURL, err := provider.ArtistImage(ctx, "mf-bad-artist") + // Act + imgURL, err := provider.ArtistImage(ctx, "mf-bad-artist") - // Assert - Expect(err).To(MatchError(model.ErrNotFound)) - Expect(imgURL).To(BeNil()) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") // GetEntityByID sequence - mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") // GetEntityByID sequence - mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") - mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything) - }) + // Assert + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(imgURL).To(BeNil()) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") // GetEntityByID sequence + mockAlbumRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") // GetEntityByID sequence + mockMediaFileRepo.AssertCalled(GinkgoT(), "Get", "mf-bad-artist") + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "not-found") + mockImageAgent.AssertNotCalled(GinkgoT(), "GetArtistImages", mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) - It("handles different image orders from agent", func() { - // Arrange - mockImageAgent.Mock = mock.Mock{} // Reset default expectation - mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", ""). - Return([]agents.ExternalImage{ - {URL: "http://example.com/small.jpg", Size: 200}, - {URL: "http://example.com/large.jpg", Size: 1000}, - {URL: "http://example.com/medium.jpg", Size: 500}, - }, nil).Once() - expectedURL, _ := url.Parse("http://example.com/large.jpg") + It("handles different image orders from agent", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", ""). + Return([]agents.ExternalImage{ + {URL: "http://example.com/small.jpg", Size: 200}, + {URL: "http://example.com/large.jpg", Size: 1000}, + {URL: "http://example.com/medium.jpg", Size: 500}, + }, nil).Once() + expectedURL, _ := url.Parse("http://example.com/large.jpg") - // Act - imgURL, err := provider.ArtistImage(ctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") - // Assert - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) // Still picks the largest - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) // Still picks the largest + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") + }) - It("handles agent returning only one image", func() { - // Arrange - mockImageAgent.Mock = mock.Mock{} // Reset default expectation - mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", ""). - Return([]agents.ExternalImage{ - {URL: "http://example.com/medium.jpg", Size: 500}, - }, nil).Once() - expectedURL, _ := url.Parse("http://example.com/medium.jpg") + It("handles agent returning only one image", func() { + // Arrange + mockImageAgent.Mock = mock.Mock{} // Reset default expectation + mockImageAgent.On("GetArtistImages", ctx, "artist-1", "Artist One", ""). + Return([]agents.ExternalImage{ + {URL: "http://example.com/medium.jpg", Size: 500}, + }, nil).Once() + expectedURL, _ := url.Parse("http://example.com/medium.jpg") - // Act - imgURL, err := provider.ArtistImage(ctx, "artist-1") + // Act + imgURL, err := provider.ArtistImage(ctx, "artist-1") - // Assert - Expect(err).ToNot(HaveOccurred()) - Expect(imgURL).To(Equal(expectedURL)) - mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") - mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") - }) + // Assert + Expect(err).ToNot(HaveOccurred()) + Expect(imgURL).To(Equal(expectedURL)) + mockArtistRepo.AssertCalled(GinkgoT(), "Get", "artist-1") + mockImageAgent.AssertCalled(GinkgoT(), "GetArtistImages", ctx, "artist-1", "Artist One", "") }) }) diff --git a/core/extdata/provider_similarsongs_test.go b/core/extdata/provider_similarsongs_test.go index 21a958d13..9914a4130 100644 --- a/core/extdata/provider_similarsongs_test.go +++ b/core/extdata/provider_similarsongs_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/core/agents" . "github.com/navidrome/navidrome/core/extdata" "github.com/navidrome/navidrome/model" @@ -24,11 +23,9 @@ var _ = Describe("Provider - SimilarSongs", func() { var artistRepo *mockArtistRepo var mediaFileRepo *mockMediaFileRepo var ctx context.Context - var originalAgentsConfig string BeforeEach(func() { - ctx = context.Background() - originalAgentsConfig = conf.Server.Agents + ctx = GinkgoT().Context() artistRepo = newMockArtistRepo() mediaFileRepo = newMockMediaFileRepo() @@ -50,158 +47,152 @@ var _ = Describe("Provider - SimilarSongs", func() { provider = NewProvider(ds, agentsCombined) }) - AfterEach(func() { - conf.Server.Agents = originalAgentsConfig + It("returns similar songs from main artist and similar artists", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} + song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3"} + + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Once() + + similarAgentsResp := []agents.Artist{ + {Name: "Similar Artist", MBID: "similar-mbid"}, + } + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return(similarAgentsResp, nil).Once() + + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{similarArtist}, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + {Name: "Song Two", MBID: "mbid-2"}, + }, nil).Once() + + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song Three", MBID: "mbid-3"}, + }, nil).Once() + + mediaFileRepo.FindByMBID("mbid-1", song1) + mediaFileRepo.FindByMBID("mbid-2", song2) + mediaFileRepo.FindByMBID("mbid-3", song3) + + songs, err := provider.SimilarSongs(ctx, "artist-1", 3) + + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(3)) + for _, song := range songs { + Expect(song.ID).To(BeElementOf("song-1", "song-2", "song-3")) + } }) - Describe("SimilarSongs", func() { - It("returns similar songs from main artist and similar artists", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - similarArtist := model.Artist{ID: "artist-3", Name: "Similar Artist"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} - song3 := model.MediaFile{ID: "song-3", Title: "Song Three", ArtistID: "artist-3"} + It("returns ErrNotFound when artist is not found", func() { + artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("Get", "artist-3").Return(&similarArtist, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{}, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Once() + songs, err := provider.SimilarSongs(ctx, "artist-unknown-artist", 5) - similarAgentsResp := []agents.Artist{ - {Name: "Similar Artist", MBID: "similar-mbid"}, - } - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return(similarAgentsResp, nil).Once() + Expect(err).To(Equal(model.ErrNotFound)) + Expect(songs).To(BeNil()) + }) - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{similarArtist}, nil).Once() + It("returns songs from main artist when GetSimilarArtists returns error", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song One", MBID: "mbid-1"}, - {Name: "Song Two", MBID: "mbid-2"}, - }, nil).Once() + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-3", "Similar Artist", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song Three", MBID: "mbid-3"}, - }, nil).Once() + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return(nil, errors.New("error getting similar artists")).Once() - mediaFileRepo.FindByMBID("mbid-1", song1) - mediaFileRepo.FindByMBID("mbid-2", song2) - mediaFileRepo.FindByMBID("mbid-3", song3) + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() - songs, err := provider.SimilarSongs(ctx, "artist-1", 3) + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + }, nil).Once() - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(3)) - for _, song := range songs { - Expect(song.ID).To(BeElementOf("song-1", "song-2", "song-3")) - } - }) + mediaFileRepo.FindByMBID("mbid-1", song1) - It("returns ErrNotFound when artist is not found", func() { - artistRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) - mediaFileRepo.On("Get", "artist-unknown-artist").Return(nil, model.ErrNotFound) + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{}, nil).Maybe() + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + }) - songs, err := provider.SimilarSongs(ctx, "artist-unknown-artist", 5) + It("returns empty list when GetArtistTopSongs returns error", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - Expect(err).To(Equal(model.ErrNotFound)) - Expect(songs).To(BeNil()) - }) + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() - It("returns songs from main artist when GetSimilarArtists returns error", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{}, nil).Once() - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return(nil, errors.New("error getting similar artists")).Once() + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return(nil, errors.New("error getting top songs")).Once() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{}, nil).Once() + songs, err := provider.SimilarSongs(ctx, "artist-1", 5) - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song One", MBID: "mbid-1"}, - }, nil).Once() + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(BeEmpty()) + }) - mediaFileRepo.FindByMBID("mbid-1", song1) + It("respects count parameter", func() { + artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} - songs, err := provider.SimilarSongs(ctx, "artist-1", 5) + artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 1 && opt.Filters != nil + })).Return(model.Artists{artist1}, nil).Maybe() - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("song-1")) - }) + mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). + Return([]agents.Artist{}, nil).Once() - It("returns empty list when GetArtistTopSongs returns error", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} + artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { + return opt.Max == 0 && opt.Filters != nil + })).Return(model.Artists{}, nil).Once() - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Maybe() + mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). + Return([]agents.Song{ + {Name: "Song One", MBID: "mbid-1"}, + {Name: "Song Two", MBID: "mbid-2"}, + }, nil).Once() - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return([]agents.Artist{}, nil).Once() + mediaFileRepo.FindByMBID("mbid-1", song1) + mediaFileRepo.FindByMBID("mbid-2", song2) - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{}, nil).Once() + songs, err := provider.SimilarSongs(ctx, "artist-1", 1) - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return(nil, errors.New("error getting top songs")).Once() - - songs, err := provider.SimilarSongs(ctx, "artist-1", 5) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(BeEmpty()) - }) - - It("respects count parameter", func() { - artist1 := model.Artist{ID: "artist-1", Name: "Artist One"} - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1"} - - artistRepo.On("Get", "artist-1").Return(&artist1, nil).Maybe() - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 1 && opt.Filters != nil - })).Return(model.Artists{artist1}, nil).Maybe() - - mockAgent.On("GetSimilarArtists", mock.Anything, "artist-1", "Artist One", "", 15). - Return([]agents.Artist{}, nil).Once() - - artistRepo.On("GetAll", mock.MatchedBy(func(opt model.QueryOptions) bool { - return opt.Max == 0 && opt.Filters != nil - })).Return(model.Artists{}, nil).Once() - - mockAgent.On("GetArtistTopSongs", mock.Anything, "artist-1", "Artist One", "", mock.Anything). - Return([]agents.Song{ - {Name: "Song One", MBID: "mbid-1"}, - {Name: "Song Two", MBID: "mbid-2"}, - }, nil).Once() - - mediaFileRepo.FindByMBID("mbid-1", song1) - mediaFileRepo.FindByMBID("mbid-2", song2) - - songs, err := provider.SimilarSongs(ctx, "artist-1", 1) - - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(BeElementOf("song-1", "song-2")) - }) + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(BeElementOf("song-1", "song-2")) }) }) diff --git a/core/extdata/provider_topsongs_test.go b/core/extdata/provider_topsongs_test.go index 9324d314e..c526ed68e 100644 --- a/core/extdata/provider_topsongs_test.go +++ b/core/extdata/provider_topsongs_test.go @@ -4,7 +4,6 @@ import ( "context" "errors" - "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" @@ -19,17 +18,15 @@ import ( var _ = Describe("Provider - TopSongs", func() { var ( - p Provider - artistRepo *mockArtistRepo // From provider_helper_test.go - mediaFileRepo *mockMediaFileRepo // From provider_helper_test.go - ag *mockAgents // Consolidated mock from export_test.go - ctx context.Context - originalAgentsConfig string + p Provider + artistRepo *mockArtistRepo // From provider_helper_test.go + mediaFileRepo *mockMediaFileRepo // From provider_helper_test.go + ag *mockAgents // Consolidated mock from export_test.go + ctx context.Context ) BeforeEach(func() { - ctx = context.Background() - originalAgentsConfig = conf.Server.Agents + ctx = GinkgoT().Context() artistRepo = newMockArtistRepo() // Use helper mock mediaFileRepo = newMockMediaFileRepo() // Use helper mock @@ -45,158 +42,152 @@ var _ = Describe("Provider - TopSongs", func() { p = NewProvider(ds, ag) }) - AfterEach(func() { - conf.Server.Agents = originalAgentsConfig + BeforeEach(func() { + // Setup expectations in individual tests }) - Describe("TopSongs", func() { - BeforeEach(func() { - // Setup expectations in individual tests - }) + It("returns top songs for a known artist", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() - It("returns top songs for a known artist", func() { - // Mock finding the artist - artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + // Mock agent response + agentSongs := []agents.Song{ + {Name: "Song One", MBID: "mbid-song-1"}, + {Name: "Song Two", MBID: "mbid-song-2"}, + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() - // Mock agent response - agentSongs := []agents.Song{ - {Name: "Song One", MBID: "mbid-song-1"}, - {Name: "Song Two", MBID: "mbid-song-2"}, - } - ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() + // Mock finding matching tracks + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} + song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once() - // Mock finding matching tracks - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} - song2 := model.MediaFile{ID: "song-2", Title: "Song Two", ArtistID: "artist-1", MbzRecordingID: "mbid-song-2"} - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song2}, nil).Once() + songs, err := p.TopSongs(ctx, "Artist One", 2) - songs, err := p.TopSongs(ctx, "Artist One", 2) + 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")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) - 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")) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertExpectations(GinkgoT()) - mediaFileRepo.AssertExpectations(GinkgoT()) - }) + It("returns nil for an unknown artist", func() { + // Mock artist not found + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{}, nil).Once() - It("returns nil for an unknown artist", func() { - // Mock artist not found - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{}, nil).Once() + songs, err := p.TopSongs(ctx, "Unknown Artist", 5) - songs, err := p.TopSongs(ctx, "Unknown Artist", 5) + Expect(err).ToNot(HaveOccurred()) // TopSongs returns nil error if artist not found + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertNotCalled(GinkgoT(), "GetArtistTopSongs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) + }) - Expect(err).ToNot(HaveOccurred()) // TopSongs returns nil error if artist not found - Expect(songs).To(BeNil()) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertNotCalled(GinkgoT(), "GetArtistTopSongs", mock.Anything, mock.Anything, mock.Anything, mock.Anything, mock.Anything) - }) + It("returns error when the agent returns an error", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() - It("returns error when the agent returns an error", func() { - // Mock finding the artist - artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + // Mock agent error + agentErr := errors.New("agent error") + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agentErr).Once() - // Mock agent error - agentErr := errors.New("agent error") - ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agentErr).Once() + songs, err := p.TopSongs(ctx, "Artist One", 5) - songs, err := p.TopSongs(ctx, "Artist One", 5) + Expect(err).To(MatchError(agentErr)) + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + }) - Expect(err).To(MatchError(agentErr)) - Expect(songs).To(BeNil()) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertExpectations(GinkgoT()) - }) + It("returns ErrNotFound when the agent returns ErrNotFound", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() - It("returns ErrNotFound when the agent returns ErrNotFound", func() { - // Mock finding the artist - artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + // Mock agent ErrNotFound + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agents.ErrNotFound).Once() - // Mock agent ErrNotFound - ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, agents.ErrNotFound).Once() + songs, err := p.TopSongs(ctx, "Artist One", 5) - songs, err := p.TopSongs(ctx, "Artist One", 5) + Expect(err).To(MatchError(model.ErrNotFound)) + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + }) - Expect(err).To(MatchError(model.ErrNotFound)) - Expect(songs).To(BeNil()) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertExpectations(GinkgoT()) - }) + It("returns fewer songs if count is less than available top songs", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() - It("returns fewer songs if count is less than available top songs", func() { - // Mock finding the artist - artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + // Mock agent response (only need 1 for the test) + agentSongs := []agents.Song{{Name: "Song One", MBID: "mbid-song-1"}} + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once() - // Mock agent response (only need 1 for the test) - agentSongs := []agents.Song{{Name: "Song One", MBID: "mbid-song-1"}} - ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 1).Return(agentSongs, nil).Once() + // Mock finding matching track + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - // Mock finding matching track - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + songs, err := p.TopSongs(ctx, "Artist One", 1) - songs, err := p.TopSongs(ctx, "Artist One", 1) + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("song-1")) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertExpectations(GinkgoT()) - mediaFileRepo.AssertExpectations(GinkgoT()) - }) + It("returns fewer songs if fewer matching tracks are found", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() - It("returns fewer songs if fewer matching tracks are found", func() { - // Mock finding the artist - artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + // Mock agent response + agentSongs := []agents.Song{ + {Name: "Song One", MBID: "mbid-song-1"}, + {Name: "Song Two", MBID: "mbid-song-2"}, + } + ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() - // Mock agent response - agentSongs := []agents.Song{ - {Name: "Song One", MBID: "mbid-song-1"}, - {Name: "Song Two", MBID: "mbid-song-2"}, - } - ag.On("GetArtistTopSongs", ctx, "artist-1", "Artist One", "mbid-artist-1", 2).Return(agentSongs, nil).Once() + // Mock finding matching tracks (only find song 1) + song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For mbid-song-2 (fails) + mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For title fallback (fails) - // Mock finding matching tracks (only find song 1) - song1 := model.MediaFile{ID: "song-1", Title: "Song One", ArtistID: "artist-1", MbzRecordingID: "mbid-song-1"} - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{song1}, nil).Once() - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For mbid-song-2 (fails) - mediaFileRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.MediaFiles{}, nil).Once() // For title fallback (fails) + songs, err := p.TopSongs(ctx, "Artist One", 2) - songs, err := p.TopSongs(ctx, "Artist One", 2) + Expect(err).ToNot(HaveOccurred()) + Expect(songs).To(HaveLen(1)) + Expect(songs[0].ID).To(Equal("song-1")) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) + mediaFileRepo.AssertExpectations(GinkgoT()) + }) - Expect(err).ToNot(HaveOccurred()) - Expect(songs).To(HaveLen(1)) - Expect(songs[0].ID).To(Equal("song-1")) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertExpectations(GinkgoT()) - mediaFileRepo.AssertExpectations(GinkgoT()) - }) + It("returns error when context is canceled during agent call", func() { + // Mock finding the artist + artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} + artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() - It("returns error when context is canceled during agent call", func() { - // Mock finding the artist - artist1 := model.Artist{ID: "artist-1", Name: "Artist One", MbzArtistID: "mbid-artist-1"} - artistRepo.On("GetAll", mock.AnythingOfType("model.QueryOptions")).Return(model.Artists{artist1}, nil).Once() + // Setup context that will be canceled + canceledCtx, cancel := context.WithCancel(ctx) - // Setup context that will be canceled - canceledCtx, cancel := context.WithCancel(ctx) + // Mock agent call to return context canceled error + ag.On("GetArtistTopSongs", canceledCtx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, context.Canceled).Once() - // Mock agent call to return context canceled error - ag.On("GetArtistTopSongs", canceledCtx, "artist-1", "Artist One", "mbid-artist-1", 5).Return(nil, context.Canceled).Once() + cancel() // Cancel the context before calling + songs, err := p.TopSongs(canceledCtx, "Artist One", 5) - cancel() // Cancel the context before calling - songs, err := p.TopSongs(canceledCtx, "Artist One", 5) - - Expect(err).To(MatchError(context.Canceled)) - Expect(songs).To(BeNil()) - artistRepo.AssertExpectations(GinkgoT()) - ag.AssertExpectations(GinkgoT()) - }) + Expect(err).To(MatchError(context.Canceled)) + Expect(songs).To(BeNil()) + artistRepo.AssertExpectations(GinkgoT()) + ag.AssertExpectations(GinkgoT()) }) }) diff --git a/core/extdata/provider_updatealbuminfo_test.go b/core/extdata/provider_updatealbuminfo_test.go index 125b49af2..c9b7cfc9e 100644 --- a/core/extdata/provider_updatealbuminfo_test.go +++ b/core/extdata/provider_updatealbuminfo_test.go @@ -31,7 +31,7 @@ var _ = Describe("Provider - UpdateAlbumInfo", func() { ) BeforeEach(func() { - ctx = context.Background() + ctx = GinkgoT().Context() ds = new(tests.MockDataStore) ag = new(mockAgents) p = extdata.NewProvider(ds, ag) diff --git a/core/extdata/provider_updateartistinfo_test.go b/core/extdata/provider_updateartistinfo_test.go index e9a86aa53..629adf597 100644 --- a/core/extdata/provider_updateartistinfo_test.go +++ b/core/extdata/provider_updateartistinfo_test.go @@ -6,6 +6,7 @@ import ( "time" "github.com/navidrome/navidrome/conf" + "github.com/navidrome/navidrome/conf/configtest" "github.com/navidrome/navidrome/core/agents" "github.com/navidrome/navidrome/core/extdata" "github.com/navidrome/navidrome/log" @@ -31,12 +32,13 @@ var _ = Describe("Provider - UpdateArtistInfo", func() { ) BeforeEach(func() { - ctx = context.Background() + DeferCleanup(configtest.SetupConfig()) + conf.Server.DevArtistInfoTimeToLive = 1 * time.Hour + ctx = GinkgoT().Context() ds = new(tests.MockDataStore) ag = new(mockAgents) p = extdata.NewProvider(ds, ag) mockArtistRepo = ds.Artist(ctx).(*tests.MockArtistRepo) - conf.Server.DevArtistInfoTimeToLive = 1 * time.Hour }) It("returns error when artist is not found", func() {