test(storage): re-enable local storage tests on Windows (#5654)

* refactor(storage): extract LocalPathToURL from storage.For

* test(storage): re-enable local storage tests on Windows (#5381)

* test(storage): fix Windows drive-letter path expectation

The 'should handle Windows drive letters correctly' test was gated behind
the now-removed SkipOnWindows BeforeEach, so its expectation had never run.
On Windows, newLocalStorage re-joins u.Host+u.Path via filepath.Join, which
yields a backslash path (C:\music), not C:/music. Assert against
filepath.Join so the expectation matches the OS-native result.

* test(storage): probe Windows drive-letter path in LocalPathToURL

Three review bots flagged that LocalPathToURL escapes the drive-letter
colon (C: -> C%3A), which url.Parse rejects. There are no Windows bug
reports, so add a Windows-gated test that exercises the real conversion
on a drive-letter path and let CI decide whether the bug is real before
changing production code.

core/storage/local/local_test.go

@@ -13,7 +13,6 @@ import (
 	"github.com/navidrome/navidrome/consts"
 	"github.com/navidrome/navidrome/core/storage"
 	"github.com/navidrome/navidrome/model/metadata"
-	"github.com/navidrome/navidrome/tests"
 	. "github.com/onsi/ginkgo/v2"
 	. "github.com/onsi/gomega"
 )
@@ -45,16 +44,13 @@ var _ = Describe("LocalStorage", func() {
 	})
 
 	Describe("newLocalStorage", func() {
-		BeforeEach(func() {
-			tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)")
-		})
 
 		Context("with valid path", func() {
 			It("should create a localStorage instance with correct path", func() {
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
 
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				localStorage := storage.(*localStorage)
 
 				Expect(localStorage.u.Scheme).To(Equal("file"))
@@ -94,10 +90,10 @@ var _ = Describe("LocalStorage", func() {
 				err = os.Symlink(realDir, linkDir)
 				Expect(err).ToNot(HaveOccurred())
 
-				u, err := url.Parse("file://" + linkDir)
+				u, err := storage.LocalPathToURL(linkDir)
 				Expect(err).ToNot(HaveOccurred())
 
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				localStorage, ok := storage.(*localStorage)
 				Expect(ok).To(BeTrue())
 
@@ -110,10 +106,10 @@ var _ = Describe("LocalStorage", func() {
 				// Use a non-existent path to trigger symlink resolution failure
 				nonExistentPath := filepath.Join(tempDir, "non-existent")
 
-				u, err := url.Parse("file://" + nonExistentPath)
+				u, err := storage.LocalPathToURL(nonExistentPath)
 				Expect(err).ToNot(HaveOccurred())
 
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				localStorage, ok := storage.(*localStorage)
 				Expect(ok).To(BeTrue())
 
@@ -137,7 +133,9 @@ var _ = Describe("LocalStorage", func() {
 				localStorage, ok := storage.(*localStorage)
 				Expect(ok).To(BeTrue())
 
-				Expect(localStorage.u.Path).To(Equal("C:/music"))
+				// newLocalStorage re-joins the drive letter (u.Host) with u.Path via
+				// filepath.Join, which yields an OS-native (backslash) path on Windows.
+				Expect(localStorage.u.Path).To(Equal(filepath.Join("C:", "/music")))
 			})
 		})
 
@@ -159,10 +157,10 @@ var _ = Describe("LocalStorage", func() {
 			It("falls back to the default extractor instead of crashing", func() {
 				conf.Server.Scanner.Extractor = "nonexistent-extractor"
 
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
 
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				ls, ok := storage.(*localStorage)
 				Expect(ok).To(BeTrue())
 				Expect(ls.extractor).To(BeIdenticalTo(defaultExtractor))
@@ -171,16 +169,13 @@ var _ = Describe("LocalStorage", func() {
 	})
 
 	Describe("localStorage.FS", func() {
-		BeforeEach(func() {
-			tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)")
-		})
 
 		Context("with existing directory", func() {
 			It("should return a localFS instance", func() {
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
 
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				musicFS, err := storage.FS()
 				Expect(err).ToNot(HaveOccurred())
 				Expect(musicFS).ToNot(BeNil())
@@ -193,10 +188,10 @@ var _ = Describe("LocalStorage", func() {
 		Context("with non-existent directory", func() {
 			It("should return an error", func() {
 				nonExistentPath := filepath.Join(tempDir, "non-existent")
-				u, err := url.Parse("file://" + nonExistentPath)
+				u, err := storage.LocalPathToURL(nonExistentPath)
 				Expect(err).ToNot(HaveOccurred())
 
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				_, err = storage.FS()
 				Expect(err).To(HaveOccurred())
 				Expect(err.Error()).To(ContainSubstring(nonExistentPath))
@@ -208,7 +203,6 @@ var _ = Describe("LocalStorage", func() {
 		var testFile string
 
 		BeforeEach(func() {
-			tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)")
 			// Create a test file
 			testFile = filepath.Join(tempDir, "test.mp3")
 			err := os.WriteFile(testFile, []byte("test data"), 0600)
@@ -235,9 +229,9 @@ var _ = Describe("LocalStorage", func() {
 
 				testExtractor.results["test.mp3"] = expectedInfo
 
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				musicFS, err := storage.FS()
 				Expect(err).ToNot(HaveOccurred())
 
@@ -259,9 +253,9 @@ var _ = Describe("LocalStorage", func() {
 
 				testExtractor.results["test.mp3"] = incompleteInfo
 
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				musicFS, err := storage.FS()
 				Expect(err).ToNot(HaveOccurred())
 
@@ -288,9 +282,9 @@ var _ = Describe("LocalStorage", func() {
 
 				testExtractor.results["non-existent.mp3"] = incompleteInfo
 
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				musicFS, err := storage.FS()
 				Expect(err).ToNot(HaveOccurred())
 
@@ -303,9 +297,9 @@ var _ = Describe("LocalStorage", func() {
 			It("should return the extractor error", func() {
 				testExtractor.err = &extractorError{message: "extractor failed"}
 
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				musicFS, err := storage.FS()
 				Expect(err).ToNot(HaveOccurred())
 
@@ -334,9 +328,9 @@ var _ = Describe("LocalStorage", func() {
 				testExtractor.results["test.mp3"] = info1
 				testExtractor.results["test2.mp3"] = info2
 
-				u, err := url.Parse("file://" + tempDir)
+				u, err := storage.LocalPathToURL(tempDir)
 				Expect(err).ToNot(HaveOccurred())
-				storage := newLocalStorage(*u)
+				storage := newLocalStorage(u)
 				musicFS, err := storage.FS()
 				Expect(err).ToNot(HaveOccurred())
 
@@ -390,9 +384,8 @@ var _ = Describe("LocalStorage", func() {
 
 	Describe("Storage registration", func() {
 		It("should register localStorage for file scheme", func() {
-			tests.SkipOnWindows("path separator bug (#TBD-path-sep-storage-local)")
 			// This tests the init() function indirectly
-			storage, err := storage.For("file://" + tempDir)
+			storage, err := storage.For(tempDir)
 			Expect(err).ToNot(HaveOccurred())
 			Expect(storage).To(BeAssignableToTypeOf(&localStorage{}))
 		})

core/storage/storage.go

@@ -25,6 +25,27 @@ func Register(schema string, c constructor) {
 	registry[schema] = c
 }
 
+// LocalPathToURL converts a bare OS filesystem path into an absolute file:// URL,
+// applying the same slash-normalisation and per-component escaping the scanner
+// relies on. It is the single source of truth for how a local path becomes a
+// storage URL, shared by For and by storage tests.
+func LocalPathToURL(osPath string) (url.URL, error) {
+	abs, _ := filepath.Abs(osPath)
+	abs = filepath.ToSlash(abs)
+
+	// Properly escape each path component using URL standards
+	pathParts := strings.Split(abs, "/")
+	escapedParts := slice.Map(pathParts, func(s string) string {
+		return url.PathEscape(s)
+	})
+
+	u, err := url.Parse(LocalSchemaID + "://" + strings.Join(escapedParts, "/"))
+	if err != nil {
+		return url.URL{}, err
+	}
+	return *u, nil
+}
+
 // For returns a Storage implementation for the given URI.
 // It uses the schema part of the URI to find the correct registered
 // Storage constructor.
@@ -34,24 +55,22 @@ func For(uri string) (Storage, error) {
 	defer lock.RUnlock()
 	parts := strings.Split(uri, "://")
 
+	var u *url.URL
 	// Paths without schema are treated as file:// and use the default LocalStorage implementation
 	if len(parts) < 2 {
-		uri, _ = filepath.Abs(uri)
-		uri = filepath.ToSlash(uri)
-
-		// Properly escape each path component using URL standards
-		pathParts := strings.Split(uri, "/")
-		escapedParts := slice.Map(pathParts, func(s string) string {
-			return url.PathEscape(s)
-		})
-
-		uri = LocalSchemaID + "://" + strings.Join(escapedParts, "/")
+		parsed, err := LocalPathToURL(uri)
+		if err != nil {
+			return nil, err
+		}
+		u = &parsed
+	} else {
+		parsed, err := url.Parse(uri)
+		if err != nil {
+			return nil, err
+		}
+		u = parsed
 	}
 
-	u, err := url.Parse(uri)
-	if err != nil {
-		return nil, err
-	}
 	c, ok := registry[u.Scheme]
 	if !ok {
 		return nil, errors.New("schema '" + u.Scheme + "' not registered")

core/storage/storage_test.go

@@ -4,6 +4,7 @@ import (
 	"net/url"
 	"os"
 	"path/filepath"
+	"runtime"
 	"testing"
 
 	"github.com/navidrome/navidrome/tests"
@@ -83,6 +84,43 @@ var _ = Describe("Storage", func() {
 			Entry("multiple special chars", "/tmp/Song #1 & More?.mp3"),
 		)
 	})
+
+	Describe("LocalPathToURL", func() {
+		It("builds a file:// URL from an absolute path", func() {
+			u, err := LocalPathToURL("/tmp/music")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(u.Scheme).To(Equal("file"))
+			Expect(u.Path).To(Equal("/tmp/music"))
+		})
+
+		It("escapes special characters and decodes them back in Path", func() {
+			u, err := LocalPathToURL("/tmp/Song #1 & More?.mp3")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(u.Path).To(Equal("/tmp/Song #1 & More?.mp3"))
+		})
+
+		It("produces the same url.URL that For uses for a bare path", func() {
+			registry = map[string]constructor{}
+			Register("file", func(url url.URL) Storage { return &fakeLocalStorage{u: url} })
+			s, err := For("/tmp/music")
+			Expect(err).ToNot(HaveOccurred())
+			direct, err := LocalPathToURL("/tmp/music")
+			Expect(err).ToNot(HaveOccurred())
+			Expect(s.(*fakeLocalStorage).u).To(Equal(direct))
+		})
+
+		// On Windows, library paths are drive-lettered (e.g. C:\Music). This
+		// exercises the real conversion to confirm a drive-letter path yields a
+		// usable file:// URL on the actual platform (no-op on Unix).
+		It("handles a Windows drive-letter path", func() {
+			if runtime.GOOS != "windows" {
+				Skip("Windows-specific path handling")
+			}
+			u, err := LocalPathToURL(`C:\Music`)
+			Expect(err).ToNot(HaveOccurred())
+			Expect(u.Scheme).To(Equal("file"))
+		})
+	})
 })
 
 type fakeLocalStorage struct {