From ca8f4be3696008cde2214d3f768818fdbc8a0376 Mon Sep 17 00:00:00 2001 From: Deluan Date: Sat, 25 Jul 2026 15:16:35 -0400 Subject: [PATCH] test(artwork): inject the unreadable source instead of chmod MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit os.Chmod cannot revoke read access on Windows — it only toggles the read-only attribute — so the findImageInFolder spec opened the file happily and failed there, and the upload spec passed for the wrong reason: outcomeFailed came from the 1-byte payload failing to decode, not from the source being unreadable. findImageInFolder takes an fs.FS, so the failure is now injected and the spec is filesystem-independent. The upload path goes through os.Open directly and has nothing to inject, so it skips on Windows rather than pretend to cover it. --- core/artwork/folders_artist_test.go | 28 ++++++++++++++-------------- core/artwork/processor_test.go | 6 ++++++ 2 files changed, 20 insertions(+), 14 deletions(-) diff --git a/core/artwork/folders_artist_test.go b/core/artwork/folders_artist_test.go index e27b591b2..27b52c9e4 100644 --- a/core/artwork/folders_artist_test.go +++ b/core/artwork/folders_artist_test.go @@ -3,26 +3,31 @@ package artwork import ( "context" "errors" - "os" - "path/filepath" + "io/fs" + "testing/fstest" . "github.com/onsi/ginkgo/v2" . "github.com/onsi/gomega" ) +// unreadableFS globs like its embedded MapFS but refuses to open anything, standing in for a +// stale mount or a permissions failure. Injecting the error keeps this independent of the +// filesystem: os.Chmod does not restrict read access on Windows. +type unreadableFS struct{ fstest.MapFS } + +func (u unreadableFS) Open(string) (fs.File, error) { return nil, fs.ErrPermission } + var _ = Describe("findImageInFolder", func() { var ctx context.Context - var dir string + var files fstest.MapFS BeforeEach(func() { ctx = context.Background() - dir = GinkgoT().TempDir() + files = fstest.MapFS{"artist.jpg": &fstest.MapFile{Data: []byte("img")}} }) It("returns the first matching image", func() { - Expect(os.WriteFile(filepath.Join(dir, "artist.jpg"), []byte("img"), 0o600)).To(Succeed()) - - r, hit, err := findImageInFolder(ctx, os.DirFS(dir), ".", dir, "artist.*") + r, hit, err := findImageInFolder(ctx, files, ".", "/lib", "artist.*") Expect(err).ToNot(HaveOccurred()) defer r.Close() Expect(hit).To(HaveSuffix("artist.jpg")) @@ -31,17 +36,12 @@ var _ = Describe("findImageInFolder", func() { // The glob matched, so the image exists; failing to open it says nothing about whether the // artist has one, and must not let the resolver settle on absent. It("reports a matched but unreadable image as unreadable, not as a miss", func() { - img := filepath.Join(dir, "artist.jpg") - Expect(os.WriteFile(img, []byte("img"), 0o600)).To(Succeed()) - Expect(os.Chmod(img, 0o000)).To(Succeed()) - DeferCleanup(func() { _ = os.Chmod(img, 0o600) }) - - _, _, err := findImageInFolder(ctx, os.DirFS(dir), ".", dir, "artist.*") + _, _, err := findImageInFolder(ctx, unreadableFS{files}, ".", "/lib", "artist.*") Expect(err).To(MatchError(errSourceUnreadable)) }) It("reports a plain miss when nothing matches", func() { - _, _, err := findImageInFolder(ctx, os.DirFS(dir), ".", dir, "artist.*") + _, _, err := findImageInFolder(ctx, files, ".", "/lib", "nothing.*") Expect(err).To(HaveOccurred()) Expect(errors.Is(err, errSourceUnreadable)).To(BeFalse(), "no match is definitive, not transient") }) diff --git a/core/artwork/processor_test.go b/core/artwork/processor_test.go index 45b288072..0f5f48c50 100644 --- a/core/artwork/processor_test.go +++ b/core/artwork/processor_test.go @@ -9,6 +9,7 @@ import ( "net/http/httptest" "os" "path/filepath" + "runtime" "time" "github.com/navidrome/navidrome/conf" @@ -161,6 +162,11 @@ var _ = Describe("processItem", func() { // An upload outranks every other source, so an unreadable one must neither settle absent // nor let a lower-priority image take its place. It("failed-on-unreadable-upload: an upload that will not open never records absent", func() { + if runtime.GOOS == "windows" { + // os.Chmod cannot revoke read access there, so the file would open and the spec + // would pass on the decode error instead of the unreadable source. + Skip("chmod does not restrict read access on Windows") + } radioRepo := tests.CreateMockedRadioRepo() radioRepo.Data = map[string]*model.Radio{} ds.MockedRadio = radioRepo