From 98fdc42d097300f9c32a10437851c5516e564bed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Deluan=20Quint=C3=A3o?= Date: Thu, 22 May 2025 15:48:24 -0400 Subject: [PATCH] test: fix ignored artwork tests (#4103) * Fix artwork internal tests * fix: rename artistReader functions to artistArtworkReader for clarity Signed-off-by: Deluan * fix: update artwork internal tests to handle corrupted cover scenarios Signed-off-by: Deluan --------- Signed-off-by: Deluan --- core/artwork/artwork.go | 2 +- core/artwork/artwork_internal_test.go | 119 +++++++++++++++++--------- core/artwork/reader_artist.go | 2 +- core/artwork/reader_artist_test.go | 2 +- 4 files changed, 80 insertions(+), 45 deletions(-) diff --git a/core/artwork/artwork.go b/core/artwork/artwork.go index f1f571e0..2e92b24c 100644 --- a/core/artwork/artwork.go +++ b/core/artwork/artwork.go @@ -115,7 +115,7 @@ func (a *artwork) getArtworkReader(ctx context.Context, artID model.ArtworkID, s } else { switch artID.Kind { case model.KindArtistArtwork: - artReader, err = newArtistReader(ctx, a, artID, a.provider) + artReader, err = newArtistArtworkReader(ctx, a, artID, a.provider) case model.KindAlbumArtwork: artReader, err = newAlbumArtworkReader(ctx, a, artID, a.provider) case model.KindMediaFileArtwork: diff --git a/core/artwork/artwork_internal_test.go b/core/artwork/artwork_internal_test.go index 46202708..cfb7850b 100644 --- a/core/artwork/artwork_internal_test.go +++ b/core/artwork/artwork_internal_test.go @@ -4,7 +4,11 @@ import ( "context" "errors" "image" + "image/jpeg" + "image/png" "io" + "os" + "path/filepath" "github.com/navidrome/navidrome/conf" "github.com/navidrome/navidrome/conf/configtest" @@ -15,11 +19,11 @@ import ( . "github.com/onsi/gomega" ) -// TODO Fix tests -var _ = XDescribe("Artwork", func() { +var _ = Describe("Artwork", func() { var aw *artwork var ds model.DataStore var ffmpeg *tests.MockFFmpeg + var folderRepo *fakeFolderRepo ctx := log.NewContext(context.TODO()) var alOnlyEmbed, alEmbedNotFound, alOnlyExternal, alExternalNotFound, alMultipleCovers model.Album var arMultipleCovers model.Artist @@ -30,20 +34,21 @@ var _ = XDescribe("Artwork", func() { conf.Server.ImageCacheSize = "0" // Disable cache conf.Server.CoverArtPriority = "folder.*, cover.*, embedded , front.*" - ds = &tests.MockDataStore{MockedTranscoding: &tests.MockTranscodingRepo{}} - alOnlyEmbed = model.Album{ID: "222", Name: "Only embed", EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3"} - alEmbedNotFound = model.Album{ID: "333", Name: "Embed not found", EmbedArtPath: "tests/fixtures/NON_EXISTENT.mp3"} - //alOnlyExternal = model.Album{ID: "444", Name: "Only external", ImageFiles: "tests/fixtures/artist/an-album/front.png"} - //alExternalNotFound = model.Album{ID: "555", Name: "External not found", ImageFiles: "tests/fixtures/NON_EXISTENT.png"} + folderRepo = &fakeFolderRepo{} + ds = &tests.MockDataStore{ + MockedTranscoding: &tests.MockTranscodingRepo{}, + MockedFolder: folderRepo, + } + alOnlyEmbed = model.Album{ID: "222", Name: "Only embed", EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", FolderIDs: []string{"f1"}} + alEmbedNotFound = model.Album{ID: "333", Name: "Embed not found", EmbedArtPath: "tests/fixtures/NON_EXISTENT.mp3", FolderIDs: []string{"f1"}} + alOnlyExternal = model.Album{ID: "444", Name: "Only external", FolderIDs: []string{"f1"}} + alExternalNotFound = model.Album{ID: "555", Name: "External not found", FolderIDs: []string{"f2"}} arMultipleCovers = model.Artist{ID: "777", Name: "All options"} alMultipleCovers = model.Album{ - ID: "666", - Name: "All options", - EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", - //Paths: []string{"tests/fixtures/artist/an-album"}, - //ImageFiles: "tests/fixtures/artist/an-album/cover.jpg" + consts.Zwsp + - // "tests/fixtures/artist/an-album/front.png" + consts.Zwsp + - // "tests/fixtures/artist/an-album/artist.png", + ID: "666", + Name: "All options", + EmbedArtPath: "tests/fixtures/artist/an-album/test.mp3", + FolderIDs: []string{"f1"}, AlbumArtistID: "777", } mfWithEmbed = model.MediaFile{ID: "22", Path: "tests/fixtures/test.mp3", HasCoverArt: true, AlbumID: "222"} @@ -65,6 +70,7 @@ var _ = XDescribe("Artwork", func() { }) Context("Embed images", func() { BeforeEach(func() { + folderRepo.result = nil ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alOnlyEmbed, alEmbedNotFound, @@ -87,12 +93,17 @@ var _ = XDescribe("Artwork", func() { }) Context("External images", func() { BeforeEach(func() { + folderRepo.result = []model.Folder{} ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alOnlyExternal, alExternalNotFound, }) }) It("returns external cover", func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"front.png"}, + }} aw, err := newAlbumArtworkReader(ctx, aw, alOnlyExternal.CoverArtID(), nil) Expect(err).ToNot(HaveOccurred()) _, path, err := aw.Reader(ctx) @@ -100,6 +111,7 @@ var _ = XDescribe("Artwork", func() { Expect(path).To(Equal("tests/fixtures/artist/an-album/front.png")) }) It("returns ErrUnavailable if external file is not available", func() { + folderRepo.result = []model.Folder{} aw, err := newAlbumArtworkReader(ctx, aw, alExternalNotFound.CoverArtID(), nil) Expect(err).ToNot(HaveOccurred()) _, _, err = aw.Reader(ctx) @@ -108,6 +120,10 @@ var _ = XDescribe("Artwork", func() { }) Context("Multiple covers", func() { BeforeEach(func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"cover.jpg", "front.png", "artist.png"}, + }} ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alMultipleCovers, }) @@ -130,6 +146,10 @@ var _ = XDescribe("Artwork", func() { Describe("artistArtworkReader", func() { Context("Multiple covers", func() { BeforeEach(func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"artist.png"}, + }} ds.Artist(ctx).(*tests.MockArtistRepo).SetData(model.Artists{ arMultipleCovers, }) @@ -143,7 +163,7 @@ var _ = XDescribe("Artwork", func() { DescribeTable("ArtistArtPriority", func(priority string, expected string) { conf.Server.ArtistArtPriority = priority - aw, err := newArtistReader(ctx, aw, arMultipleCovers.CoverArtID(), nil) + aw, err := newArtistArtworkReader(ctx, aw, arMultipleCovers.CoverArtID(), nil) Expect(err).ToNot(HaveOccurred()) _, path, err := aw.Reader(ctx) Expect(err).ToNot(HaveOccurred()) @@ -157,12 +177,16 @@ var _ = XDescribe("Artwork", func() { Describe("mediafileArtworkReader", func() { Context("ID not found", func() { It("returns ErrNotFound if mediafile is not in the DB", func() { - _, err := newAlbumArtworkReader(ctx, aw, alMultipleCovers.CoverArtID(), nil) + _, err := newMediafileArtworkReader(ctx, aw, model.MustParseArtworkID("mf-NOT-FOUND")) Expect(err).To(MatchError(model.ErrNotFound)) }) }) Context("Embed images", func() { BeforeEach(func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"front.png"}, + }} ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alOnlyEmbed, alOnlyExternal, @@ -185,11 +209,17 @@ var _ = XDescribe("Artwork", func() { Expect(err).ToNot(HaveOccurred()) r, path, err := aw.Reader(ctx) Expect(err).ToNot(HaveOccurred()) - Expect(io.ReadAll(r)).To(Equal([]byte("content from ffmpeg"))) + data, _ := io.ReadAll(r) + Expect(data).ToNot(BeEmpty()) Expect(path).To(Equal("tests/fixtures/test.ogg")) }) It("returns album cover if cannot read embed artwork", func() { + // Force fromTag to fail + mfCorruptedCover.Path = "tests/fixtures/DOES_NOT_EXIST.ogg" + Expect(ds.MediaFile(ctx).(*tests.MockMediaFileRepo).Put(&mfCorruptedCover)).To(Succeed()) + // Simulate ffmpeg error ffmpeg.Error = errors.New("not available") + aw, err := newMediafileArtworkReader(ctx, aw, mfCorruptedCover.CoverArtID()) Expect(err).ToNot(HaveOccurred()) _, path, err := aw.Reader(ctx) @@ -207,6 +237,10 @@ var _ = XDescribe("Artwork", func() { }) Describe("resizedArtworkReader", func() { BeforeEach(func() { + folderRepo.result = []model.Folder{{ + Path: "tests/fixtures/artist/an-album", + ImageFiles: []string{"cover.jpg", "front.png"}, + }} ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alMultipleCovers, }) @@ -241,12 +275,13 @@ var _ = XDescribe("Artwork", func() { DescribeTable("resize", func(format string, landscape bool, size int) { coverFileName := "cover." + format - //dirName := createImage(format, landscape, size) + dirName := createImage(format, landscape, size) alCover = model.Album{ - ID: "444", - Name: "Only external", - //ImageFiles: filepath.Join(dirName, coverFileName), + ID: "444", + Name: "Only external", + FolderIDs: []string{"tmp"}, } + folderRepo.result = []model.Folder{{Path: dirName, ImageFiles: []string{coverFileName}}} ds.Album(ctx).(*tests.MockAlbumRepo).SetData(model.Albums{ alCover, }) @@ -270,24 +305,24 @@ var _ = XDescribe("Artwork", func() { }) }) -//func createImage(format string, landscape bool, size int) string { -// var img image.Image -// -// if landscape { -// img = image.NewRGBA(image.Rect(0, 0, size, size/2)) -// } else { -// img = image.NewRGBA(image.Rect(0, 0, size/2, size)) -// } -// -// tmpDir := GinkgoT().TempDir() -// f, _ := os.Create(filepath.Join(tmpDir, "cover."+format)) -// defer f.Close() -// switch format { -// case "png": -// _ = png.Encode(f, img) -// case "jpg": -// _ = jpeg.Encode(f, img, &jpeg.Options{Quality: 75}) -// } -// -// return tmpDir -//} +func createImage(format string, landscape bool, size int) string { + var img image.Image + + if landscape { + img = image.NewRGBA(image.Rect(0, 0, size, size/2)) + } else { + img = image.NewRGBA(image.Rect(0, 0, size/2, size)) + } + + tmpDir := GinkgoT().TempDir() + f, _ := os.Create(filepath.Join(tmpDir, "cover."+format)) + defer f.Close() + switch format { + case "png": + _ = png.Encode(f, img) + case "jpg": + _ = jpeg.Encode(f, img, &jpeg.Options{Quality: 75}) + } + + return tmpDir +} diff --git a/core/artwork/reader_artist.go b/core/artwork/reader_artist.go index 217044b7..487346b4 100644 --- a/core/artwork/reader_artist.go +++ b/core/artwork/reader_artist.go @@ -29,7 +29,7 @@ type artistReader struct { imgFiles []string } -func newArtistReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, provider external.Provider) (*artistReader, error) { +func newArtistArtworkReader(ctx context.Context, artwork *artwork, artID model.ArtworkID, provider external.Provider) (*artistReader, error) { ar, err := artwork.ds.Artist(ctx).Get(artID.ID) if err != nil { return nil, err diff --git a/core/artwork/reader_artist_test.go b/core/artwork/reader_artist_test.go index a8dfddea..294a5db0 100644 --- a/core/artwork/reader_artist_test.go +++ b/core/artwork/reader_artist_test.go @@ -12,7 +12,7 @@ import ( . "github.com/onsi/gomega" ) -var _ = Describe("artistReader", func() { +var _ = Describe("artistArtworkReader", func() { var _ = Describe("loadArtistFolder", func() { var ( ctx context.Context