From a18a2c4dbd801cfd11538c5b6b8df0ed454789a5 Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 4 Mar 2026 08:44:49 +0800 Subject: [PATCH 1/2] fix(mediascanner): prevent indefinite hangs on stale network mounts Wrap filesystem operations (os.Stat, os.ReadDir, os.Lstat, filepath.EvalSymlinks) with context-aware timeouts to prevent the media scanner from hanging indefinitely on stale NFS/CIFS mounts. Introduce a directory cache (dirCache) for case-insensitive path lookups to reduce repeated filesystem calls during scanning. Also: - Handle SQLite UNIQUE constraint failures gracefully during indexing - Add phase constants (PhaseDiscovering, PhaseInitializing) replacing magic strings in status reporting - Switch GOPROXY from goproxy.io to direct - Fix test assertion for updated Total calculation formula --- Taskfile.dist.yml | 2 +- pkg/api/methods/media.go | 8 +- pkg/api/methods/methods_test.go | 15 +- pkg/database/mediascanner/dircache.go | 78 ++++++++ pkg/database/mediascanner/dircache_test.go | 123 +++++++++++++ .../mediascanner/findpath_fuzz_test.go | 6 +- pkg/database/mediascanner/findpath_test.go | 41 +++-- pkg/database/mediascanner/fsutil.go | 113 ++++++++++++ pkg/database/mediascanner/fsutil_test.go | 157 ++++++++++++++++ pkg/database/mediascanner/mediascanner.go | 172 +++++++++++++++--- .../mediascanner/mediascanner_test.go | 8 +- pkg/platforms/mister/platform.go | 8 +- pkg/platforms/windows/retrobat.go | 4 +- 13 files changed, 672 insertions(+), 63 deletions(-) create mode 100644 pkg/database/mediascanner/dircache.go create mode 100644 pkg/database/mediascanner/dircache_test.go create mode 100644 pkg/database/mediascanner/fsutil.go create mode 100644 pkg/database/mediascanner/fsutil_test.go diff --git a/Taskfile.dist.yml b/Taskfile.dist.yml index 89472ed8..73795eca 100644 --- a/Taskfile.dist.yml +++ b/Taskfile.dist.yml @@ -2,7 +2,7 @@ version: "3" env: GOTOOLCHAIN: auto - GOPROXY: https://goproxy.io,direct + GOPROXY: direct APP_VERSION: sh: echo -n "${APP_VERSION:-$(git rev-parse --short HEAD)-dev}" UID: diff --git a/pkg/api/methods/media.go b/pkg/api/methods/media.go index 34cd9843..3498f878 100644 --- a/pkg/api/methods/media.go +++ b/pkg/api/methods/media.go @@ -275,10 +275,12 @@ func GenerateMediaDB( defer db.MediaDB.BackgroundOperationDone() total, err := mediascanner.NewNamesIndex(indexCtx, pl, cfg, systems, db, func(status mediascanner.IndexStatus) { var desc string - switch status.Step { - case 1: + switch { + case status.Phase == mediascanner.PhaseDiscovering: desc = "Finding media folders" - case status.Total: + case status.Phase == mediascanner.PhaseInitializing: + desc = "Initializing database" + case status.Step == status.Total: desc = "Writing database" default: system, err := systemdefs.GetSystem(status.SystemID) diff --git a/pkg/api/methods/methods_test.go b/pkg/api/methods/methods_test.go index 4ae6a947..54bdb7e2 100644 --- a/pkg/api/methods/methods_test.go +++ b/pkg/api/methods/methods_test.go @@ -24,6 +24,7 @@ import ( "database/sql" "errors" "testing" + "time" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models" "github.com/ZaparooProject/zaparoo-core/v2/pkg/api/models/requests" @@ -578,8 +579,10 @@ func TestHandleGenerateMedia_SystemFiltering(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // Reset indexing status to prevent race conditions between parallel tests - statusInstance.clear() + // Cancel any leftover background goroutine and wait for it + // to release the global statusInstance before starting. + CancelIndexing() + ClearIndexingStatus() // Create mock environment mockPlatform := mocks.NewMockPlatform() @@ -674,6 +677,14 @@ func TestHandleGenerateMedia_SystemFiltering(t *testing.T) { require.NoError(t, err) assert.Nil(t, result) + // Cancel the background indexing goroutine and wait for it + // to finish so it doesn't leak into the next subtest. + appState.StopService() + require.Eventually(t, func() bool { + return !statusInstance.isRunning() + }, 5*time.Second, 10*time.Millisecond, + "background indexing goroutine did not stop") + // Verify mock expectations were met mockMediaDB.AssertExpectations(t) mockPlatform.AssertExpectations(t) diff --git a/pkg/database/mediascanner/dircache.go b/pkg/database/mediascanner/dircache.go new file mode 100644 index 00000000..e823664c --- /dev/null +++ b/pkg/database/mediascanner/dircache.go @@ -0,0 +1,78 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package mediascanner + +import ( + "context" + "os" + "runtime" + "strings" +) + +// dirCache caches directory listings to avoid redundant filesystem operations +// during path discovery. Not safe for concurrent use — intended to be created +// and consumed within a single GetSystemPaths call. +type dirCache struct { + entries map[string][]os.DirEntry +} + +func newDirCache() *dirCache { + return &dirCache{entries: make(map[string][]os.DirEntry)} +} + +// list returns the directory entries for the given path, reading from cache +// if available or performing a readDirWithContext and caching the result. +func (c *dirCache) list(ctx context.Context, path string) ([]os.DirEntry, error) { + if cached, ok := c.entries[path]; ok { + return cached, nil + } + entries, err := readDirWithContext(ctx, path) + if err != nil { + return nil, err + } + c.entries[path] = entries + return entries, nil +} + +// findEntry does a case-insensitive lookup for name in the directory listing +// for dirPath. On Linux, prefers an exact case match. Returns the actual +// entry name, or empty string if not found. +func (c *dirCache) findEntry(ctx context.Context, dirPath, name string) (string, error) { + entries, err := c.list(ctx, dirPath) + if err != nil { + return "", err + } + + if runtime.GOOS == "linux" { + for _, e := range entries { + if e.Name() == name { + return e.Name(), nil + } + } + } + + for _, e := range entries { + if strings.EqualFold(e.Name(), name) { + return e.Name(), nil + } + } + + return "", nil +} diff --git a/pkg/database/mediascanner/dircache_test.go b/pkg/database/mediascanner/dircache_test.go new file mode 100644 index 00000000..791fc53d --- /dev/null +++ b/pkg/database/mediascanner/dircache_test.go @@ -0,0 +1,123 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package mediascanner + +import ( + "context" + "os" + "path/filepath" + "runtime" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestDirCache_CachesResults(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "file.txt"), []byte("x"), 0o600)) + + cache := newDirCache() + ctx := context.Background() + + entries1, err := cache.list(ctx, dir) + require.NoError(t, err) + require.Len(t, entries1, 1) + + // Add another file — cached result should still show 1 entry + require.NoError(t, os.WriteFile(filepath.Join(dir, "file2.txt"), []byte("y"), 0o600)) + + entries2, err := cache.list(ctx, dir) + require.NoError(t, err) + assert.Len(t, entries2, 1, "should return cached result, not re-read directory") +} + +func TestDirCache_FindEntry_ExactMatch(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "NES"), 0o750)) + + cache := newDirCache() + ctx := context.Background() + + name, err := cache.findEntry(ctx, dir, "NES") + require.NoError(t, err) + assert.Equal(t, "NES", name) +} + +func TestDirCache_FindEntry_CaseInsensitive(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "SNES"), 0o750)) + + cache := newDirCache() + ctx := context.Background() + + name, err := cache.findEntry(ctx, dir, "snes") + require.NoError(t, err) + assert.Equal(t, "SNES", name) +} + +func TestDirCache_FindEntry_PrefersExactOnLinux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skip("exact-match preference only applies on Linux") + } + + dir := t.TempDir() + // Create both cases — Linux allows this on case-sensitive filesystems + require.NoError(t, os.Mkdir(filepath.Join(dir, "Games"), 0o750)) + require.NoError(t, os.Mkdir(filepath.Join(dir, "games"), 0o750)) + + cache := newDirCache() + ctx := context.Background() + + name, err := cache.findEntry(ctx, dir, "Games") + require.NoError(t, err) + assert.Equal(t, "Games", name) + + name, err = cache.findEntry(ctx, dir, "games") + require.NoError(t, err) + assert.Equal(t, "games", name) +} + +func TestDirCache_FindEntry_NotFound(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.Mkdir(filepath.Join(dir, "NES"), 0o750)) + + cache := newDirCache() + ctx := context.Background() + + name, err := cache.findEntry(ctx, dir, "GENESIS") + require.NoError(t, err) + assert.Empty(t, name) +} + +func TestDirCache_ContextCancellation(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + cache := newDirCache() + + _, err := cache.list(ctx, t.TempDir()) + require.Error(t, err) + require.ErrorIs(t, err, context.Canceled) + + _, err = cache.findEntry(ctx, t.TempDir(), "anything") + require.Error(t, err) + require.ErrorIs(t, err, context.Canceled) +} diff --git a/pkg/database/mediascanner/findpath_fuzz_test.go b/pkg/database/mediascanner/findpath_fuzz_test.go index 0f8b93b3..59767287 100644 --- a/pkg/database/mediascanner/findpath_fuzz_test.go +++ b/pkg/database/mediascanner/findpath_fuzz_test.go @@ -20,6 +20,7 @@ package mediascanner import ( + "context" "os" "path/filepath" "testing" @@ -63,7 +64,8 @@ func FuzzFindPath(f *testing.F) { fullPath := filepath.Join(tmpDir, pathFragment) // EXECUTE: Call FindPath - result, err := FindPath(fullPath) + ctx := context.Background() + result, err := FindPath(ctx, fullPath) // INVARIANT CHECKS if err == nil { @@ -78,7 +80,7 @@ func FuzzFindPath(f *testing.F) { } // INVARIANT 4: Idempotency - calling FindPath on result should return same result - result2, err2 := FindPath(result) + result2, err2 := FindPath(ctx, result) if err2 != nil { t.Errorf("FindPath failed on its own output: %s (error: %v)", result, err2) } else if result != result2 { diff --git a/pkg/database/mediascanner/findpath_test.go b/pkg/database/mediascanner/findpath_test.go index 811189a8..e61ca134 100644 --- a/pkg/database/mediascanner/findpath_test.go +++ b/pkg/database/mediascanner/findpath_test.go @@ -20,6 +20,7 @@ package mediascanner import ( + "context" "os" "path/filepath" "runtime" @@ -30,6 +31,8 @@ import ( "github.com/stretchr/testify/require" ) +var bgCtx = context.Background() + // TestFindPath_BasicFunctionality tests that FindPath works for simple cases func TestFindPath_BasicFunctionality(t *testing.T) { t.Parallel() @@ -45,18 +48,18 @@ func TestFindPath_BasicFunctionality(t *testing.T) { require.NoError(t, os.WriteFile(testFile, []byte("test"), 0o600)) // Test finding existing directory - result, err := FindPath(testDir) + result, err := FindPath(bgCtx, testDir) require.NoError(t, err) assert.Equal(t, testDir, result) // Test finding existing file - result, err = FindPath(testFile) + result, err = FindPath(bgCtx, testFile) require.NoError(t, err) assert.Equal(t, testFile, result) // Test non-existent path nonExistent := filepath.Join(tmpDir, "DoesNotExist") - _, err = FindPath(nonExistent) + _, err = FindPath(bgCtx, nonExistent) assert.Error(t, err) } @@ -131,7 +134,7 @@ func TestFindPath_CaseNormalization(t *testing.T) { t.Skip("Skipping on Windows") } - result, err := FindPath(tt.inputPath) + result, err := FindPath(bgCtx, tt.inputPath) require.NoError(t, err, "FindPath should succeed for: %s", tt.description) // The critical assertion: result should match filesystem case, not input case @@ -171,7 +174,7 @@ func TestFindPath_RegressionWindowsCaseBug(t *testing.T) { // Input path with wrong case (lowercase 'b') wrongCasePath := filepath.Join(tmpDir, "Retrobat", "roms", "megadrive", "3 Ninjas Kick Back (USA).md") - result, err := FindPath(wrongCasePath) + result, err := FindPath(bgCtx, wrongCasePath) require.NoError(t, err, "FindPath must succeed even with wrong case input") // CRITICAL: Result must have correct case, not input case @@ -209,7 +212,7 @@ func TestFindPath_ErrorCases(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - _, err := FindPath(tt.inputPath) + _, err := FindPath(bgCtx, tt.inputPath) if tt.shouldError { assert.Error(t, err, tt.description) } else { @@ -230,7 +233,7 @@ func TestFindPath_WindowsVolumes(t *testing.T) { // Test that volume names are preserved // We can't easily create fake volumes, so we test with C: which should exist cDrive := "C:\\" - result, err := FindPath(cDrive) + result, err := FindPath(bgCtx, cDrive) // This might fail if C: doesn't exist (unlikely but possible) if err != nil { t.Skip("C:\\ not accessible, skipping volume test") @@ -257,7 +260,7 @@ func TestFindPath_Performance(t *testing.T) { } // FindPath should complete in reasonable time even for deep paths - result, err := FindPath(deepPath) + result, err := FindPath(bgCtx, deepPath) require.NoError(t, err) assert.Equal(t, deepPath, result) } @@ -281,7 +284,7 @@ func TestFindPath_SymlinkHandling(t *testing.T) { require.NoError(t, os.Symlink(target, link)) // FindPath should resolve the symlink - result, err := FindPath(link) + result, err := FindPath(bgCtx, link) require.NoError(t, err) assert.NotEmpty(t, result) } @@ -305,12 +308,12 @@ func TestFindPath_LinuxCaseAmbiguity(t *testing.T) { require.NoError(t, os.WriteFile(lowerFile, []byte("lower"), 0o600)) // When we ask for FILE.txt, we should get FILE.txt (exact match), not file.txt - result, err := FindPath(upperFile) + result, err := FindPath(bgCtx, upperFile) require.NoError(t, err) assert.Equal(t, upperFile, result, "Should prefer exact match on Linux") // When we ask for file.txt, we should get file.txt (exact match), not FILE.txt - result, err = FindPath(lowerFile) + result, err = FindPath(bgCtx, lowerFile) require.NoError(t, err) assert.Equal(t, lowerFile, result, "Should prefer exact match on Linux") @@ -341,7 +344,7 @@ func TestFindPath_ShortFilenames(t *testing.T) { } // FindPath should handle short names via the os.Stat fallback - result, err := FindPath(shortPath) + result, err := FindPath(bgCtx, shortPath) // We accept either success with short name, or success with long name if err != nil { t.Logf("FindPath failed on short name: %v", err) @@ -391,7 +394,7 @@ func TestFindPath_DirtyPaths(t *testing.T) { t.Run(tt.name, func(t *testing.T) { dirtyPath := tt.buildPath(tmpDir) - result, err := FindPath(dirtyPath) + result, err := FindPath(bgCtx, dirtyPath) require.NoError(t, err, tt.description) assert.Equal(t, targetFile, result, tt.description) }) @@ -415,7 +418,7 @@ func TestFindPath_Unicode(t *testing.T) { require.NoError(t, os.WriteFile(unicodeFile, []byte("test"), 0o600)) // Test finding the file - result, err := FindPath(unicodeFile) + result, err := FindPath(bgCtx, unicodeFile) require.NoError(t, err, "Should handle Unicode characters in path") assert.Equal(t, unicodeFile, result) } @@ -433,7 +436,7 @@ func TestFindPath_DotFiles(t *testing.T) { require.NoError(t, os.WriteFile(dotFile, []byte("test"), 0o600)) // FindPath should handle dot files/directories - result, err := FindPath(dotFile) + result, err := FindPath(bgCtx, dotFile) require.NoError(t, err, "Should handle dot files") assert.Equal(t, dotFile, result) } @@ -451,7 +454,7 @@ func TestFindPath_SpecialCharacters(t *testing.T) { require.NoError(t, os.WriteFile(specialFile, []byte("test"), 0o600)) // FindPath should handle special characters - result, err := FindPath(specialFile) + result, err := FindPath(bgCtx, specialFile) require.NoError(t, err, "Should handle special characters in path") assert.Equal(t, specialFile, result) } @@ -477,7 +480,7 @@ func TestFindPath_VeryDeepStructure(t *testing.T) { file := filepath.Join(current, "test.txt") require.NoError(t, os.WriteFile(file, []byte("data"), 0o600)) - result, err := FindPath(file) + result, err := FindPath(bgCtx, file) require.NoError(t, err, "Should handle very long paths") assert.Equal(t, file, result) @@ -495,7 +498,7 @@ func TestFindPath_RootPaths(t *testing.T) { t.Skip("Skipping Unix root test on Windows") } - result, err := FindPath("/") + result, err := FindPath(bgCtx, "/") if err != nil { t.Logf("Root path not accessible: %v", err) return @@ -508,7 +511,7 @@ func TestFindPath_RootPaths(t *testing.T) { // Test current directory t.Run("Current directory", func(t *testing.T) { - result, err := FindPath(".") + result, err := FindPath(bgCtx, ".") if err != nil { t.Logf("Current directory not accessible: %v", err) return diff --git a/pkg/database/mediascanner/fsutil.go b/pkg/database/mediascanner/fsutil.go new file mode 100644 index 00000000..1a977660 --- /dev/null +++ b/pkg/database/mediascanner/fsutil.go @@ -0,0 +1,113 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package mediascanner + +import ( + "context" + "errors" + "fmt" + "os" + "path/filepath" + "time" + + "github.com/rs/zerolog/log" +) + +// ErrFsTimeout is returned when a filesystem operation exceeds the timeout. +var ErrFsTimeout = errors.New("filesystem operation timed out") + +// defaultFsTimeout is the maximum time to wait for a single filesystem +// operation before giving up. This prevents indefinite hangs on stale +// network mounts (CIFS, NFS with hard mounts). +// +// Limitation: when a timeout or context cancellation fires, the underlying +// goroutine performing the syscall (os.Stat, os.ReadDir, etc.) cannot be +// interrupted and will remain blocked until the kernel returns. On a stale +// NFS/CIFS mount this may take minutes. The number of leaked goroutines is +// bounded by the number of distinct paths checked during a scan (typically +// <50 root/system folders), after which context cancellation prevents +// further operations from being started. +const defaultFsTimeout = 10 * time.Second + +// doWithTimeout runs fn in a goroutine and waits for it to complete, the +// context to be cancelled, or the defaultFsTimeout to elapse — whichever +// comes first. The op and path parameters are used for logging and error +// messages when the timeout fires. +func doWithTimeout[T any](ctx context.Context, fn func() (T, error), op, path string) (T, error) { + type result struct { + val T + err error + } + ch := make(chan result, 1) + go func() { + val, err := fn() + ch <- result{val, err} + }() + + timeoutCtx, cancel := context.WithTimeout(ctx, defaultFsTimeout) + defer cancel() + + select { + case r := <-ch: + return r.val, r.err + case <-timeoutCtx.Done(): + var zero T + if ctx.Err() != nil { + return zero, ctx.Err() + } + log.Warn().Str("path", path).Dur("timeout", defaultFsTimeout). + Msgf("filesystem %s timed out (possible stale mount)", op) + return zero, fmt.Errorf("%w: %s %s", ErrFsTimeout, op, path) + } +} + +// statWithContext runs os.Stat with a timeout. If the context is cancelled +// or the timeout elapses, it returns immediately. The underlying goroutine +// may continue running if the kernel call is blocked (e.g., on a stale +// mount), but will eventually unblock when the mount times out or recovers. +func statWithContext(ctx context.Context, path string) (os.FileInfo, error) { + return doWithTimeout(ctx, func() (os.FileInfo, error) { + return os.Stat(path) + }, "stat", path) +} + +// readDirWithContext runs os.ReadDir with a timeout. Same timeout and +// cancellation semantics as statWithContext. +func readDirWithContext(ctx context.Context, path string) ([]os.DirEntry, error) { + return doWithTimeout(ctx, func() ([]os.DirEntry, error) { + return os.ReadDir(path) + }, "readdir", path) +} + +// lstatWithContext runs os.Lstat with a timeout. Same timeout and +// cancellation semantics as statWithContext. +func lstatWithContext(ctx context.Context, path string) (os.FileInfo, error) { + return doWithTimeout(ctx, func() (os.FileInfo, error) { + return os.Lstat(path) + }, "lstat", path) +} + +// evalSymlinksWithContext runs filepath.EvalSymlinks with a timeout. Same +// timeout and cancellation semantics as statWithContext. +func evalSymlinksWithContext(ctx context.Context, path string) (string, error) { + return doWithTimeout(ctx, func() (string, error) { + return filepath.EvalSymlinks(path) + }, "evalsymlinks", path) +} diff --git a/pkg/database/mediascanner/fsutil_test.go b/pkg/database/mediascanner/fsutil_test.go new file mode 100644 index 00000000..12077b97 --- /dev/null +++ b/pkg/database/mediascanner/fsutil_test.go @@ -0,0 +1,157 @@ +// Zaparoo Core +// Copyright (c) 2026 The Zaparoo Project Contributors. +// SPDX-License-Identifier: GPL-3.0-or-later +// +// This file is part of Zaparoo Core. +// +// Zaparoo Core is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// Zaparoo Core is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with Zaparoo Core. If not, see . + +package mediascanner + +import ( + "context" + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestStatWithContext_Success(t *testing.T) { + dir := t.TempDir() + info, err := statWithContext(context.Background(), dir) + require.NoError(t, err) + assert.True(t, info.IsDir()) +} + +func TestStatWithContext_File(t *testing.T) { + dir := t.TempDir() + f := filepath.Join(dir, "test.txt") + require.NoError(t, os.WriteFile(f, []byte("hello"), 0o600)) + + info, err := statWithContext(context.Background(), f) + require.NoError(t, err) + assert.False(t, info.IsDir()) + assert.Equal(t, "test.txt", info.Name()) +} + +func TestStatWithContext_NotExist(t *testing.T) { + _, err := statWithContext(context.Background(), "/nonexistent/path/should/not/exist") + require.Error(t, err) + assert.True(t, os.IsNotExist(err)) +} + +func TestStatWithContext_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := statWithContext(ctx, t.TempDir()) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) +} + +func TestReadDirWithContext_Success(t *testing.T) { + dir := t.TempDir() + require.NoError(t, os.WriteFile(filepath.Join(dir, "a.txt"), []byte("a"), 0o600)) + require.NoError(t, os.WriteFile(filepath.Join(dir, "b.txt"), []byte("b"), 0o600)) + + entries, err := readDirWithContext(context.Background(), dir) + require.NoError(t, err) + assert.Len(t, entries, 2) + + names := make([]string, len(entries)) + for i, e := range entries { + names[i] = e.Name() + } + assert.Contains(t, names, "a.txt") + assert.Contains(t, names, "b.txt") +} + +func TestReadDirWithContext_EmptyDir(t *testing.T) { + dir := t.TempDir() + entries, err := readDirWithContext(context.Background(), dir) + require.NoError(t, err) + assert.Empty(t, entries) +} + +func TestReadDirWithContext_NotExist(t *testing.T) { + _, err := readDirWithContext(context.Background(), "/nonexistent/path/should/not/exist") + require.Error(t, err) +} + +func TestReadDirWithContext_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := readDirWithContext(ctx, t.TempDir()) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) +} + +func TestLstatWithContext_Success(t *testing.T) { + dir := t.TempDir() + info, err := lstatWithContext(context.Background(), dir) + require.NoError(t, err) + assert.True(t, info.IsDir()) +} + +func TestLstatWithContext_Symlink(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + require.NoError(t, os.Mkdir(target, 0o750)) + link := filepath.Join(dir, "link") + require.NoError(t, os.Symlink(target, link)) + + info, err := lstatWithContext(context.Background(), link) + require.NoError(t, err) + assert.NotEqual(t, 0, info.Mode()&os.ModeSymlink, "should report symlink mode") +} + +func TestLstatWithContext_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := lstatWithContext(ctx, t.TempDir()) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) +} + +func TestEvalSymlinksWithContext_Success(t *testing.T) { + dir := t.TempDir() + target := filepath.Join(dir, "target") + require.NoError(t, os.Mkdir(target, 0o750)) + link := filepath.Join(dir, "link") + require.NoError(t, os.Symlink(target, link)) + + resolved, err := evalSymlinksWithContext(context.Background(), link) + require.NoError(t, err) + assert.Equal(t, target, resolved) +} + +func TestEvalSymlinksWithContext_NoSymlink(t *testing.T) { + dir := t.TempDir() + resolved, err := evalSymlinksWithContext(context.Background(), dir) + require.NoError(t, err) + assert.Equal(t, dir, resolved) +} + +func TestEvalSymlinksWithContext_ContextCancelled(t *testing.T) { + ctx, cancel := context.WithCancel(context.Background()) + cancel() + + _, err := evalSymlinksWithContext(ctx, t.TempDir()) + require.Error(t, err) + assert.ErrorIs(t, err, context.Canceled) +} diff --git a/pkg/database/mediascanner/mediascanner.go b/pkg/database/mediascanner/mediascanner.go index f1f1dd99..1722de85 100644 --- a/pkg/database/mediascanner/mediascanner.go +++ b/pkg/database/mediascanner/mediascanner.go @@ -38,6 +38,7 @@ import ( "github.com/ZaparooProject/zaparoo-core/v2/pkg/database/systemdefs" "github.com/ZaparooProject/zaparoo-core/v2/pkg/helpers" "github.com/ZaparooProject/zaparoo-core/v2/pkg/platforms" + sqlite3 "github.com/mattn/go-sqlite3" "github.com/rs/zerolog/log" ) @@ -92,9 +93,9 @@ type PathResult struct { // - Linux: Prefers exact match before case-insensitive match (handles File.txt vs file.txt) // - Windows: Handles 8.3 short names (PROGRA~1) via fallback to os.Stat // - All platforms: Works with symlinks, UNC paths, network drives -func FindPath(path string) (string, error) { +func FindPath(ctx context.Context, path string) (string, error) { // Check if path exists first - if _, err := os.Stat(path); err != nil { + if _, err := statWithContext(ctx, path); err != nil { return "", fmt.Errorf("path does not exist: %s", path) } @@ -134,11 +135,17 @@ func FindPath(path string) (string, error) { parts := strings.Split(relPath, string(filepath.Separator)) for _, part := range parts { + select { + case <-ctx.Done(): + return "", ctx.Err() + default: + } + if part == "" || part == "." { continue } - entries, err := os.ReadDir(currentPath) + entries, err := readDirWithContext(ctx, currentPath) if err != nil { return "", fmt.Errorf("failed to read directory %s: %w", currentPath, err) } @@ -173,7 +180,7 @@ func FindPath(path string) (string, error) { // it might be a short name or special filesystem entry if !found { targetCheck := filepath.Join(currentPath, part) - if _, err := os.Stat(targetCheck); err == nil { + if _, err := statWithContext(ctx, targetCheck); err == nil { // Path exists but wasn't found in directory listing // Accept the component as-is (likely 8.3 short name) currentPath = targetCheck @@ -190,40 +197,77 @@ func FindPath(path string) (string, error) { } func GetSystemPaths( + ctx context.Context, _ *config.Instance, _ platforms.Platform, rootFolders []string, systems []systemdefs.System, ) []PathResult { var matches []PathResult + cache := newDirCache() + + log.Info(). + Int("rootFolders", len(rootFolders)). + Int("systems", len(systems)). + Msg("starting path discovery") // Validate root folders ONCE before iterating systems // This prevents logging the same error 200+ times (once per system) validRootFolders := make([]string, 0, len(rootFolders)) for _, folder := range rootFolders { - gf, err := FindPath(folder) + gf, err := FindPath(ctx, folder) if err != nil { - log.Debug().Err(err).Str("path", folder).Msg("skipping root folder - not found or inaccessible") + switch { + case errors.Is(err, ErrFsTimeout): + log.Warn().Str("path", folder).Dur("timeout", defaultFsTimeout). + Msg("root folder timed out (possible stale mount)") + case ctx.Err() != nil: + log.Info().Msg("path discovery cancelled") + return matches + default: + log.Debug().Err(err).Str("path", folder).Msg("skipping root folder - not found or inaccessible") + } continue } validRootFolders = append(validRootFolders, gf) + + // Pre-cache the root folder listing + if _, cacheErr := cache.list(ctx, gf); cacheErr != nil { + if ctx.Err() != nil { + log.Info().Msg("path discovery cancelled") + return matches + } + log.Debug().Err(cacheErr).Str("path", gf).Msg("failed to cache root folder listing") + } } + log.Info(). + Int("validRoots", len(validRootFolders)). + Int("totalRoots", len(rootFolders)). + Msg("root folder validation complete") + for _, system := range systems { + select { + case <-ctx.Done(): + log.Info().Msg("path discovery cancelled") + return matches + default: + } + // GlobalLauncherCache is assumed to be read-only after initialization launchers := helpers.GlobalLauncherCache.GetLaunchersBySystem(system.ID) var folders []string - for i := range launchers { + for j := range launchers { // Skip filesystem scanning for launchers that don't need it - if launchers[i].SkipFilesystemScan { + if launchers[j].SkipFilesystemScan { log.Trace(). - Str("launcher", launchers[i].ID). + Str("launcher", launchers[j].ID). Str("system", system.ID). Msg("skipping filesystem scan for launcher") continue } - for _, folder := range launchers[i].Folders { + for _, folder := range launchers[j].Folders { if !helpers.Contains(folders, folder) { folders = append(folders, folder) } @@ -240,9 +284,31 @@ func GetSystemPaths( // check for / for _, gf := range validRootFolders { for _, folder := range folders { + // Use cached directory listing for single-component folder names + if !strings.Contains(folder, string(filepath.Separator)) && !filepath.IsAbs(folder) { + actualName, findErr := cache.findEntry(ctx, gf, folder) + if findErr != nil { + if ctx.Err() != nil { + return matches + } + continue + } + if actualName != "" { + matches = append(matches, PathResult{ + System: system, + Path: filepath.Join(gf, actualName), + }) + } + continue + } + + // Multi-component paths fall through to full FindPath systemFolder := filepath.Join(gf, folder) - path, err := FindPath(systemFolder) + path, err := FindPath(ctx, systemFolder) if err != nil { + if ctx.Err() != nil { + return matches + } log.Debug().Err(err).Str("path", systemFolder).Str("system", system.ID). Msg("skipping system folder - not found or inaccessible") continue @@ -259,8 +325,11 @@ func GetSystemPaths( for _, folder := range folders { if filepath.IsAbs(folder) { systemFolder := folder - path, err := FindPath(systemFolder) + path, err := FindPath(ctx, systemFolder) if err != nil { + if ctx.Err() != nil { + return matches + } log.Debug().Err(err).Str("path", systemFolder).Str("system", system.ID). Msg("skipping absolute path - not found or inaccessible") continue @@ -273,7 +342,24 @@ func GetSystemPaths( } } - return matches + // Deduplicate by (SystemID, resolved Path) to prevent UNIQUE constraint + // failures when multiple root folders resolve to the same directory + seen := make(map[string]bool) + deduplicated := make([]PathResult, 0, len(matches)) + for _, m := range matches { + key := m.System.ID + ":" + m.Path + if !seen[key] { + seen[key] = true + deduplicated = append(deduplicated, m) + } + } + + log.Info(). + Int("matches", len(deduplicated)). + Int("duplicatesRemoved", len(matches)-len(deduplicated)). + Msg("path discovery complete") + + return deduplicated } type resultsStack struct { @@ -351,7 +437,7 @@ func GetFiles( if file.IsDir() { key := path if file.Type()&os.ModeSymlink != 0 { - realPath, symlinkErr := filepath.EvalSymlinks(path) + realPath, symlinkErr := evalSymlinksWithContext(ctx, path) if symlinkErr != nil { return fmt.Errorf("failed to evaluate symlink %s: %w", path, symlinkErr) } @@ -364,7 +450,7 @@ func GetFiles( // Check for .zaparooignore marker file markerPath := filepath.Join(path, ".zaparooignore") - if _, statErr := os.Stat(markerPath); statErr == nil { + if _, statErr := statWithContext(ctx, markerPath); statErr == nil { log.Info().Str("path", path).Msg("skipping directory with .zaparooignore marker") return filepath.SkipDir } @@ -378,13 +464,13 @@ func GetFiles( return fmt.Errorf("failed to get absolute path for %s: %w", path, absErr) } - realPath, realPathErr := filepath.EvalSymlinks(absSym) + realPath, realPathErr := evalSymlinksWithContext(ctx, absSym) if realPathErr != nil { return fmt.Errorf("failed to evaluate symlink %s: %w", absSym, realPathErr) } log.Trace().Str("symlink", path).Str("target", realPath).Msg("resolved symlink") - file, statErr := os.Stat(realPath) + file, statErr := statWithContext(ctx, realPath) if statErr != nil { return fmt.Errorf("failed to stat symlink target %s: %w", realPath, statErr) } @@ -443,7 +529,7 @@ func GetFiles( stack.push() defer stack.pop() - root, err := os.Lstat(path) + root, err := lstatWithContext(ctx, path) if err != nil { return nil, fmt.Errorf("failed to stat path %s: %w", path, err) } @@ -453,13 +539,13 @@ func GetFiles( if root.Mode()&os.ModeSymlink == 0 { realPath = path } else { - realPath, err = filepath.EvalSymlinks(path) + realPath, err = evalSymlinksWithContext(ctx, path) if err != nil { return nil, fmt.Errorf("failed to evaluate symlink %s: %w", path, err) } } - realRoot, err := os.Stat(realPath) + realRoot, err := statWithContext(ctx, realPath) if err != nil { return nil, fmt.Errorf("failed to stat real path %s: %w", realPath, err) } @@ -538,8 +624,14 @@ func handleCancellationWithRollback(ctx context.Context, db database.MediaDBI, m return 0, ctx.Err() } +const ( + PhaseDiscovering = "discovering" + PhaseInitializing = "initializing" +) + type IndexStatus struct { SystemID string + Phase string // PhaseDiscovering, PhaseInitializing, or empty during indexing Total int Step int Files int @@ -602,7 +694,8 @@ func NewNamesIndex( case "": log.Info().Msg("starting fresh indexing (no previous indexing status)") case mediadb.IndexingStatusRunning, mediadb.IndexingStatusPending: - log.Info().Msgf("found interrupted indexing with status: %s", indexingStatus) + log.Warn().Str("status", indexingStatus). + Msg("previous indexing was interrupted, attempting to resume") var getSystemErr error lastIndexedSystemID, getSystemErr = db.GetLastIndexedSystem() if getSystemErr != nil { @@ -635,12 +728,15 @@ func NewNamesIndex( } // Get the ordered list of systems for this run (deterministic by ID) + update(IndexStatus{Phase: PhaseDiscovering}) systemPaths := make(map[string][]string) - for _, v := range GetSystemPaths(cfg, platform, platform.RootDirs(cfg), systems) { + for _, v := range GetSystemPaths(ctx, cfg, platform, platform.RootDirs(cfg), systems) { systemPaths[v.System.ID] = append(systemPaths[v.System.ID], v.Path) } sysPathIDs := helpers.AlphaMapKeys(systemPaths) + update(IndexStatus{Phase: PhaseInitializing}) + // Check for cancellation select { case <-ctx.Done(): @@ -836,10 +932,13 @@ func NewNamesIndex( }() status := IndexStatus{ - Total: len(sysPathIDs) + 2, // Adjusted total steps for the current list - Step: 1, + Total: len(sysPathIDs) + 1, // +1 for final "Writing database" step + Step: 0, } + // Track UNIQUE constraint failures across all systems + var uniqueConstraintFailures int + // Track which launchers have already been scanned to prevent double-execution scannedLaunchers := make(map[string]bool) // This map tracks systems that have been fully processed and committed @@ -857,8 +956,6 @@ func NewNamesIndex( } } - update(status) - scannedSystems := make(map[string]bool) for _, s := range systemdefs.AllSystems() { scannedSystems[s.ID] = false @@ -1014,6 +1111,12 @@ func NewNamesIndex( _, _, addErr := AddMediaPath(db, &scanState, systemID, file.Path, file.NoExt, shouldStrip, cfg) if addErr != nil { + var sqliteErr sqlite3.Error + if errors.As(addErr, &sqliteErr) && sqliteErr.ExtendedCode == sqlite3.ErrConstraintUnique { + uniqueConstraintFailures++ + log.Debug().Err(addErr).Str("path", file.Path).Msg("skipping duplicate media entry") + continue + } return 0, fmt.Errorf("unrecoverable error adding media path %q: %w", file.Path, addErr) } filesInBatch++ @@ -1182,6 +1285,12 @@ func NewNamesIndex( // Custom scanner files: don't apply number stripping heuristic (false) _, _, addErr := AddMediaPath(db, &scanState, systemID, result.Path, result.NoExt, false, cfg) if addErr != nil { + var sqliteErr sqlite3.Error + if errors.As(addErr, &sqliteErr) && sqliteErr.ExtendedCode == sqlite3.ErrConstraintUnique { + uniqueConstraintFailures++ + log.Debug().Err(addErr).Str("path", result.Path).Msg("skipping duplicate media entry") + continue + } return 0, fmt.Errorf( "unrecoverable error adding custom scanner path %q: %w", result.Path, @@ -1287,6 +1396,12 @@ func NewNamesIndex( db, &scanState, systemID, scanResult.Path, scanResult.NoExt, false, cfg, ) if addErr != nil { + var sqliteErr sqlite3.Error + if errors.As(addErr, &sqliteErr) && sqliteErr.ExtendedCode == sqlite3.ErrConstraintUnique { + uniqueConstraintFailures++ + log.Debug().Err(addErr).Str("path", scanResult.Path).Msg("skipping duplicate media entry") + continue + } return 0, fmt.Errorf( "unrecoverable error adding 'any' scanner path %q: %w", scanResult.Path, @@ -1410,6 +1525,11 @@ func NewNamesIndex( } log.Debug().Msgf("indexed systems: %v", indexedSystems) + if uniqueConstraintFailures > 0 { + log.Warn().Int("count", uniqueConstraintFailures). + Msg("UNIQUE constraint failures during indexing (possible duplicate paths)") + } + indexedFiles = status.Files indexElapsed := time.Since(indexStartTime) diff --git a/pkg/database/mediascanner/mediascanner_test.go b/pkg/database/mediascanner/mediascanner_test.go index 1bf1e3be..53d3de9d 100644 --- a/pkg/database/mediascanner/mediascanner_test.go +++ b/pkg/database/mediascanner/mediascanner_test.go @@ -165,7 +165,7 @@ func TestGetSystemPathsRespectsSkipFilesystemScan(t *testing.T) { // Call GetSystemPaths - this will test the folder aggregation logic // Even with empty root folders, we can verify the function respects SkipFilesystemScan - results := GetSystemPaths(cfg, mockPlatform, []string{}, systems) + results := GetSystemPaths(context.Background(), cfg, mockPlatform, []string{}, systems) // Since GetSystemPaths tries to resolve actual paths and we have no real folders, // we expect empty results, but the important part is that the function @@ -1235,11 +1235,11 @@ func TestAnyScannerProgressUpdates(t *testing.T) { "Status update should have been called with the test system ID from 'any' scanner") // Verify that status.Total was dynamically expanded (should be > initial value) - // Initial total is len(sysPathIDs) + 2, but since there are no paths, it starts at 2 + // Initial total is len(sysPathIDs) + 1, but since there are no paths, it starts at 1 // After "any" scanner finds results, Total should be incremented if len(statusUpdates) > 0 { lastStatus := statusUpdates[len(statusUpdates)-1] - assert.GreaterOrEqual(t, lastStatus.Total, 3, + assert.GreaterOrEqual(t, lastStatus.Total, 2, "Total should be dynamically expanded when 'any' scanner finds results") } } @@ -1288,7 +1288,7 @@ func TestGetSystemPaths_CustomLauncherAbsolutePaths(t *testing.T) { // Call GetSystemPaths with no root folders (custom launchers use absolute paths) systems := []systemdefs.System{{ID: systemdefs.SystemPS2}} - results := GetSystemPaths(cfg, platform, []string{}, systems) + results := GetSystemPaths(context.Background(), cfg, platform, []string{}, systems) // The custom launcher's absolute folder should be resolved require.Len(t, results, 1, "Should find exactly one path for PS2 custom launcher") diff --git a/pkg/platforms/mister/platform.go b/pkg/platforms/mister/platform.go index 6d3cd35a..fb09e3ce 100644 --- a/pkg/platforms/mister/platform.go +++ b/pkg/platforms/mister/platform.go @@ -822,7 +822,7 @@ func (p *Platform) Launchers(cfg *config.Instance) []platforms.Launcher { return results, fmt.Errorf("failed to get Amiga system: %w", err) } - sfs := mediascanner.GetSystemPaths(cfg, p, p.RootDirs(cfg), []systemdefs.System{*s}) + sfs := mediascanner.GetSystemPaths(ctx, cfg, p, p.RootDirs(cfg), []systemdefs.System{*s}) log.Debug().Int("paths", len(sfs)).Msg("amigavision scan paths found") for _, sf := range sfs { @@ -833,7 +833,7 @@ func (p *Platform) Launchers(cfg *config.Instance) []platforms.Launcher { } for _, txt := range []string{aGamesPath, aDemosPath} { - tp, err := mediascanner.FindPath(filepath.Join(sf.Path, txt)) + tp, err := mediascanner.FindPath(ctx, filepath.Join(sf.Path, txt)) if err == nil { f, err := os.Open(tp) //nolint:gosec // Internal amiga games/demos path if err != nil { @@ -905,7 +905,7 @@ func (p *Platform) Launchers(cfg *config.Instance) []platforms.Launcher { return results, fmt.Errorf("failed to get NeoGeo system: %w", err) } - sfs := mediascanner.GetSystemPaths(cfg, p, p.RootDirs(cfg), []systemdefs.System{*s}) + sfs := mediascanner.GetSystemPaths(ctx, cfg, p, p.RootDirs(cfg), []systemdefs.System{*s}) log.Debug().Int("paths", len(sfs)).Msg("neogeo scan paths found") // Collect NEOGEO paths for filtering @@ -922,7 +922,7 @@ func (p *Platform) Launchers(cfg *config.Instance) []platforms.Launcher { default: } - rsf, err := mediascanner.FindPath(filepath.Join(sf.Path, romsetsFilename)) + rsf, err := mediascanner.FindPath(ctx, filepath.Join(sf.Path, romsetsFilename)) if err == nil { romsets, readErr := readRomsets(rsf) if readErr != nil { diff --git a/pkg/platforms/windows/retrobat.go b/pkg/platforms/windows/retrobat.go index edb6fde8..0ba04cc7 100644 --- a/pkg/platforms/windows/retrobat.go +++ b/pkg/platforms/windows/retrobat.go @@ -44,7 +44,7 @@ import ( func findRetroBatDir(cfg *config.Instance) (string, error) { // Check user-configured directory first if def := cfg.LookupLauncherDefaults("RetroBat", nil); def.InstallDir != "" { - if normalizedPath, err := mediascanner.FindPath(def.InstallDir); err == nil { + if normalizedPath, err := mediascanner.FindPath(context.Background(), def.InstallDir); err == nil { log.Debug().Msgf("using user-configured RetroBat directory: %s", normalizedPath) return normalizedPath, nil } @@ -67,7 +67,7 @@ func findRetroBatDir(cfg *config.Instance) (string, error) { retroBatExe := filepath.Join(path, "retrobat.exe") if _, err := os.Stat(retroBatExe); err == nil { // Use FindPath to get the actual filesystem case - if normalizedPath, err := mediascanner.FindPath(path); err == nil { + if normalizedPath, err := mediascanner.FindPath(context.Background(), path); err == nil { return normalizedPath, nil } // Fallback to original path if FindPath fails (shouldn't happen) From 49cb79522333f5d6eafb475d150a7076b34fbcea Mon Sep 17 00:00:00 2001 From: Callan Barrett Date: Wed, 4 Mar 2026 08:58:25 +0800 Subject: [PATCH 2/2] fix(mediascanner): resolve symlinks in fsutil test expectations EvalSymlinks resolves all symlinks in the path including those in the temp directory itself (e.g. /var -> /private/var on macOS, 8.3 short names on Windows). Compare against the resolved expected path instead of the raw t.TempDir() path. --- pkg/database/mediascanner/fsutil_test.go | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/pkg/database/mediascanner/fsutil_test.go b/pkg/database/mediascanner/fsutil_test.go index 12077b97..ddbbe55f 100644 --- a/pkg/database/mediascanner/fsutil_test.go +++ b/pkg/database/mediascanner/fsutil_test.go @@ -137,14 +137,22 @@ func TestEvalSymlinksWithContext_Success(t *testing.T) { resolved, err := evalSymlinksWithContext(context.Background(), link) require.NoError(t, err) - assert.Equal(t, target, resolved) + // EvalSymlinks resolves all symlinks in the path, including those in + // the temp dir itself (e.g. /var -> /private/var on macOS). + expected, err := filepath.EvalSymlinks(target) + require.NoError(t, err) + assert.Equal(t, expected, resolved) } func TestEvalSymlinksWithContext_NoSymlink(t *testing.T) { dir := t.TempDir() resolved, err := evalSymlinksWithContext(context.Background(), dir) require.NoError(t, err) - assert.Equal(t, dir, resolved) + // EvalSymlinks resolves all symlinks in the path, including those in + // the temp dir itself (e.g. /var -> /private/var on macOS). + expected, err := filepath.EvalSymlinks(dir) + require.NoError(t, err) + assert.Equal(t, expected, resolved) } func TestEvalSymlinksWithContext_ContextCancelled(t *testing.T) {