From ca9b48260e7ba838d93f7992bb8c0e520339de45 Mon Sep 17 00:00:00 2001 From: Justin Kirkegaard Date: Tue, 22 Jul 2025 22:38:25 -0500 Subject: [PATCH 1/4] Update embedfs implementation: - Fix path normalization to support billy's absolute path convention ("/") - Implement missing Lstat method (was returning ErrNotSupported) - Add proper embed.FS to billy path conversion throughout all methods Additional test coverage: - Add dedicated test files for different functionality areas - Test File interface methods including ReadAt, Close, Lock/Unlock - Add path normalization and edge case testing - Test empty file handling and boundary conditions - Verify proper error handling for read-only operations Test infrastructure: - Create embedfs_testdata package for reusable test data - Resolve import cycles with clean provider pattern - Add test data including nested directories and various file types - Organize tests by functionality (fs, dir, file operations) The embedfs implementation now provides billy.Filesystem compliance for read-only embedded filesystems. --- embedfs/debug_test.go | 30 ++ embedfs/dir_test.go | 63 ++++ embedfs/embed.go | 26 +- embedfs/embed_test.go | 275 ++++++++++++++---- embedfs/fs_test.go | 109 +++++++ embedfs/testdata/empty2.txt | 1 - embedfs_testdata/provider.go | 16 + .../testdata/empty.txt | 0 embedfs_testdata/testdata/file1.txt | 1 + embedfs_testdata/testdata/file2.txt | 1 + embedfs_testdata/testdata/subdir/nested.txt | 1 + 11 files changed, 462 insertions(+), 61 deletions(-) create mode 100644 embedfs/debug_test.go create mode 100644 embedfs/dir_test.go create mode 100644 embedfs/fs_test.go delete mode 100644 embedfs/testdata/empty2.txt create mode 100644 embedfs_testdata/provider.go rename {embedfs => embedfs_testdata}/testdata/empty.txt (100%) create mode 100644 embedfs_testdata/testdata/file1.txt create mode 100644 embedfs_testdata/testdata/file2.txt create mode 100644 embedfs_testdata/testdata/subdir/nested.txt diff --git a/embedfs/debug_test.go b/embedfs/debug_test.go new file mode 100644 index 0000000..2bc01b7 --- /dev/null +++ b/embedfs/debug_test.go @@ -0,0 +1,30 @@ +package embedfs + +import ( + "embed" + "testing" + + "github.com/go-git/go-billy/v6/embedfs_testdata" +) + +var emptyEmbedFS embed.FS + +func TestDebugErrors(t *testing.T) { + // Test 1: Empty embed.FS with empty path + emptyFS := New(&emptyEmbedFS) + _, err1 := emptyFS.ReadDir("") + t.Logf("Empty FS, empty path: %v", err1) + + // Test 2: Empty embed.FS with root path + _, err2 := emptyFS.ReadDir("/") + t.Logf("Empty FS, root path: %v", err2) + + // Test 3: Non-empty embed.FS with empty path + richFS := New(embedfs_testdata.GetTestData()) + _, err3 := richFS.ReadDir("") + t.Logf("Rich FS, empty path: %v", err3) + + // Test 4: Non-empty embed.FS with root path + entries, err4 := richFS.ReadDir("/") + t.Logf("Rich FS, root path: %d entries, err: %v", len(entries), err4) +} \ No newline at end of file diff --git a/embedfs/dir_test.go b/embedfs/dir_test.go new file mode 100644 index 0000000..b12ba54 --- /dev/null +++ b/embedfs/dir_test.go @@ -0,0 +1,63 @@ +package embedfs + +import ( + "os" + "testing" + + "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// Directory operation tests adapted from test/dir_test.go for read-only embedfs + +func TestDir_ReadDirNested(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test reading nested directories + entries, err := fs.ReadDir("/testdata/subdir") + require.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, "nested.txt", entries[0].Name()) + assert.False(t, entries[0].IsDir()) +} + +func TestDir_ReadDirFileInfo(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + entries, err := fs.ReadDir("/testdata") + require.NoError(t, err) + + // Verify all entries have proper FileInfo + for _, entry := range entries { + assert.NotEmpty(t, entry.Name()) + assert.NotNil(t, entry.ModTime()) + assert.Greater(t, entry.Size(), int64(-1)) // Size can be 0 but not negative + } +} + +func TestDir_ReadDirFileInfoDirs(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + entries, err := fs.ReadDir("/testdata") + require.NoError(t, err) + + // Find the subdirectory entry + var subdirEntry os.FileInfo + for _, entry := range entries { + if entry.Name() == "subdir" { + subdirEntry = entry + break + } + } + + require.NotNil(t, subdirEntry, "subdir should be found") + assert.True(t, subdirEntry.IsDir(), "subdir should be a directory") + assert.Equal(t, "subdir", subdirEntry.Name()) +} \ No newline at end of file diff --git a/embedfs/embed.go b/embedfs/embed.go index d0b7c8c..0e69f24 100644 --- a/embedfs/embed.go +++ b/embedfs/embed.go @@ -32,11 +32,26 @@ func New(efs *embed.FS) billy.Filesystem { return fs } +// normalizePath converts billy's absolute paths to embed.FS relative paths +func (fs *Embed) normalizePath(path string) string { + // embed.FS uses "." for root directory, but billy uses "/" + if path == "/" { + return "." + } + // Remove leading slash for embed.FS + if strings.HasPrefix(path, "/") { + return path[1:] + } + return path +} + func (fs *Embed) Root() string { return "" } func (fs *Embed) Stat(filename string) (os.FileInfo, error) { + filename = fs.normalizePath(filename) + f, err := fs.underlying.Open(filename) if err != nil { return nil, err @@ -53,6 +68,7 @@ func (fs *Embed) OpenFile(filename string, flag int, _ os.FileMode) (billy.File, return nil, billy.ErrReadOnly } + filename = fs.normalizePath(filename) f, err := fs.underlying.Open(filename) if err != nil { return nil, err @@ -91,6 +107,8 @@ func (fs *Embed) Join(elem ...string) string { } func (fs *Embed) ReadDir(path string) ([]os.FileInfo, error) { + path = fs.normalizePath(path) + e, err := fs.underlying.ReadDir(path) if err != nil { return nil, err @@ -114,11 +132,9 @@ func (fs *Embed) Chroot(_ string) (billy.Filesystem, error) { return nil, billy.ErrNotSupported } -// Lstat is not supported. -// -// Calls will always return billy.ErrNotSupported. -func (fs *Embed) Lstat(_ string) (os.FileInfo, error) { - return nil, billy.ErrNotSupported +// Lstat behaves the same as Stat for embedded filesystems since there are no symlinks. +func (fs *Embed) Lstat(filename string) (os.FileInfo, error) { + return fs.Stat(filename) } // Readlink is not supported. diff --git a/embedfs/embed_test.go b/embedfs/embed_test.go index 2501afe..61dad08 100644 --- a/embedfs/embed_test.go +++ b/embedfs/embed_test.go @@ -8,17 +8,11 @@ import ( "testing" "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/embedfs_testdata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) -//go:embed testdata/empty.txt -var singleFile embed.FS - -//go:embed testdata -var testdataDir embed.FS - -var empty embed.FS func TestOpen(t *testing.T) { t.Parallel() @@ -29,12 +23,12 @@ func TestOpen(t *testing.T) { wantErr bool }{ { - name: "testdata/empty.txt", - want: []byte(""), + name: "testdata/file1.txt", + want: []byte("Hello from embedfs!"), }, { - name: "testdata/empty2.txt", - want: []byte("test"), + name: "testdata/file2.txt", + want: []byte("Another test file"), }, { name: "non-existent", @@ -44,7 +38,7 @@ func TestOpen(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&testdataDir) + fs := New(embedfs_testdata.GetTestData()) var got []byte f, err := fs.Open(tc.name) @@ -74,48 +68,48 @@ func TestOpenFileFlags(t *testing.T) { }{ { name: "O_CREATE", - file: "testdata/empty.txt", + file: "testdata/file1.txt", flag: os.O_CREATE, wantErr: "read-only filesystem", }, { name: "O_WRONLY", - file: "testdata/empty.txt", + file: "testdata/file1.txt", flag: os.O_WRONLY, wantErr: "read-only filesystem", }, { name: "O_TRUNC", - file: "testdata/empty.txt", + file: "testdata/file1.txt", flag: os.O_TRUNC, wantErr: "read-only filesystem", }, { name: "O_RDWR", - file: "testdata/empty.txt", + file: "testdata/file1.txt", flag: os.O_RDWR, wantErr: "read-only filesystem", }, { name: "O_EXCL", - file: "testdata/empty.txt", + file: "testdata/file1.txt", flag: os.O_EXCL, wantErr: "read-only filesystem", }, { name: "O_RDONLY", - file: "testdata/empty.txt", + file: "testdata/file1.txt", flag: os.O_RDONLY, }, { name: "no flags", - file: "testdata/empty.txt", + file: "testdata/file1.txt", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&testdataDir) + fs := New(embedfs_testdata.GetTestData()) _, err := fs.OpenFile(tc.file, tc.flag, 0o700) if tc.wantErr != "" { @@ -137,12 +131,12 @@ func TestStat(t *testing.T) { wantErr bool }{ { - name: "testdata/empty.txt", - want: "empty.txt", + name: "testdata/file1.txt", + want: "file1.txt", }, { - name: "testdata/empty2.txt", - want: "empty2.txt", + name: "testdata/file2.txt", + want: "file2.txt", }, { name: "non-existent", @@ -157,7 +151,7 @@ func TestStat(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&testdataDir) + fs := New(embedfs_testdata.GetTestData()) fi, err := fs.Stat(tc.name) if tc.wantErr { @@ -184,30 +178,23 @@ func TestReadDir(t *testing.T) { wantErr bool }{ { - name: "singleFile", + name: "testdata", path: "testdata", - fs: &singleFile, - want: []string{"empty.txt"}, + fs: embedfs_testdata.GetTestData(), + want: []string{"empty.txt", "file1.txt", "file2.txt", "subdir"}, }, { - name: "empty", + name: "empty path", path: "", - fs: &empty, + fs: embedfs_testdata.GetTestData(), want: []string{}, wantErr: true, }, { - name: "testdataDir w/ path", - path: "testdata", - fs: &testdataDir, - want: []string{"empty.txt", "empty2.txt"}, - }, - { - name: "testdataDir return no dir names", - path: "", - fs: &testdataDir, - want: []string{}, - wantErr: true, + name: "root path", + path: "/", + fs: embedfs_testdata.GetTestData(), + want: []string{"testdata"}, }, } @@ -241,7 +228,7 @@ func TestReadDir(t *testing.T) { func TestUnsupported(t *testing.T) { t.Parallel() - fs := New(&testdataDir) + fs := New(embedfs_testdata.GetTestData()) _, err := fs.Create("test") require.ErrorIs(t, err, billy.ErrReadOnly) @@ -259,9 +246,9 @@ func TestUnsupported(t *testing.T) { func TestFileUnsupported(t *testing.T) { t.Parallel() - fs := New(&testdataDir) + fs := New(embedfs_testdata.GetTestData()) - f, err := fs.Open("testdata/empty.txt") + f, err := fs.Open("testdata/file1.txt") require.NoError(t, err) assert.NotNil(t, f) @@ -273,9 +260,9 @@ func TestFileUnsupported(t *testing.T) { } func TestFileSeek(t *testing.T) { - fs := New(&testdataDir) + fs := New(embedfs_testdata.GetTestData()) - f, err := fs.Open("testdata/empty2.txt") + f, err := fs.Open("testdata/file2.txt") require.NoError(t, err) assert.NotNil(t, f) @@ -284,14 +271,14 @@ func TestFileSeek(t *testing.T) { seekWhence int want string }{ - {seekOff: 3, seekWhence: io.SeekStart, want: ""}, - {seekOff: 3, seekWhence: io.SeekStart, want: "t"}, - {seekOff: 2, seekWhence: io.SeekStart, want: "st"}, - {seekOff: 1, seekWhence: io.SeekStart, want: "est"}, - {seekOff: 0, seekWhence: io.SeekStart, want: "test"}, - {seekOff: 0, seekWhence: io.SeekStart, want: "t"}, - {seekOff: 1, seekWhence: io.SeekCurrent, want: "s"}, - {seekOff: -2, seekWhence: io.SeekEnd, want: "st"}, + {seekOff: 8, seekWhence: io.SeekStart, want: "test file"}, // pos now at 17 + {seekOff: 8, seekWhence: io.SeekStart, want: "t"}, // pos now at 9 + {seekOff: 9, seekWhence: io.SeekStart, want: "est"}, // pos now at 12 + {seekOff: 1, seekWhence: io.SeekStart, want: "nother test file"}, // pos now at 17 + {seekOff: 0, seekWhence: io.SeekStart, want: "Another test file"}, // pos now at 17 + {seekOff: 0, seekWhence: io.SeekStart, want: "A"}, // pos now at 1 + {seekOff: 0, seekWhence: io.SeekCurrent, want: "n"}, // pos now at 2 + {seekOff: -4, seekWhence: io.SeekEnd, want: "file"}, // pos now at 17 } for i, tc := range tests { @@ -338,10 +325,188 @@ func TestJoin(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&empty) + fs := New(embedfs_testdata.GetTestData()) got := fs.Join(tc.path...) assert.Equal(t, tc.want, got) }) } } + +// Additional comprehensive tests using rich test data + +func TestEmbedfs_ComprehensiveOpen(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test opening existing embedded file with content + f, err := fs.Open("/testdata/file1.txt") + require.NoError(t, err) + assert.Equal(t, "file1.txt", f.Name()) + require.NoError(t, f.Close()) +} + +func TestEmbedfs_ComprehensiveRead(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + f, err := fs.Open("/testdata/file1.txt") + require.NoError(t, err) + defer f.Close() + + // Read the actual content + buf := make([]byte, 100) + n, err := f.Read(buf) + require.NoError(t, err) + assert.Equal(t, "Hello from embedfs!", string(buf[:n])) +} + +func TestEmbedfs_NestedFileOperations(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test nested file read + f, err := fs.Open("/testdata/subdir/nested.txt") + require.NoError(t, err) + defer f.Close() + + buf := make([]byte, 100) + n, err := f.Read(buf) + require.NoError(t, err) + assert.Equal(t, "Nested file content", string(buf[:n])) +} + +func TestEmbedfs_PathNormalization(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test that our path normalization works across all methods + tests := []struct { + name string + path string + }{ + {"root", "/"}, + {"top-level", "/testdata"}, + {"nested", "/testdata/subdir"}, + {"deep file", "/testdata/subdir/nested.txt"}, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // All these should work with our path normalization + _, err := fs.Stat(tt.path) + if tt.name == "deep file" { + require.NoError(t, err, "file should exist") + } else { + require.NoError(t, err, "directory should exist") + } + }) + } +} + +func TestFile_ReadAt(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + f, err := fs.Open("/testdata/file1.txt") + require.NoError(t, err) + defer f.Close() + + // Test ReadAt without affecting file position + tests := []struct { + name string + offset int64 + length int + want string + }{ + {"beginning", 0, 5, "Hello"}, + {"middle", 6, 4, "from"}, + {"end", 15, 4, "dfs!"}, + {"full content", 0, 19, "Hello from embedfs!"}, + {"beyond end", 100, 10, ""}, // Should return EOF + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + buf := make([]byte, tt.length) + n, err := f.ReadAt(buf, tt.offset) + + if tt.offset >= 19 { // Beyond file size + require.Error(t, err) + assert.Equal(t, 0, n) + } else { + if tt.offset+int64(tt.length) > 19 { + // Partial read at end of file + require.Error(t, err) // Should be EOF + assert.Greater(t, n, 0) + assert.Equal(t, tt.want, string(buf[:n])) + } else { + require.NoError(t, err) + assert.Equal(t, tt.length, n) + assert.Equal(t, tt.want, string(buf[:n])) + } + } + }) + } + + // Verify ReadAt doesn't change file position + pos, err := f.Seek(0, 1) // Get current position + require.NoError(t, err) + assert.Equal(t, int64(0), pos, "ReadAt should not change file position") +} + +func TestFile_Close(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + f, err := fs.Open("/testdata/file1.txt") + require.NoError(t, err) + + // Test first close + err = f.Close() + require.NoError(t, err) + + // Test multiple closes (should be safe) + err = f.Close() + require.NoError(t, err) + + err = f.Close() + require.NoError(t, err) + + // Note: embedfs doesn't necessarily fail operations after close + // since embed.FS files remain readable. This tests that Close() works + // without error, but doesn't enforce post-close failure behavior. +} + +func TestFile_LockUnlock(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + f, err := fs.Open("/testdata/file1.txt") + require.NoError(t, err) + defer f.Close() + + // Lock/Unlock should be no-ops that don't error + err = f.Lock() + require.NoError(t, err) + + err = f.Unlock() + require.NoError(t, err) + + // Multiple lock/unlock sequences should work + err = f.Lock() + require.NoError(t, err) + err = f.Lock() + require.NoError(t, err) + err = f.Unlock() + require.NoError(t, err) + err = f.Unlock() + require.NoError(t, err) +} diff --git a/embedfs/fs_test.go b/embedfs/fs_test.go new file mode 100644 index 0000000..833d9b2 --- /dev/null +++ b/embedfs/fs_test.go @@ -0,0 +1,109 @@ +package embedfs + +import ( + "testing" + + "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// General filesystem tests adapted from test/fs_test.go for read-only embedfs + +func TestFS_ReadDir(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test basic ReadDir functionality + entries, err := fs.ReadDir("/") + require.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, "testdata", entries[0].Name()) + assert.True(t, entries[0].IsDir()) + + // Test reading a subdirectory + entries, err = fs.ReadDir("/testdata") + require.NoError(t, err) + assert.Equal(t, 4, len(entries), "testdata should contain 4 entries") + + // Verify we can find expected files + names := make([]string, len(entries)) + for i, entry := range entries { + names[i] = entry.Name() + } + assert.Contains(t, names, "empty.txt") + assert.Contains(t, names, "file1.txt") + assert.Contains(t, names, "file2.txt") + assert.Contains(t, names, "subdir") +} + +func TestFS_ReadDirNonExistent(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test reading non-existent directory + _, err := fs.ReadDir("/non-existent") + require.Error(t, err) +} + +func TestFS_StatExisting(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test stating existing file + fi, err := fs.Stat("/testdata/file1.txt") + require.NoError(t, err) + assert.Equal(t, "file1.txt", fi.Name()) + assert.False(t, fi.IsDir()) + assert.Greater(t, fi.Size(), int64(0)) + + // Test stating existing directory + fi, err = fs.Stat("/testdata") + require.NoError(t, err) + assert.Equal(t, "testdata", fi.Name()) + assert.True(t, fi.IsDir()) +} + +func TestFS_StatNonExistent(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test stating non-existent file + _, err := fs.Stat("/non-existent") + require.Error(t, err) +} + + +func TestFS_EmptyFileHandling(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test empty file stat + fi, err := fs.Stat("/testdata/empty.txt") + require.NoError(t, err) + assert.Equal(t, "empty.txt", fi.Name()) + assert.False(t, fi.IsDir()) + assert.Equal(t, int64(0), fi.Size()) + + // Test opening empty file + f, err := fs.Open("/testdata/empty.txt") + require.NoError(t, err) + defer f.Close() + + // Test reading from empty file + buf := make([]byte, 10) + n, err := f.Read(buf) + require.Error(t, err) // Should be EOF + assert.Equal(t, 0, n) + + // Test ReadAt on empty file + n, err = f.ReadAt(buf, 0) + require.Error(t, err) // Should be EOF + assert.Equal(t, 0, n) +} + diff --git a/embedfs/testdata/empty2.txt b/embedfs/testdata/empty2.txt deleted file mode 100644 index 30d74d2..0000000 --- a/embedfs/testdata/empty2.txt +++ /dev/null @@ -1 +0,0 @@ -test \ No newline at end of file diff --git a/embedfs_testdata/provider.go b/embedfs_testdata/provider.go new file mode 100644 index 0000000..fcd2ff1 --- /dev/null +++ b/embedfs_testdata/provider.go @@ -0,0 +1,16 @@ +// Package embedfs_testdata provides embedded test data for billy embedfs testing. +// This package is only imported by test code and won't be included in production builds. +package embedfs_testdata + +import ( + "embed" +) + +//go:embed testdata +var TestData embed.FS + +// GetTestData returns the raw embed.FS for tests to wrap with their own embedfs.New(). +// This avoids import cycles while providing embedded test data. +func GetTestData() *embed.FS { + return &TestData +} \ No newline at end of file diff --git a/embedfs/testdata/empty.txt b/embedfs_testdata/testdata/empty.txt similarity index 100% rename from embedfs/testdata/empty.txt rename to embedfs_testdata/testdata/empty.txt diff --git a/embedfs_testdata/testdata/file1.txt b/embedfs_testdata/testdata/file1.txt new file mode 100644 index 0000000..7f71942 --- /dev/null +++ b/embedfs_testdata/testdata/file1.txt @@ -0,0 +1 @@ +Hello from embedfs! \ No newline at end of file diff --git a/embedfs_testdata/testdata/file2.txt b/embedfs_testdata/testdata/file2.txt new file mode 100644 index 0000000..b2679d1 --- /dev/null +++ b/embedfs_testdata/testdata/file2.txt @@ -0,0 +1 @@ +Another test file \ No newline at end of file diff --git a/embedfs_testdata/testdata/subdir/nested.txt b/embedfs_testdata/testdata/subdir/nested.txt new file mode 100644 index 0000000..276dd4b --- /dev/null +++ b/embedfs_testdata/testdata/subdir/nested.txt @@ -0,0 +1 @@ +Nested file content \ No newline at end of file From 5d154a2b90389e1e280638c1e47c9cb09ff8b29c Mon Sep 17 00:00:00 2001 From: Justin Kirkegaard Date: Wed, 23 Jul 2025 11:24:27 -0500 Subject: [PATCH 2/4] Add chroot support to embedfs using billy's chroot helper - Remove memfs dependency and use local sorting for ReadDir - Implement chroot support by wrapping embedfs with chroot.New() in New() - Add comprehensive chroot tests covering read-only operations - Follow same pattern as memfs and osfs for consistent API - Maintain read-only behavior - write operations still return ErrReadOnly This enables embedfs to work with filesystem abstractions that require chroot support for path isolation. --- embedfs/chroot_test.go | 294 +++++++++++++++++++++++++++++++++++++++++ embedfs/embed.go | 15 +-- go.mod | 2 + go.sum | 1 + 4 files changed, 303 insertions(+), 9 deletions(-) create mode 100644 embedfs/chroot_test.go diff --git a/embedfs/chroot_test.go b/embedfs/chroot_test.go new file mode 100644 index 0000000..370bac8 --- /dev/null +++ b/embedfs/chroot_test.go @@ -0,0 +1,294 @@ +package embedfs + +import ( + "io" + "testing" + + "github.com/go-git/go-billy/v6" + "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestChroot_Basic(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test chroot to existing directory + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + require.NotNil(t, chrootFS) + + // Test that we can access files in the chrooted filesystem + f, err := chrootFS.Open("file1.txt") + require.NoError(t, err) + defer f.Close() + + content, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, "Hello from embedfs!", string(content)) +} + +func TestChroot_NestedDirectory(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test chroot to nested directory + chrootFS, err := fs.Chroot("testdata/subdir") + require.NoError(t, err) + require.NotNil(t, chrootFS) + + // Test that we can access nested files from the chrooted root + f, err := chrootFS.Open("nested.txt") + require.NoError(t, err) + defer f.Close() + + content, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, "Nested file content", string(content)) +} + +func TestChroot_StatInChroot(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + + // Test stat on files that exist in chrooted directory + fi, err := chrootFS.Stat("file1.txt") + require.NoError(t, err) + assert.Equal(t, "file1.txt", fi.Name()) + assert.False(t, fi.IsDir()) + + // Test stat on directories that exist in chrooted directory + fi, err = chrootFS.Stat("subdir") + require.NoError(t, err) + assert.Equal(t, "subdir", fi.Name()) + assert.True(t, fi.IsDir()) + + // Test stat with absolute path in chrooted filesystem + fi, err = chrootFS.Stat("/file2.txt") + require.NoError(t, err) + assert.Equal(t, "file2.txt", fi.Name()) + assert.False(t, fi.IsDir()) +} + +func TestChroot_ReadDirInChroot(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + + // Test reading directory contents from chrooted root + entries, err := chrootFS.ReadDir("/") + require.NoError(t, err) + + expectedFiles := []string{"empty.txt", "file1.txt", "file2.txt", "subdir"} + assert.Len(t, entries, len(expectedFiles)) + + foundFiles := make(map[string]bool) + for _, entry := range entries { + foundFiles[entry.Name()] = true + } + + for _, expected := range expectedFiles { + assert.True(t, foundFiles[expected], "Expected file %s not found", expected) + } + + // Test reading subdirectory from chrooted filesystem + entries, err = chrootFS.ReadDir("subdir") + require.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, "nested.txt", entries[0].Name()) +} + +func TestChroot_PathNormalization(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test chroot with different path formats + tests := []struct { + name string + chrootPath string + openPath string + expectFile string + }{ + { + name: "absolute chroot path", + chrootPath: "/testdata", + openPath: "file1.txt", + expectFile: "file1.txt", + }, + { + name: "relative chroot path", + chrootPath: "testdata", + openPath: "file1.txt", + expectFile: "file1.txt", + }, + { + name: "absolute open path in chroot", + chrootPath: "testdata", + openPath: "/file1.txt", + expectFile: "file1.txt", + }, + { + name: "nested chroot", + chrootPath: "testdata/subdir", + openPath: "nested.txt", + expectFile: "nested.txt", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + chrootFS, err := fs.Chroot(tt.chrootPath) + require.NoError(t, err) + + f, err := chrootFS.Open(tt.openPath) + require.NoError(t, err) + defer f.Close() + + assert.Equal(t, tt.expectFile, f.Name()) + }) + } +} + +func TestChroot_NonExistentPath(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test chroot to non-existent directory - billy's chroot helper allows this + chrootFS, err := fs.Chroot("nonexistent") + require.NoError(t, err) + require.NotNil(t, chrootFS) + + // But accessing files within the non-existent chroot should fail + _, err = chrootFS.Open("anyfile.txt") + assert.Error(t, err) +} + +func TestChroot_Join(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + + // Test Join operation in chrooted filesystem + joined := chrootFS.Join("subdir", "nested.txt") + assert.Equal(t, "subdir/nested.txt", joined) + + // Test that joined path can be used to open file + f, err := chrootFS.Open(joined) + require.NoError(t, err) + defer f.Close() + + content, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, "Nested file content", string(content)) +} + +func TestChroot_UnsupportedOperations(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + + // Test that write operations still fail in chrooted embedfs + _, err = chrootFS.Create("newfile.txt") + require.ErrorIs(t, err, billy.ErrReadOnly) + + err = chrootFS.Remove("file1.txt") + require.ErrorIs(t, err, billy.ErrReadOnly) + + err = chrootFS.Rename("file1.txt", "renamed.txt") + require.ErrorIs(t, err, billy.ErrReadOnly) + + err = chrootFS.MkdirAll("newdir", 0755) + require.ErrorIs(t, err, billy.ErrReadOnly) +} + +func TestChroot_NestedChroot(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + + // Test creating nested chrootfs + firstChroot, err := fs.Chroot("testdata") + require.NoError(t, err) + + secondChroot, err := firstChroot.Chroot("subdir") + require.NoError(t, err) + + // Test that nested chroot works correctly + f, err := secondChroot.Open("nested.txt") + require.NoError(t, err) + defer f.Close() + + content, err := io.ReadAll(f) + require.NoError(t, err) + assert.Equal(t, "Nested file content", string(content)) + + // Test that we can't access parent directory from nested chroot + entries, err := secondChroot.ReadDir("/") + require.NoError(t, err) + assert.Len(t, entries, 1) + assert.Equal(t, "nested.txt", entries[0].Name()) +} + +func TestChroot_FileOperations(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + + // Test file operations in chrooted filesystem + f, err := chrootFS.Open("file2.txt") + require.NoError(t, err) + defer f.Close() + + // Test Read + buf := make([]byte, 10) + n, err := f.Read(buf) + require.NoError(t, err) + assert.Equal(t, "Another te", string(buf[:n])) + + // Test Seek + _, err = f.Seek(0, io.SeekStart) + require.NoError(t, err) + + // Test ReadAt + buf2 := make([]byte, 7) + n, err = f.ReadAt(buf2, 8) + require.NoError(t, err) + assert.Equal(t, "test fi", string(buf2[:n])) + + // Test that file position wasn't affected by ReadAt + n, err = f.Read(buf) + require.NoError(t, err) + assert.Equal(t, "Another te", string(buf[:n])) +} + +func TestChroot_Lstat(t *testing.T) { + t.Parallel() + + fs := New(embedfs_testdata.GetTestData()) + chrootFS, err := fs.Chroot("testdata") + require.NoError(t, err) + + // Test Lstat in chrooted filesystem (should behave same as Stat for embedfs) + fi, err := chrootFS.Lstat("file1.txt") + require.NoError(t, err) + assert.Equal(t, "file1.txt", fi.Name()) + assert.False(t, fi.IsDir()) +} \ No newline at end of file diff --git a/embedfs/embed.go b/embedfs/embed.go index 0e69f24..20abf4d 100644 --- a/embedfs/embed.go +++ b/embedfs/embed.go @@ -13,7 +13,7 @@ import ( "sync" "github.com/go-git/go-billy/v6" - "github.com/go-git/go-billy/v6/memfs" + "github.com/go-git/go-billy/v6/helper/chroot" ) type Embed struct { @@ -29,7 +29,7 @@ func New(efs *embed.FS) billy.Filesystem { fs.underlying = &embed.FS{} } - return fs + return chroot.New(fs, "/") } // normalizePath converts billy's absolute paths to embed.FS relative paths @@ -120,17 +120,14 @@ func (fs *Embed) ReadDir(path string) ([]os.FileInfo, error) { entries = append(entries, fi) } - sort.Sort(memfs.ByName(entries)) + sort.Slice(entries, func(i, j int) bool { + return entries[i].Name() < entries[j].Name() + }) return entries, nil } -// Chroot is not supported. -// -// Calls will always return billy.ErrNotSupported. -func (fs *Embed) Chroot(_ string) (billy.Filesystem, error) { - return nil, billy.ErrNotSupported -} + // Lstat behaves the same as Stat for embedded filesystems since there are no symlinks. func (fs *Embed) Lstat(filename string) (os.FileInfo, error) { diff --git a/go.mod b/go.mod index 7aada81..c5044fc 100644 --- a/go.mod +++ b/go.mod @@ -14,3 +14,5 @@ require ( github.com/pmezard/go-difflib v1.0.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) + +replace github.com/go-git/go-billy/v6 => github.com/this-kirke/go-billy/v6 v6.0.0 diff --git a/go.sum b/go.sum index 508dd26..b9a18ef 100644 --- a/go.sum +++ b/go.sum @@ -2,6 +2,7 @@ github.com/cyphar/filepath-securejoin v0.5.0 h1:hIAhkRBMQ8nIeuVwcAoymp7MY4oherZd github.com/cyphar/filepath-securejoin v0.5.0/go.mod h1:Sdj7gXlvMcPZsbhwhQ33GguGLDGQL7h7bg04C/+u9jI= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= +github.com/go-git/go-billy/v6 v6.0.0-20250711053805-c1f149aaab07/go.mod h1:Pa0/zeE0tC0GiZLFFtOYXOky9SgpNF+zkrj7aEJhBVg= github.com/pmezard/go-difflib v1.0.0 h1:4DBwDE0NGyQoBHbLQYPwSUPoCMWR5BEzIk/f1lZbAQM= github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/stretchr/testify v1.11.1 h1:7s2iGBzp5EwR7/aIZr8ao5+dra3wiQyKjjFuvgVKu7U= From 5d4833752c4d530fe2b4985405ec27101b325695 Mon Sep 17 00:00:00 2001 From: Justin Kirkegaard Date: Sat, 26 Jul 2025 22:45:58 -0500 Subject: [PATCH 3/4] Rename embedfs_testdata to internal/testdata - Move embedfs_testdata directory to embedfs/internal/testdata - Change package name from embedfs_testdata to testdata - Update all import paths in test files to use new location - Update function calls from embedfs_testdata.GetTestData() to testdata.GetTestData() - Fix test expectations for ReadDir("") to match actual billy filesystem behavior - Fix file name assertion to match embedded filesystem path structure - Add missing newlines to end of test files Co-authored-by: Paulo Gomes --- embedfs/chroot_test.go | 26 ++++++------ embedfs/debug_test.go | 6 +-- embedfs/dir_test.go | 10 ++--- embedfs/embed_test.go | 42 +++++++++---------- embedfs/fs_test.go | 12 +++--- .../internal/testdata}/provider.go | 4 +- .../internal/testdata}/testdata/empty.txt | 0 .../internal/testdata}/testdata/file1.txt | 0 .../internal/testdata}/testdata/file2.txt | 0 .../testdata}/testdata/subdir/nested.txt | 0 go.mod | 1 - 11 files changed, 50 insertions(+), 51 deletions(-) rename {embedfs_testdata => embedfs/internal/testdata}/provider.go (94%) rename {embedfs_testdata => embedfs/internal/testdata}/testdata/empty.txt (100%) rename {embedfs_testdata => embedfs/internal/testdata}/testdata/file1.txt (100%) rename {embedfs_testdata => embedfs/internal/testdata}/testdata/file2.txt (100%) rename {embedfs_testdata => embedfs/internal/testdata}/testdata/subdir/nested.txt (100%) diff --git a/embedfs/chroot_test.go b/embedfs/chroot_test.go index 370bac8..c545a41 100644 --- a/embedfs/chroot_test.go +++ b/embedfs/chroot_test.go @@ -5,7 +5,7 @@ import ( "testing" "github.com/go-git/go-billy/v6" - "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/go-git/go-billy/v6/embedfs/internal/testdata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -13,7 +13,7 @@ import ( func TestChroot_Basic(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test chroot to existing directory chrootFS, err := fs.Chroot("testdata") @@ -33,7 +33,7 @@ func TestChroot_Basic(t *testing.T) { func TestChroot_NestedDirectory(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test chroot to nested directory chrootFS, err := fs.Chroot("testdata/subdir") @@ -53,7 +53,7 @@ func TestChroot_NestedDirectory(t *testing.T) { func TestChroot_StatInChroot(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) chrootFS, err := fs.Chroot("testdata") require.NoError(t, err) @@ -80,7 +80,7 @@ func TestChroot_StatInChroot(t *testing.T) { func TestChroot_ReadDirInChroot(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) chrootFS, err := fs.Chroot("testdata") require.NoError(t, err) @@ -111,7 +111,7 @@ func TestChroot_ReadDirInChroot(t *testing.T) { func TestChroot_PathNormalization(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test chroot with different path formats tests := []struct { @@ -163,7 +163,7 @@ func TestChroot_PathNormalization(t *testing.T) { func TestChroot_NonExistentPath(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test chroot to non-existent directory - billy's chroot helper allows this chrootFS, err := fs.Chroot("nonexistent") @@ -178,7 +178,7 @@ func TestChroot_NonExistentPath(t *testing.T) { func TestChroot_Join(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) chrootFS, err := fs.Chroot("testdata") require.NoError(t, err) @@ -199,7 +199,7 @@ func TestChroot_Join(t *testing.T) { func TestChroot_UnsupportedOperations(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) chrootFS, err := fs.Chroot("testdata") require.NoError(t, err) @@ -220,7 +220,7 @@ func TestChroot_UnsupportedOperations(t *testing.T) { func TestChroot_NestedChroot(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test creating nested chrootfs firstChroot, err := fs.Chroot("testdata") @@ -248,7 +248,7 @@ func TestChroot_NestedChroot(t *testing.T) { func TestChroot_FileOperations(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) chrootFS, err := fs.Chroot("testdata") require.NoError(t, err) @@ -282,7 +282,7 @@ func TestChroot_FileOperations(t *testing.T) { func TestChroot_Lstat(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) chrootFS, err := fs.Chroot("testdata") require.NoError(t, err) @@ -291,4 +291,4 @@ func TestChroot_Lstat(t *testing.T) { require.NoError(t, err) assert.Equal(t, "file1.txt", fi.Name()) assert.False(t, fi.IsDir()) -} \ No newline at end of file +} diff --git a/embedfs/debug_test.go b/embedfs/debug_test.go index 2bc01b7..72329e2 100644 --- a/embedfs/debug_test.go +++ b/embedfs/debug_test.go @@ -4,7 +4,7 @@ import ( "embed" "testing" - "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/go-git/go-billy/v6/embedfs/internal/testdata" ) var emptyEmbedFS embed.FS @@ -20,11 +20,11 @@ func TestDebugErrors(t *testing.T) { t.Logf("Empty FS, root path: %v", err2) // Test 3: Non-empty embed.FS with empty path - richFS := New(embedfs_testdata.GetTestData()) + richFS := New(testdata.GetTestData()) _, err3 := richFS.ReadDir("") t.Logf("Rich FS, empty path: %v", err3) // Test 4: Non-empty embed.FS with root path entries, err4 := richFS.ReadDir("/") t.Logf("Rich FS, root path: %d entries, err: %v", len(entries), err4) -} \ No newline at end of file +} diff --git a/embedfs/dir_test.go b/embedfs/dir_test.go index b12ba54..1bb9d7c 100644 --- a/embedfs/dir_test.go +++ b/embedfs/dir_test.go @@ -4,7 +4,7 @@ import ( "os" "testing" - "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/go-git/go-billy/v6/embedfs/internal/testdata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -14,7 +14,7 @@ import ( func TestDir_ReadDirNested(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test reading nested directories entries, err := fs.ReadDir("/testdata/subdir") @@ -27,7 +27,7 @@ func TestDir_ReadDirNested(t *testing.T) { func TestDir_ReadDirFileInfo(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) entries, err := fs.ReadDir("/testdata") require.NoError(t, err) @@ -43,7 +43,7 @@ func TestDir_ReadDirFileInfo(t *testing.T) { func TestDir_ReadDirFileInfoDirs(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) entries, err := fs.ReadDir("/testdata") require.NoError(t, err) @@ -60,4 +60,4 @@ func TestDir_ReadDirFileInfoDirs(t *testing.T) { require.NotNil(t, subdirEntry, "subdir should be found") assert.True(t, subdirEntry.IsDir(), "subdir should be a directory") assert.Equal(t, "subdir", subdirEntry.Name()) -} \ No newline at end of file +} diff --git a/embedfs/embed_test.go b/embedfs/embed_test.go index 61dad08..eeedba4 100644 --- a/embedfs/embed_test.go +++ b/embedfs/embed_test.go @@ -8,7 +8,7 @@ import ( "testing" "github.com/go-git/go-billy/v6" - "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/go-git/go-billy/v6/embedfs/internal/testdata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -38,7 +38,7 @@ func TestOpen(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) var got []byte f, err := fs.Open(tc.name) @@ -109,7 +109,7 @@ func TestOpenFileFlags(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) _, err := fs.OpenFile(tc.file, tc.flag, 0o700) if tc.wantErr != "" { @@ -151,7 +151,7 @@ func TestStat(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) fi, err := fs.Stat(tc.name) if tc.wantErr { @@ -180,20 +180,20 @@ func TestReadDir(t *testing.T) { { name: "testdata", path: "testdata", - fs: embedfs_testdata.GetTestData(), + fs: testdata.GetTestData(), want: []string{"empty.txt", "file1.txt", "file2.txt", "subdir"}, }, { name: "empty path", path: "", - fs: embedfs_testdata.GetTestData(), - want: []string{}, - wantErr: true, + fs: testdata.GetTestData(), + want: []string{"testdata"}, + wantErr: false, }, { name: "root path", path: "/", - fs: embedfs_testdata.GetTestData(), + fs: testdata.GetTestData(), want: []string{"testdata"}, }, } @@ -228,7 +228,7 @@ func TestReadDir(t *testing.T) { func TestUnsupported(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) _, err := fs.Create("test") require.ErrorIs(t, err, billy.ErrReadOnly) @@ -246,7 +246,7 @@ func TestUnsupported(t *testing.T) { func TestFileUnsupported(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) f, err := fs.Open("testdata/file1.txt") require.NoError(t, err) @@ -260,7 +260,7 @@ func TestFileUnsupported(t *testing.T) { } func TestFileSeek(t *testing.T) { - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) f, err := fs.Open("testdata/file2.txt") require.NoError(t, err) @@ -325,7 +325,7 @@ func TestJoin(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) got := fs.Join(tc.path...) assert.Equal(t, tc.want, got) @@ -338,19 +338,19 @@ func TestJoin(t *testing.T) { func TestEmbedfs_ComprehensiveOpen(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test opening existing embedded file with content f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) - assert.Equal(t, "file1.txt", f.Name()) + assert.Equal(t, "testdata/file1.txt", f.Name()) require.NoError(t, f.Close()) } func TestEmbedfs_ComprehensiveRead(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) @@ -366,7 +366,7 @@ func TestEmbedfs_ComprehensiveRead(t *testing.T) { func TestEmbedfs_NestedFileOperations(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test nested file read f, err := fs.Open("/testdata/subdir/nested.txt") @@ -382,7 +382,7 @@ func TestEmbedfs_NestedFileOperations(t *testing.T) { func TestEmbedfs_PathNormalization(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test that our path normalization works across all methods tests := []struct { @@ -411,7 +411,7 @@ func TestEmbedfs_PathNormalization(t *testing.T) { func TestFile_ReadAt(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) @@ -463,7 +463,7 @@ func TestFile_ReadAt(t *testing.T) { func TestFile_Close(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) @@ -487,7 +487,7 @@ func TestFile_Close(t *testing.T) { func TestFile_LockUnlock(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) diff --git a/embedfs/fs_test.go b/embedfs/fs_test.go index 833d9b2..ae8896e 100644 --- a/embedfs/fs_test.go +++ b/embedfs/fs_test.go @@ -3,7 +3,7 @@ package embedfs import ( "testing" - "github.com/go-git/go-billy/v6/embedfs_testdata" + "github.com/go-git/go-billy/v6/embedfs/internal/testdata" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -13,7 +13,7 @@ import ( func TestFS_ReadDir(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test basic ReadDir functionality entries, err := fs.ReadDir("/") @@ -41,7 +41,7 @@ func TestFS_ReadDir(t *testing.T) { func TestFS_ReadDirNonExistent(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test reading non-existent directory _, err := fs.ReadDir("/non-existent") @@ -51,7 +51,7 @@ func TestFS_ReadDirNonExistent(t *testing.T) { func TestFS_StatExisting(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test stating existing file fi, err := fs.Stat("/testdata/file1.txt") @@ -70,7 +70,7 @@ func TestFS_StatExisting(t *testing.T) { func TestFS_StatNonExistent(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test stating non-existent file _, err := fs.Stat("/non-existent") @@ -81,7 +81,7 @@ func TestFS_StatNonExistent(t *testing.T) { func TestFS_EmptyFileHandling(t *testing.T) { t.Parallel() - fs := New(embedfs_testdata.GetTestData()) + fs := New(testdata.GetTestData()) // Test empty file stat fi, err := fs.Stat("/testdata/empty.txt") diff --git a/embedfs_testdata/provider.go b/embedfs/internal/testdata/provider.go similarity index 94% rename from embedfs_testdata/provider.go rename to embedfs/internal/testdata/provider.go index fcd2ff1..a7a71bb 100644 --- a/embedfs_testdata/provider.go +++ b/embedfs/internal/testdata/provider.go @@ -1,6 +1,6 @@ // Package embedfs_testdata provides embedded test data for billy embedfs testing. // This package is only imported by test code and won't be included in production builds. -package embedfs_testdata +package testdata import ( "embed" @@ -13,4 +13,4 @@ var TestData embed.FS // This avoids import cycles while providing embedded test data. func GetTestData() *embed.FS { return &TestData -} \ No newline at end of file +} diff --git a/embedfs_testdata/testdata/empty.txt b/embedfs/internal/testdata/testdata/empty.txt similarity index 100% rename from embedfs_testdata/testdata/empty.txt rename to embedfs/internal/testdata/testdata/empty.txt diff --git a/embedfs_testdata/testdata/file1.txt b/embedfs/internal/testdata/testdata/file1.txt similarity index 100% rename from embedfs_testdata/testdata/file1.txt rename to embedfs/internal/testdata/testdata/file1.txt diff --git a/embedfs_testdata/testdata/file2.txt b/embedfs/internal/testdata/testdata/file2.txt similarity index 100% rename from embedfs_testdata/testdata/file2.txt rename to embedfs/internal/testdata/testdata/file2.txt diff --git a/embedfs_testdata/testdata/subdir/nested.txt b/embedfs/internal/testdata/testdata/subdir/nested.txt similarity index 100% rename from embedfs_testdata/testdata/subdir/nested.txt rename to embedfs/internal/testdata/testdata/subdir/nested.txt diff --git a/go.mod b/go.mod index c5044fc..0a0218b 100644 --- a/go.mod +++ b/go.mod @@ -15,4 +15,3 @@ require ( gopkg.in/yaml.v3 v3.0.1 // indirect ) -replace github.com/go-git/go-billy/v6 => github.com/this-kirke/go-billy/v6 v6.0.0 From 7774d7e527d86468770923b26a49eb313a0ef683 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Tue, 14 Oct 2025 09:50:23 +0100 Subject: [PATCH 4/4] embedfs: Apply PR feedback Signed-off-by: Paulo Gomes --- embedfs/embed_test.go | 59 ++++++++++++++++++++----------------------- 1 file changed, 27 insertions(+), 32 deletions(-) diff --git a/embedfs/embed_test.go b/embedfs/embed_test.go index eeedba4..3f6bbb3 100644 --- a/embedfs/embed_test.go +++ b/embedfs/embed_test.go @@ -13,7 +13,6 @@ import ( "github.com/stretchr/testify/require" ) - func TestOpen(t *testing.T) { t.Parallel() @@ -186,8 +185,8 @@ func TestReadDir(t *testing.T) { { name: "empty path", path: "", - fs: testdata.GetTestData(), - want: []string{"testdata"}, + fs: &emptyEmbedFS, + want: []string{}, wantErr: false, }, { @@ -271,14 +270,14 @@ func TestFileSeek(t *testing.T) { seekWhence int want string }{ - {seekOff: 8, seekWhence: io.SeekStart, want: "test file"}, // pos now at 17 - {seekOff: 8, seekWhence: io.SeekStart, want: "t"}, // pos now at 9 - {seekOff: 9, seekWhence: io.SeekStart, want: "est"}, // pos now at 12 - {seekOff: 1, seekWhence: io.SeekStart, want: "nother test file"}, // pos now at 17 + {seekOff: 8, seekWhence: io.SeekStart, want: "test file"}, // pos now at 17 + {seekOff: 8, seekWhence: io.SeekStart, want: "t"}, // pos now at 9 + {seekOff: 9, seekWhence: io.SeekStart, want: "est"}, // pos now at 12 + {seekOff: 1, seekWhence: io.SeekStart, want: "nother test file"}, // pos now at 17 {seekOff: 0, seekWhence: io.SeekStart, want: "Another test file"}, // pos now at 17 - {seekOff: 0, seekWhence: io.SeekStart, want: "A"}, // pos now at 1 - {seekOff: 0, seekWhence: io.SeekCurrent, want: "n"}, // pos now at 2 - {seekOff: -4, seekWhence: io.SeekEnd, want: "file"}, // pos now at 17 + {seekOff: 0, seekWhence: io.SeekStart, want: "A"}, // pos now at 1 + {seekOff: 0, seekWhence: io.SeekCurrent, want: "n"}, // pos now at 2 + {seekOff: -4, seekWhence: io.SeekEnd, want: "file"}, // pos now at 17 } for i, tc := range tests { @@ -337,9 +336,9 @@ func TestJoin(t *testing.T) { func TestEmbedfs_ComprehensiveOpen(t *testing.T) { t.Parallel() - + fs := New(testdata.GetTestData()) - + // Test opening existing embedded file with content f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) @@ -349,13 +348,13 @@ func TestEmbedfs_ComprehensiveOpen(t *testing.T) { func TestEmbedfs_ComprehensiveRead(t *testing.T) { t.Parallel() - + fs := New(testdata.GetTestData()) - + f, err := fs.Open("/testdata/file1.txt") require.NoError(t, err) defer f.Close() - + // Read the actual content buf := make([]byte, 100) n, err := f.Read(buf) @@ -365,14 +364,14 @@ func TestEmbedfs_ComprehensiveRead(t *testing.T) { func TestEmbedfs_NestedFileOperations(t *testing.T) { t.Parallel() - + fs := New(testdata.GetTestData()) - + // Test nested file read f, err := fs.Open("/testdata/subdir/nested.txt") require.NoError(t, err) defer f.Close() - + buf := make([]byte, 100) n, err := f.Read(buf) require.NoError(t, err) @@ -381,9 +380,9 @@ func TestEmbedfs_NestedFileOperations(t *testing.T) { func TestEmbedfs_PathNormalization(t *testing.T) { t.Parallel() - + fs := New(testdata.GetTestData()) - + // Test that our path normalization works across all methods tests := []struct { name string @@ -394,7 +393,7 @@ func TestEmbedfs_PathNormalization(t *testing.T) { {"nested", "/testdata/subdir"}, {"deep file", "/testdata/subdir/nested.txt"}, } - + for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { // All these should work with our path normalization @@ -428,21 +427,21 @@ func TestFile_ReadAt(t *testing.T) { {"middle", 6, 4, "from"}, {"end", 15, 4, "dfs!"}, {"full content", 0, 19, "Hello from embedfs!"}, - {"beyond end", 100, 10, ""}, // Should return EOF + {"beyond end", 100, 10, ""}, // Should return EOF } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { buf := make([]byte, tt.length) n, err := f.ReadAt(buf, tt.offset) - - if tt.offset >= 19 { // Beyond file size + + if tt.offset >= 19 { // Beyond file size require.Error(t, err) assert.Equal(t, 0, n) } else { if tt.offset+int64(tt.length) > 19 { // Partial read at end of file - require.Error(t, err) // Should be EOF + require.ErrorIs(t, err, io.EOF) assert.Greater(t, n, 0) assert.Equal(t, tt.want, string(buf[:n])) } else { @@ -472,16 +471,12 @@ func TestFile_Close(t *testing.T) { err = f.Close() require.NoError(t, err) - // Test multiple closes (should be safe) + // Multiple closes must not fail. err = f.Close() require.NoError(t, err) err = f.Close() require.NoError(t, err) - - // Note: embedfs doesn't necessarily fail operations after close - // since embed.FS files remain readable. This tests that Close() works - // without error, but doesn't enforce post-close failure behavior. } func TestFile_LockUnlock(t *testing.T) { @@ -493,14 +488,14 @@ func TestFile_LockUnlock(t *testing.T) { require.NoError(t, err) defer f.Close() - // Lock/Unlock should be no-ops that don't error + // Lock/Unlock are no-ops and must not error. err = f.Lock() require.NoError(t, err) err = f.Unlock() require.NoError(t, err) - // Multiple lock/unlock sequences should work + // Invalid lock/unlock sequences are not checked. err = f.Lock() require.NoError(t, err) err = f.Lock()