mirror of
https://github.com/navidrome/navidrome.git
synced 2026-08-01 07:21:17 +00:00
test(artwork): cover the album-root and artist-folder path arithmetic
The unit specs for these helpers lived in the readers #5856 patched, both deleted here, leaving the album-root promotion and the artist-folder climb covered only end-to-end. A layout can show which image won but not why, so these pin the parts behind it: that the parent is fetched only when it could qualify as an album root, that a failed fetch propagates or degrades, and that commonDir keeps a shared name fragment from reading as a shared folder. Each spec was checked against a mutant: removing the library-root guard, the other-album audio check, the single-folder short circuit, the single-root climb, or commonDir's separator all turn one red. Master's remaining specs were dropped as redundant — they cover sources this branch already exercises under different names.
This commit is contained in:
Deluan
parent
b876bdf28e
commit
fa8cc91698
@ -82,12 +82,19 @@ func (s *osDirStorage) FS() (storage.MusicFS, error) {
|
||||
return osDirFS{os.DirFS(s.root)}, nil
|
||||
}
|
||||
|
||||
// fakeFolderRepo covers the three FolderRepository methods the resolvers reach for; only the
|
||||
// folder listing varies per spec, so the other two answer as an unremarkable library does.
|
||||
// fakeFolderRepo covers the three FolderRepository methods the resolvers reach for. The zero value
|
||||
// answers as an unremarkable library does; the fields drive the album-root lookup and its failures.
|
||||
type fakeFolderRepo struct {
|
||||
model.FolderRepository
|
||||
result []model.Folder
|
||||
err error
|
||||
result []model.Folder
|
||||
err error
|
||||
parentResult *model.Folder
|
||||
getErr error
|
||||
getCallCount int
|
||||
// hasOtherAudio is returned by HasAudioOutsideFolders (the album-root check).
|
||||
// False means the parent qualifies as an album root.
|
||||
hasOtherAudio bool
|
||||
otherAudioErr error
|
||||
}
|
||||
|
||||
func (f *fakeFolderRepo) GetAll(...model.QueryOptions) ([]model.Folder, error) {
|
||||
@ -95,9 +102,16 @@ func (f *fakeFolderRepo) GetAll(...model.QueryOptions) ([]model.Folder, error) {
|
||||
}
|
||||
|
||||
func (f *fakeFolderRepo) HasAudioOutsideFolders(model.Folder, []string) (bool, error) {
|
||||
return false, nil
|
||||
return f.hasOtherAudio, f.otherAudioErr
|
||||
}
|
||||
|
||||
func (f *fakeFolderRepo) Get(string) (*model.Folder, error) {
|
||||
f.getCallCount++
|
||||
if f.getErr != nil {
|
||||
return nil, f.getErr
|
||||
}
|
||||
if f.parentResult != nil {
|
||||
return f.parentResult, nil
|
||||
}
|
||||
return nil, model.ErrNotFound
|
||||
}
|
||||
|
||||
214
core/artwork/folders_album_test.go
Normal file
214
core/artwork/folders_album_test.go
Normal file
@ -0,0 +1,214 @@
|
||||
package artwork
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
// The e2e specs cover which image wins for a layout; these pin what a layout cannot reach: that
|
||||
// the album-root parent is only fetched when it could qualify, and what happens when that fails.
|
||||
var _ = Describe("loadAlbumFoldersPaths", func() {
|
||||
var (
|
||||
ctx context.Context
|
||||
ds *tests.MockDataStore
|
||||
repo *fakeFolderRepo
|
||||
album model.Album
|
||||
now time.Time
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = context.Background()
|
||||
now = time.Now().Truncate(time.Second)
|
||||
repo = &fakeFolderRepo{}
|
||||
ds = &tests.MockDataStore{MockedFolder: repo}
|
||||
album = model.Album{
|
||||
ID: "album1",
|
||||
Name: "Album",
|
||||
FolderIDs: []string{"folder1", "folder2", "folder3"},
|
||||
}
|
||||
})
|
||||
|
||||
It("does not query the parent when it is already one of the album's folders", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist", Name: "Album", ParentID: "folder2",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
{ID: "folder2", Path: "", Name: "Artist", ImagesUpdatedAt: now},
|
||||
}
|
||||
|
||||
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("Artist/Album/cover.jpg"))
|
||||
Expect(repo.getCallCount).To(BeZero())
|
||||
})
|
||||
|
||||
It("does not query the parent when the album's folders have different parents", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist1/Album", Name: "part1", ParentID: "parentA",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
{ID: "folder2", Path: "Artist2/Album", Name: "part2", ParentID: "parentB",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
|
||||
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("Artist1/Album/part1/cover.jpg"))
|
||||
Expect(repo.getCallCount).To(BeZero())
|
||||
})
|
||||
|
||||
It("does not query the parent for a single-folder album that has images of its own", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist", Name: "Album", ParentID: "artistFolder",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
}
|
||||
|
||||
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("Artist/Album/cover.jpg"))
|
||||
Expect(repo.getCallCount).To(BeZero())
|
||||
})
|
||||
|
||||
It("does not promote the library root, so its images never become album art", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: ".", Name: "AlbumPart1", ParentID: "rootFolder",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
{ID: "folder2", Path: ".", Name: "AlbumPart2", ParentID: "rootFolder",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
repo.parentResult = &model.Folder{ID: "rootFolder", Name: ".", ImageFiles: []string{"unrelated.jpg"}}
|
||||
|
||||
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("AlbumPart1/cover.jpg"))
|
||||
Expect(repo.getCallCount).To(Equal(1))
|
||||
})
|
||||
|
||||
It("does not promote a parent that holds another album's audio", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist/Album", Name: "CD1", ParentID: "albumFolder",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
{ID: "folder2", Path: "Artist/Album", Name: "CD2", ParentID: "albumFolder",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
repo.parentResult = &model.Folder{ID: "albumFolder", Path: "Artist", Name: "Album",
|
||||
ParentID: "artistFolder", ImageFiles: []string{"artist.jpg"}}
|
||||
repo.hasOtherAudio = true
|
||||
|
||||
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("Artist/Album/CD1/cover.jpg"))
|
||||
})
|
||||
|
||||
It("promotes the album root parent into the returned paths", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist/Album", Name: "CD1", ParentID: "albumFolder",
|
||||
ImagesUpdatedAt: now},
|
||||
{ID: "folder2", Path: "Artist/Album", Name: "CD2", ParentID: "albumFolder",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
repo.parentResult = &model.Folder{ID: "albumFolder", Path: "Artist", Name: "Album",
|
||||
ParentID: "artistFolder", ImageFiles: []string{"cover.jpg"}}
|
||||
|
||||
paths, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("Artist/Album/cover.jpg"))
|
||||
Expect(paths).To(HaveLen(3))
|
||||
})
|
||||
|
||||
It("propagates errors from the album-root check", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist/Album", Name: "disc1", ParentID: "albumFolder",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
repo.parentResult = &model.Folder{ID: "albumFolder", Path: "Artist", Name: "Album",
|
||||
ParentID: "artistFolder", ImageFiles: []string{"cover.jpg"}}
|
||||
repo.otherAudioErr = errors.New("db connection failed")
|
||||
|
||||
_, _, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).To(MatchError("db connection failed"))
|
||||
})
|
||||
|
||||
It("propagates non-ErrNotFound errors from the parent folder lookup", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist/Album", Name: "CD1", ParentID: "parentFolder",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
{ID: "folder2", Path: "Artist/Album", Name: "CD2", ParentID: "parentFolder",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
repo.getErr = errors.New("db connection failed")
|
||||
|
||||
_, _, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).To(MatchError("db connection failed"))
|
||||
Expect(repo.getCallCount).To(Equal(1))
|
||||
})
|
||||
|
||||
It("continues when the parent folder has been deleted", func() {
|
||||
repo.result = []model.Folder{
|
||||
{ID: "folder1", Path: "Artist/Album", Name: "CD1", ParentID: "missingParent",
|
||||
ImagesUpdatedAt: now, ImageFiles: []string{"cover.jpg"}},
|
||||
{ID: "folder2", Path: "Artist/Album", Name: "CD2", ParentID: "missingParent",
|
||||
ImagesUpdatedAt: now},
|
||||
}
|
||||
|
||||
_, imgFiles, _, err := loadAlbumFoldersPaths(ctx, ds, album)
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(imgFiles).To(ConsistOf("Artist/Album/CD1/cover.jpg"))
|
||||
Expect(repo.getCallCount).To(Equal(1))
|
||||
})
|
||||
})
|
||||
|
||||
// folderImages is the sort that decides which of several same-named images wins, so it is pinned
|
||||
// directly rather than through a layout that can only show the winner.
|
||||
var _ = Describe("folderImages", func() {
|
||||
It("prefers base filenames over numeric-suffixed ones", func() {
|
||||
imgFiles, _ := folderImages([]model.Folder{
|
||||
{Path: "Artist", Name: "Album", ImageFiles: []string{"cover.1.jpg", "cover.jpg", "cover.2.jpg"}},
|
||||
})
|
||||
|
||||
Expect(imgFiles).To(HaveExactElements(
|
||||
"Artist/Album/cover.jpg", "Artist/Album/cover.1.jpg", "Artist/Album/cover.2.jpg"))
|
||||
})
|
||||
|
||||
It("prefers shallower paths when the base filenames tie", func() {
|
||||
imgFiles, _ := folderImages([]model.Folder{
|
||||
{Path: "Artist/Album", Name: "CD1", ImageFiles: []string{"cover.jpg"}},
|
||||
{Path: "Artist", Name: "Album", ImageFiles: []string{"cover.jpg"}},
|
||||
})
|
||||
|
||||
Expect(imgFiles).To(HaveExactElements("Artist/Album/cover.jpg", "Artist/Album/CD1/cover.jpg"))
|
||||
})
|
||||
|
||||
It("sorts case-insensitively", func() {
|
||||
imgFiles, _ := folderImages([]model.Folder{
|
||||
{Path: "Artist", Name: "Album", ImageFiles: []string{"Cover.jpg", "back.JPG"}},
|
||||
})
|
||||
|
||||
Expect(imgFiles).To(HaveExactElements("Artist/Album/back.JPG", "Artist/Album/Cover.jpg"))
|
||||
})
|
||||
|
||||
It("reports the newest ImagesUpdatedAt across the folders", func() {
|
||||
now := time.Now().Truncate(time.Second)
|
||||
newest := now.Add(5 * time.Minute)
|
||||
|
||||
_, updatedAt := folderImages([]model.Folder{
|
||||
{Path: "Artist", Name: "Album", ImagesUpdatedAt: now},
|
||||
{Path: "Artist/Album", Name: "CD1", ImagesUpdatedAt: newest},
|
||||
})
|
||||
|
||||
Expect(updatedAt).To(Equal(newest))
|
||||
})
|
||||
})
|
||||
117
core/artwork/folders_artist_paths_test.go
Normal file
117
core/artwork/folders_artist_paths_test.go
Normal file
@ -0,0 +1,117 @@
|
||||
package artwork
|
||||
|
||||
import (
|
||||
"context"
|
||||
"errors"
|
||||
"path/filepath"
|
||||
"time"
|
||||
|
||||
"github.com/navidrome/navidrome/core"
|
||||
"github.com/navidrome/navidrome/model"
|
||||
"github.com/navidrome/navidrome/tests"
|
||||
. "github.com/onsi/ginkgo/v2"
|
||||
. "github.com/onsi/gomega"
|
||||
)
|
||||
|
||||
// commonDir and loadArtistFolder decide how far above the albums the artist folder sits. A layout
|
||||
// can only show the image that won, so the arithmetic is pinned here.
|
||||
var _ = Describe("commonDir", func() {
|
||||
It("returns the folder itself for a single path", func() {
|
||||
Expect(commonDir([]string{filepath.FromSlash("/music/artist/album")})).
|
||||
To(Equal(filepath.FromSlash("/music/artist/album")))
|
||||
})
|
||||
|
||||
It("returns the deepest shared folder", func() {
|
||||
Expect(commonDir([]string{
|
||||
filepath.FromSlash("/music/artist/album/cd1"),
|
||||
filepath.FromSlash("/music/artist/album/cd2"),
|
||||
})).To(Equal(filepath.FromSlash("/music/artist/album")))
|
||||
})
|
||||
|
||||
It("does not read a shared name fragment as a shared folder", func() {
|
||||
Expect(commonDir([]string{
|
||||
filepath.FromSlash("/music/artist/Album"),
|
||||
filepath.FromSlash("/music/artist/Album2"),
|
||||
})).To(Equal(filepath.FromSlash("/music/artist")))
|
||||
})
|
||||
})
|
||||
|
||||
var _ = Describe("loadArtistFolder", func() {
|
||||
var (
|
||||
ctx context.Context
|
||||
ds *tests.MockDataStore
|
||||
repo *fakeFolderRepo
|
||||
albums model.Albums
|
||||
updatedAt time.Time
|
||||
)
|
||||
|
||||
BeforeEach(func() {
|
||||
ctx = context.Background()
|
||||
DeferCleanup(stubCoreAbsolutePath())
|
||||
|
||||
updatedAt = time.Now().Truncate(time.Second).Add(5 * time.Minute)
|
||||
repo = &fakeFolderRepo{result: []model.Folder{{ImagesUpdatedAt: updatedAt}}}
|
||||
ds = &tests.MockDataStore{MockedFolder: repo}
|
||||
albums = model.Albums{{LibraryID: 1, ID: "album1", Name: "Album 1"}}
|
||||
})
|
||||
|
||||
It("returns empty when the artist has no albums", func() {
|
||||
folder, upd, err := loadArtistFolder(ctx, ds, model.Albums{}, []string{"/dummy/path"})
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(folder).To(BeEmpty())
|
||||
Expect(upd).To(BeZero())
|
||||
})
|
||||
|
||||
It("climbs above the album folder when the artist has a single album root", func() {
|
||||
folder, upd, err := loadArtistFolder(ctx, ds, albums,
|
||||
[]string{filepath.FromSlash("/music/artist/album1")})
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(folder).To(Equal(filepath.FromSlash("/music/artist")))
|
||||
Expect(upd).To(Equal(updatedAt))
|
||||
})
|
||||
|
||||
It("climbs above the shared folder when two albums live in the same one", func() {
|
||||
folder, upd, err := loadArtistFolder(ctx, ds, albums, []string{
|
||||
filepath.FromSlash("/music/artist/split"),
|
||||
filepath.FromSlash("/music/artist/split"),
|
||||
})
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(folder).To(Equal(filepath.FromSlash("/music/artist")))
|
||||
Expect(upd).To(Equal(updatedAt))
|
||||
})
|
||||
|
||||
It("stops at the folder where distinct album roots already meet", func() {
|
||||
folder, upd, err := loadArtistFolder(ctx, ds, albums, []string{
|
||||
filepath.FromSlash("/music/artist/album1"),
|
||||
filepath.FromSlash("/music/artist/album2"),
|
||||
})
|
||||
|
||||
Expect(err).ToNot(HaveOccurred())
|
||||
Expect(folder).To(Equal(filepath.FromSlash("/music/artist")))
|
||||
Expect(upd).To(Equal(updatedAt))
|
||||
})
|
||||
|
||||
It("returns the error when the folder lookup fails", func() {
|
||||
repo.err = errors.New("fake error")
|
||||
|
||||
folder, upd, err := loadArtistFolder(ctx, ds, albums, []string{
|
||||
filepath.FromSlash("/music/artist/album1"),
|
||||
filepath.FromSlash("/music/artist/album2"),
|
||||
})
|
||||
|
||||
Expect(err).To(MatchError(ContainSubstring("fake error")))
|
||||
Expect(folder).To(BeEmpty())
|
||||
Expect(upd).To(BeZero())
|
||||
})
|
||||
})
|
||||
|
||||
func stubCoreAbsolutePath() func() {
|
||||
original := core.AbsolutePath
|
||||
core.AbsolutePath = func(context.Context, model.DataStore, int, string) string {
|
||||
return filepath.FromSlash("/music")
|
||||
}
|
||||
return func() { core.AbsolutePath = original }
|
||||
}
|
||||
Loading…
x
Reference in New Issue
Block a user