From 915b478d40dffa84bc1aaa9f2db1ff962584415c Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 5 Apr 2024 22:40:44 +0100 Subject: [PATCH 1/7] build: Bump Go to 1.20 Signed-off-by: Paulo Gomes --- .github/workflows/test.yml | 2 +- go.mod | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index e8470d4..4cd7eb2 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -9,7 +9,7 @@ jobs: strategy: fail-fast: false matrix: - go-version: [1.19.x, 1.20.x, 1.21.x] + go-version: [1.20.x, 1.21.x, 1.22.x] platform: [ubuntu-latest, macos-latest, windows-latest] runs-on: ${{ matrix.platform }} diff --git a/go.mod b/go.mod index 25d27bb..2119641 100644 --- a/go.mod +++ b/go.mod @@ -1,7 +1,7 @@ module github.com/go-git/go-git-fixtures/v4 // go-git supports the last 3 stable Go versions. -go 1.19 +go 1.20 require ( github.com/go-git/go-billy/v5 v5.5.0 From c0d4342154535ba4578ebf5004dc060bdc38cf30 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Sun, 7 Apr 2024 23:30:55 +0100 Subject: [PATCH 2/7] Add embedfs to represent embed.FS as billy.Filesystem This representation is mostly useful within the context of go-git-fixtures. go-billy Filesystem interface is too generic, which makes this implementation violate the Liskov Principle. When more narrow representations of the filesystem are available in go-billy, this could be moved upstream. Signed-off-by: Paulo Gomes --- internal/embedfs/embed.go | 208 +++++++++++++++++ internal/embedfs/embed_test.go | 322 +++++++++++++++++++++++++++ internal/embedfs/testdata/empty.txt | 0 internal/embedfs/testdata/empty2.txt | 1 + 4 files changed, 531 insertions(+) create mode 100644 internal/embedfs/embed.go create mode 100644 internal/embedfs/embed_test.go create mode 100644 internal/embedfs/testdata/empty.txt create mode 100644 internal/embedfs/testdata/empty2.txt diff --git a/internal/embedfs/embed.go b/internal/embedfs/embed.go new file mode 100644 index 0000000..1482d4f --- /dev/null +++ b/internal/embedfs/embed.go @@ -0,0 +1,208 @@ +// embedfs exposes an embed.FS as a read-only billy.Filesystem. +package embedfs + +import ( + "bytes" + "embed" + "fmt" + "io/fs" + "os" + "path/filepath" + "sort" + "strings" + "sync" + + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/helper/chroot" + "github.com/go-git/go-billy/v5/memfs" +) + +type Embed struct { + underlying *embed.FS +} + +func New(efs *embed.FS, path string) billy.Filesystem { + fs := &Embed{ + underlying: efs, + } + + if efs == nil { + fs.underlying = &embed.FS{} + } + + return chroot.New(fs, path) +} + +func (fs *Embed) Stat(filename string) (os.FileInfo, error) { + f, err := fs.underlying.Open(filename) + if err != nil { + return nil, err + } + return f.Stat() +} + +func (fs *Embed) Open(filename string) (billy.File, error) { + return fs.OpenFile(filename, os.O_RDONLY, 0) +} + +func (fs *Embed) OpenFile(filename string, flag int, perm os.FileMode) (billy.File, error) { + if flag&(os.O_CREATE|os.O_WRONLY|os.O_APPEND|os.O_RDWR|os.O_EXCL|os.O_TRUNC) != 0 { + return nil, billy.ErrReadOnly + } + + f, err := fs.underlying.Open(filename) + if err != nil { + return nil, err + } + + fi, err := f.Stat() + if err != nil { + return nil, err + } + + if fi.IsDir() { + return nil, fmt.Errorf("cannot open directory: %s", filename) + } + + data, err := fs.underlying.ReadFile(filename) + if err != nil { + return nil, err + } + + // Only load the bytes to memory if the files is needed. + lazyFunc := func() *bytes.Reader { return bytes.NewReader(data) } + return toFile(lazyFunc, fi), nil +} + +// TODO: use memfs instead +func (fs *Embed) Join(elem ...string) string { + // Function adapted from Go's filepath.Join for unix: + // https://github.com/golang/go/blob/1ed85ee228023d766b37db056311929c00091c9f/src/path/filepath/path_unix.go#L45 + for i, el := range elem { + if el != "" { + // reuses filepath.Clean, as it is OS agnostic. + return filepath.Clean(strings.Join(elem[i:], "/")) + } + } + return "" +} + +func (fs *Embed) ReadDir(path string) ([]os.FileInfo, error) { + e, err := fs.underlying.ReadDir(path) + if err != nil { + return nil, err + } + + var entries []os.FileInfo + for _, f := range e { + fi, _ := f.Info() + entries = append(entries, fi) + } + + sort.Sort(memfs.ByName(entries)) + + return entries, nil +} + +// Create is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (fs *Embed) Create(filename string) (billy.File, error) { + return nil, billy.ErrReadOnly +} + +// Rename is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (fs *Embed) Rename(from, to string) error { + return billy.ErrReadOnly +} + +// Remove is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (fs *Embed) Remove(filename string) error { + return billy.ErrReadOnly +} + +// MkdirAll is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (fs *Embed) MkdirAll(filename string, perm os.FileMode) error { + return billy.ErrReadOnly +} + +func toFile(lazy func() *bytes.Reader, fi fs.FileInfo) billy.File { + new := &file{ + lazy: lazy, + fi: fi, + } + + return new +} + +type file struct { + lazy func() *bytes.Reader + reader *bytes.Reader + fi fs.FileInfo + once sync.Once +} + +func (f *file) loadReader() { + f.reader = f.lazy() +} + +func (f *file) Name() string { + return f.fi.Name() +} + +func (f *file) Read(b []byte) (int, error) { + f.once.Do(f.loadReader) + + return f.reader.Read(b) +} + +func (f *file) ReadAt(b []byte, off int64) (int, error) { + f.once.Do(f.loadReader) + + return f.reader.ReadAt(b, off) +} + +func (f *file) Seek(offset int64, whence int) (int64, error) { + f.once.Do(f.loadReader) + + return f.reader.Seek(offset, whence) +} + +func (f *file) Stat() (os.FileInfo, error) { + return f.fi, nil +} + +// Close for embedfs file is a no-op. +func (f *file) Close() error { + return nil +} + +// Lock for embedfs file is a no-op. +func (f *file) Lock() error { + return nil +} + +// Unlock for embedfs file is a no-op. +func (f *file) Unlock() error { + return nil +} + +// Truncate is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (f *file) Truncate(size int64) error { + return billy.ErrReadOnly +} + +// Write is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (f *file) Write(p []byte) (int, error) { + return 0, billy.ErrReadOnly +} diff --git a/internal/embedfs/embed_test.go b/internal/embedfs/embed_test.go new file mode 100644 index 0000000..7e1dec5 --- /dev/null +++ b/internal/embedfs/embed_test.go @@ -0,0 +1,322 @@ +package embedfs + +import ( + "embed" + "io" + "os" + "testing" + + "github.com/go-git/go-billy/v5" + "github.com/stretchr/testify/assert" +) + +//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) { + tests := []struct { + name string + want []byte + wantErr bool + }{ + { + name: "empty.txt", + want: []byte(""), + }, + { + name: "empty2.txt", + want: []byte("test\n"), + }, + { + name: "non-existent", + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fs := New(&testdataDir, "testdata") + + var got []byte + f, err := fs.Open(tc.name) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + + got, err = io.ReadAll(f) + assert.NoError(t, err) + } + + assert.Equal(t, got, tc.want) + }) + } +} + +func TestOpenFileFlags(t *testing.T) { + tests := []struct { + name string + file string + flag int + wantErr string + }{ + { + name: "O_CREATE", + file: "empty.txt", + flag: os.O_CREATE, + wantErr: "read-only filesystem", + }, + { + name: "O_WRONLY", + file: "empty.txt", + flag: os.O_WRONLY, + wantErr: "read-only filesystem", + }, + { + name: "O_TRUNC", + file: "empty.txt", + flag: os.O_TRUNC, + wantErr: "read-only filesystem", + }, + { + name: "O_RDWR", + file: "empty.txt", + flag: os.O_RDWR, + wantErr: "read-only filesystem", + }, + { + name: "O_EXCL", + file: "empty.txt", + flag: os.O_EXCL, + wantErr: "read-only filesystem", + }, + { + name: "O_RDONLY", + file: "empty.txt", + flag: os.O_RDONLY, + }, + { + name: "no flags", + file: "empty.txt", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fs := New(&testdataDir, "testdata") + + _, err := fs.OpenFile(tc.file, tc.flag, 0o700) + if tc.wantErr != "" { + assert.ErrorContains(t, err, tc.wantErr) + } else { + assert.NoError(t, err) + } + }) + } +} + +func TestStat(t *testing.T) { + tests := []struct { + name string + want string + isDir bool + wantErr bool + }{ + { + name: "testdata/empty.txt", + want: "empty.txt", + }, + { + name: "testdata/empty2.txt", + want: "empty2.txt", + }, + { + name: "non-existent", + wantErr: true, + }, + { + name: "testdata/", + want: "testdata", + isDir: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fs := New(&testdataDir, "") + + fi, err := fs.Stat(tc.name) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + + assert.Equal(t, tc.want, fi.Name()) + assert.Equal(t, tc.isDir, fi.IsDir()) + } + }) + } +} + +func TestReadDir(t *testing.T) { + tests := []struct { + name string + chroot string + path string + fs embed.FS + want []string + wantErr bool + }{ + { + name: "singleFile w/ chroot", + chroot: "testdata/", + path: "", + fs: singleFile, + want: []string{"empty.txt"}, + }, + { + name: "singleFile w/o chroot", + chroot: "", + path: "testdata", + fs: singleFile, + want: []string{"empty.txt"}, + }, + { + name: "singleFile return no dir names", + chroot: "", + path: "", + fs: singleFile, + want: []string{}, + wantErr: true, + }, + { + name: "empty", + chroot: "", + path: "", + fs: empty, + want: []string{}, + wantErr: true, + }, + + { + name: "testdataDir w/ chroot", + chroot: "testdata", + path: "", + fs: testdataDir, + want: []string{"empty.txt", "empty2.txt"}, + }, + { + name: "testdataDir w/o chroot", + chroot: "", + path: "testdata", + fs: testdataDir, + want: []string{"empty.txt", "empty2.txt"}, + }, + { + name: "testdataDir return no dir names", + chroot: "", + path: "", + fs: testdataDir, + want: []string{}, + wantErr: true, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fs := New(&tc.fs, tc.chroot) + + fis, err := fs.ReadDir(tc.path) + if tc.wantErr { + assert.Error(t, err) + } else { + assert.NoError(t, err) + } + + assert.Len(t, fis, len(tc.want)) + matched := 0 + + for _, n := range fis { + for _, w := range tc.want { + if n.Name() == w { + matched++ + } + } + } + + assert.Equal(t, len(tc.want), matched, "not all files matched") + }) + } +} + +func TestUnsupported(t *testing.T) { + fs := New(&testdataDir, "") + + _, err := fs.Create("test") + assert.ErrorIs(t, err, billy.ErrReadOnly) + + err = fs.Remove("test") + assert.ErrorIs(t, err, billy.ErrReadOnly) + + err = fs.Rename("test", "test") + assert.ErrorIs(t, err, billy.ErrReadOnly) + + err = fs.MkdirAll("test", 0o700) + assert.ErrorIs(t, err, billy.ErrReadOnly) +} + +func TestFileUnsupported(t *testing.T) { + fs := New(&testdataDir, "testdata") + + f, err := fs.Open("empty.txt") + assert.NoError(t, err) + assert.NotNil(t, f) + + _, err = f.Write([]byte("foo")) + assert.ErrorIs(t, err, billy.ErrReadOnly) + + err = f.Truncate(0) + assert.ErrorIs(t, err, billy.ErrReadOnly) +} + +func TestFileSeek(t *testing.T) { + fs := New(&testdataDir, "testdata") + + f, err := fs.Open("empty2.txt") + assert.NoError(t, err) + assert.NotNil(t, f) + + tests := []struct { + seekOff int64 + seekWhence int + want string + }{ + {seekOff: 4, seekWhence: io.SeekStart, want: "\n"}, + {seekOff: 3, seekWhence: io.SeekStart, want: "t\n"}, + {seekOff: 2, seekWhence: io.SeekStart, want: "st\n"}, + {seekOff: 1, seekWhence: io.SeekStart, want: "est\n"}, + {seekOff: 0, seekWhence: io.SeekStart, want: "test\n"}, + {seekOff: 0, seekWhence: io.SeekStart, want: "t"}, + {seekOff: 1, seekWhence: io.SeekCurrent, want: "st\n"}, + {seekOff: -3, seekWhence: io.SeekEnd, want: "st\n"}, + } + + for _, tc := range tests { + t.Run("", func(t *testing.T) { + + _, err = f.Seek(tc.seekOff, tc.seekWhence) + assert.NoError(t, err) + + data := make([]byte, len(tc.want)) + n, err := f.Read(data) + assert.NoError(t, err) + assert.Equal(t, len(tc.want), n) + assert.Equal(t, []byte(tc.want), data) + }) + } +} diff --git a/internal/embedfs/testdata/empty.txt b/internal/embedfs/testdata/empty.txt new file mode 100644 index 0000000..e69de29 diff --git a/internal/embedfs/testdata/empty2.txt b/internal/embedfs/testdata/empty2.txt new file mode 100644 index 0000000..9daeafb --- /dev/null +++ b/internal/embedfs/testdata/empty2.txt @@ -0,0 +1 @@ +test From 414d44e11b3a5aac79a9c5dfa91fb7b27f408c53 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 8 Apr 2024 21:56:47 +0100 Subject: [PATCH 3/7] Refactor for full in-memory support The refactoring achieves a few goals: - Support for fully in-memory execution, without the need of extracting tgz files into disk. - Removal of the deprecated check.v1 dependency. Signed-off-by: Paulo Gomes --- .github/workflows/test.yml | 8 +- fixtures.go | 101 ++++++------------- fixtures_options.go | 53 ++++++++++ fixtures_test.go | 28 ++++-- go.mod | 4 - go.sum | 10 -- internal/embedfs/embed.go | 54 ++++++++-- internal/embedfs/embed_test.go | 145 +++++++++++++++------------ internal/embedfs/testdata/empty2.txt | 2 +- internal/tgz/tgz.go | 45 +++------ internal/tgz/tgz_test.go | 134 +++++++++++-------------- 11 files changed, 304 insertions(+), 280 deletions(-) create mode 100644 fixtures_options.go diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 4cd7eb2..b519d72 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -14,13 +14,13 @@ jobs: runs-on: ${{ matrix.platform }} steps: + - name: Checkout code + uses: actions/checkout@v4 + - name: Install Go uses: actions/setup-go@v5 with: - go-version: ${{ matrix.go-version }} - - - name: Checkout code - uses: actions/checkout@v4 + go-version: ${{ matrix.go-version }} - name: Test run: make test diff --git a/fixtures.go b/fixtures.go index c0229e1..f3f9a9d 100644 --- a/fixtures.go +++ b/fixtures.go @@ -9,15 +9,11 @@ import ( "testing" "github.com/go-git/go-billy/v5" - "github.com/go-git/go-billy/v5/osfs" + "github.com/go-git/go-git-fixtures/v4/internal/embedfs" "github.com/go-git/go-git-fixtures/v4/internal/tgz" - "gopkg.in/check.v1" ) -var ( - files = make(map[string]string) - Filesystem = osfs.New(os.TempDir()) -) +var Filesystem = embedfs.New(&data) //go:embed data var data embed.FS @@ -231,35 +227,8 @@ func (f *Fixture) Is(tag string) bool { return false } -func (f *Fixture) file(path string) (billy.File, error) { - if fpath, ok := files[path]; ok { - return Filesystem.Open(fpath) - } - - bytes, err := data.ReadFile("data/" + path) - if err != nil { - return nil, err - } - - file, err := Filesystem.TempFile("", "go-git-fixtures") - if err != nil { - return nil, err - } - - if _, err := file.Write(bytes); err != nil { - return nil, err - } - - if err := file.Close(); err != nil { - return nil, err - } - - files[path] = file.Name() - return Filesystem.Open(file.Name()) -} - func (f *Fixture) Packfile() billy.File { - file, err := f.file(fmt.Sprintf("pack-%s.pack", f.PackfileHash)) + file, err := Filesystem.Open(fmt.Sprintf("data/pack-%s.pack", f.PackfileHash)) if err != nil { panic(err) } @@ -268,7 +237,7 @@ func (f *Fixture) Packfile() billy.File { } func (f *Fixture) Idx() billy.File { - file, err := f.file(fmt.Sprintf("pack-%s.idx", f.PackfileHash)) + file, err := Filesystem.Open(fmt.Sprintf("data/pack-%s.idx", f.PackfileHash)) if err != nil { panic(err) } @@ -277,7 +246,7 @@ func (f *Fixture) Idx() billy.File { } func (f *Fixture) Rev() billy.File { - file, err := f.file(fmt.Sprintf("pack-%s.rev", f.PackfileHash)) + file, err := Filesystem.Open(fmt.Sprintf("data/pack-%s.rev", f.PackfileHash)) if err != nil { panic(err) } @@ -287,18 +256,23 @@ func (f *Fixture) Rev() billy.File { // DotGit creates a new temporary directory and unpacks the repository .git // directory into it. Multiple calls to DotGit returns different directories. -func (f *Fixture) DotGit() billy.Filesystem { +func (f *Fixture) DotGit(opts ...Option) billy.Filesystem { + o := newOptions() + for _, opt := range opts { + opt(o) + } + if f.DotGitHash == "" && f.WorktreeHash != "" { - fs, _ := f.Worktree().Chroot(".git") + fs, _ := f.Worktree(opts...).Chroot(".git") return fs.(billy.Filesystem) } - file, err := f.file(fmt.Sprintf("git-%s.tgz", f.DotGitHash)) + file, err := Filesystem.Open(fmt.Sprintf("data/git-%s.tgz", f.DotGitHash)) if err != nil { panic(err) } - fs, err, _ := tgz.Extract(Filesystem, file.Name()) + fs, err := tgz.Extract(file, o.fsFactory) if err != nil { panic(err) } @@ -332,13 +306,18 @@ func EnsureIsBare(fs billy.Filesystem) error { return err } -func (f *Fixture) Worktree() billy.Filesystem { - file, err := f.file(fmt.Sprintf("worktree-%s.tgz", f.WorktreeHash)) +func (f *Fixture) Worktree(opts ...Option) billy.Filesystem { + o := newOptions() + for _, opt := range opts { + opt(o) + } + + file, err := Filesystem.Open(fmt.Sprintf("data/worktree-%s.tgz", f.WorktreeHash)) if err != nil { panic(err) } - fs, err, _ := tgz.Extract(Filesystem, file.Name()) + fs, err := tgz.Extract(file, o.fsFactory) if err != nil { panic(err) } @@ -348,15 +327,6 @@ func (f *Fixture) Worktree() billy.Filesystem { type Fixtures []*Fixture -// Deprecated as part of removing check from the code base. -// Use Run instead. -func (g Fixtures) Test(c *check.C, test func(*Fixture)) { - for _, f := range g { - c.Logf("executing test at %s %s", f.URL, f.Tags) - test(f) - } -} - // Run calls test within a t.Run for each fixture in g. func (g Fixtures) Run(t *testing.T, test func(*testing.T, *Fixture)) { for _, f := range g { @@ -368,11 +338,14 @@ func (g Fixtures) Run(t *testing.T, test func(*testing.T, *Fixture)) { } func (g Fixtures) One() *Fixture { + if len(g) == 0 { + return nil + } return g[0] } func (g Fixtures) ByTag(tag string) Fixtures { - r := make(Fixtures, 0) + r := make(Fixtures, 0, len(g)) for _, f := range g { if f.Is(tag) { r = append(r, f) @@ -383,7 +356,7 @@ func (g Fixtures) ByTag(tag string) Fixtures { } func (g Fixtures) ByURL(url string) Fixtures { - r := make(Fixtures, 0) + r := make(Fixtures, 0, len(g)) for _, f := range g { if f.URL == url { r = append(r, f) @@ -394,7 +367,7 @@ func (g Fixtures) ByURL(url string) Fixtures { } func (g Fixtures) Exclude(tag string) Fixtures { - r := make(Fixtures, 0) + r := make(Fixtures, 0, len(g)) for _, f := range g { if !f.Is(tag) { r = append(r, f) @@ -403,21 +376,3 @@ func (g Fixtures) Exclude(tag string) Fixtures { return r } - -// Clean cleans all the temporal files created -func Clean() error { - for fname, f := range files { - if err := Filesystem.Remove(f); err != nil { - return err - } - delete(files, fname) - } - return nil -} - -type Suite struct{} - -// Deprecated as part of removing check from the code base. -func (s *Suite) TearDownSuite(c *check.C) { - Clean() -} diff --git a/fixtures_options.go b/fixtures_options.go new file mode 100644 index 0000000..0867773 --- /dev/null +++ b/fixtures_options.go @@ -0,0 +1,53 @@ +package fixtures + +import ( + "github.com/go-git/go-billy/v5" + "github.com/go-git/go-billy/v5/osfs" + "github.com/go-git/go-git-fixtures/v4/internal/tgz" +) + +const ( + useDefaultTempDir = "" + tmpPrefix = "tmp-tgz-" +) + +type Option func(*options) + +type options struct { + fsFactory func() (billy.Filesystem, error) +} + +func newOptions() *options { + return &options{ + fsFactory: tgz.MemFactory, + } +} + +// WithMemFS returns the option of using memfs for the fs created for Fixtures. +func WithMemFS() Option { + return func(o *options) { + o.fsFactory = tgz.MemFactory + } +} + +// WithTargetDir returns the option of using an OS-based filesystem based on a target dir. +// The target dir will be based on the name returned from dirName, which aligns with tempdir +// functions in different testing frameworks (e.g. t.TempDir, c.MkDir). +// +// The caller is responsible for removing the dir from disk. Therefore, it is recommended +// to delegate that to the testing framework: +// +// Go: +// +// WithTargetDir(t.TempDir) +// +// Check Framework: +// +// WithTargetDir(c.Mkdir) +func WithTargetDir(dirName func() string) Option { + return func(o *options) { + o.fsFactory = func() (billy.Filesystem, error) { + return osfs.New(dirName(), osfs.WithChrootOS()), nil + } + } +} diff --git a/fixtures_test.go b/fixtures_test.go index ee00966..799bcd9 100644 --- a/fixtures_test.go +++ b/fixtures_test.go @@ -7,10 +7,15 @@ import ( ) func TestDotGit(t *testing.T) { - fs := Basic().One().DotGit() + fs := Basic().One().DotGit(WithTargetDir(t.TempDir)) files, err := fs.ReadDir("/") assert.NoError(t, err) assert.True(t, len(files) > 1) + + fs = Basic().One().DotGit(WithMemFS()) + files, err = fs.ReadDir("/") + assert.NoError(t, err) + assert.True(t, len(files) > 1) } func TestEmbeddedFiles(t *testing.T) { @@ -26,14 +31,22 @@ func TestEmbeddedFiles(t *testing.T) { } if f.WorktreeHash != "" { - if f.Worktree() == nil { - assert.Fail(t, "failed to get worktree", i) + if f.Worktree(WithMemFS()) == nil { + assert.Fail(t, "[mem] failed to get worktree", i) + } + + if f.Worktree(WithTargetDir(t.TempDir)) == nil { + assert.Fail(t, "[tempdir] failed to get worktree", i) } } if f.DotGitHash != "" { - if f.DotGit() == nil { - assert.Fail(t, "failed to get dotgit", i) + if f.DotGit(WithMemFS()) == nil { + assert.Fail(t, "[mem] failed to get dotgit", i) + } + + if f.DotGit(WithTargetDir(t.TempDir)) == nil { + assert.Fail(t, "[tempdir] failed to get dotgit", i) } } } @@ -42,7 +55,6 @@ func TestEmbeddedFiles(t *testing.T) { func TestRevFiles(t *testing.T) { f := ByTag("packfile-sha256").One() - if f.Rev() == nil { - assert.Fail(t, "failed to get rev file") - } + assert.NotNil(t, f) + assert.NotNil(t, f.Rev(), "failed to get rev file") } diff --git a/go.mod b/go.mod index 2119641..4813223 100644 --- a/go.mod +++ b/go.mod @@ -6,16 +6,12 @@ go 1.20 require ( github.com/go-git/go-billy/v5 v5.5.0 github.com/stretchr/testify v1.9.0 - gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c ) require ( github.com/cyphar/filepath-securejoin v0.2.4 // indirect github.com/davecgh/go-spew v1.1.1 // indirect - github.com/kr/pretty v0.3.1 // indirect - github.com/kr/text v0.2.0 // indirect github.com/pmezard/go-difflib v1.0.0 // indirect - github.com/rogpeppe/go-internal v1.11.0 // indirect golang.org/x/sys v0.14.0 // indirect gopkg.in/yaml.v3 v3.0.1 // indirect ) diff --git a/go.sum b/go.sum index ae8e65e..b90ddcc 100644 --- a/go.sum +++ b/go.sum @@ -1,4 +1,3 @@ -github.com/creack/pty v1.1.9/go.mod h1:oKZEueFk5CKHvIhNR5MUki03XCEU+Q6VDXinZuGJ33E= github.com/cyphar/filepath-securejoin v0.2.4 h1:Ugdm7cg7i6ZK6x3xDF1oEu1nfkyfH53EtKeQYTC3kyg= github.com/cyphar/filepath-securejoin v0.2.4/go.mod h1:aPGpWjXOXUn2NCNjFvBE6aRxGGx79pTxQpKOJNYHHl4= github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c= @@ -6,20 +5,12 @@ github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSs github.com/go-git/go-billy/v5 v5.5.0 h1:yEY4yhzCDuMGSv83oGxiBotRzhwhNr8VZyphhiu+mTU= github.com/go-git/go-billy/v5 v5.5.0/go.mod h1:hmexnoNsr2SJU1Ju67OaNz5ASJY3+sHgFRpCtpDCKow= github.com/google/go-cmp v0.5.9 h1:O2Tfq5qg4qc4AmwVlvv0oLiVAGB7enBSJ2x2DqQFi38= -github.com/kr/pretty v0.2.1/go.mod h1:ipq/a2n7PKx3OHsz4KJII5eveXtPO4qwEXGdVfWzfnI= github.com/kr/pretty v0.3.1 h1:flRD4NNwYAUpkphVc1HcthR4KEIFJ65n8Mw5qdRn3LE= -github.com/kr/pretty v0.3.1/go.mod h1:hoEshYVHaxMs3cyo3Yncou5ZscifuDolrwPKZanG3xk= -github.com/kr/pty v1.1.1/go.mod h1:pFQYn66WHrOpPYNljwOMqo10TkYh1fy3cYio2l3bCsQ= -github.com/kr/text v0.1.0/go.mod h1:4Jbv+DJW3UT/LiOwJeYQe1efqtUx/iVham/4vfdArNI= github.com/kr/text v0.2.0 h1:5Nx0Ya0ZqY2ygV366QzturHI13Jq95ApcVaJBhpS+AY= -github.com/kr/text v0.2.0/go.mod h1:eLer722TekiGuMkidMxC/pM04lWEeraHUUmBw8l2grE= github.com/onsi/gomega v1.27.10 h1:naR28SdDFlqrG6kScpT8VWpu1xWY5nJRCF3XaYyBjhI= -github.com/pkg/diff v0.0.0-20210226163009-20ebb0f2a09e/go.mod h1:pJLUxLENpZxwdsKMEsNbx1VGcRFpLqf3715MtcvvzbA= 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/rogpeppe/go-internal v1.9.0/go.mod h1:WtVeX8xhTBvf0smdhujwtBcq4Qrzq/fJaraNFVN+nFs= github.com/rogpeppe/go-internal v1.11.0 h1:cWPaGQEPrBb5/AsnsZesgZZ9yb1OQ+GOISoDNXVBh4M= -github.com/rogpeppe/go-internal v1.11.0/go.mod h1:ddIwULY96R17DhadqLgMfk9H9tvdUzkipdSkR5nkCZA= github.com/stretchr/testify v1.9.0 h1:HtqpIVDClZ4nwg75+f6Lvsy/wHu+3BoSGCbBAcpTsTg= github.com/stretchr/testify v1.9.0/go.mod h1:r2ic/lqez/lEtzL7wO/rwa5dbSLXVDPFyf8C91i36aY= golang.org/x/net v0.15.0 h1:ugBLEUaxABaB5AJqW9enI0ACdci2RUd4eP51NTBvuJ8= @@ -28,6 +19,5 @@ golang.org/x/sys v0.14.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.13.0 h1:ablQoSUd0tRdKxZewP80B+BaqeKJuVhuRxj/dkrun3k= gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c h1:Hei/4ADfdWqJk1ZMxUNpqntNwaWcugrBjAiHlqqRiVk= -gopkg.in/check.v1 v1.0.0-20201130134442-10cb98267c6c/go.mod h1:JHkPIbrfpd72SG/EVd6muEfDQjcINNoR0C8j2r3qZ4Q= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/embedfs/embed.go b/internal/embedfs/embed.go index 1482d4f..be21ae9 100644 --- a/internal/embedfs/embed.go +++ b/internal/embedfs/embed.go @@ -13,7 +13,6 @@ import ( "sync" "github.com/go-git/go-billy/v5" - "github.com/go-git/go-billy/v5/helper/chroot" "github.com/go-git/go-billy/v5/memfs" ) @@ -21,7 +20,7 @@ type Embed struct { underlying *embed.FS } -func New(efs *embed.FS, path string) billy.Filesystem { +func New(efs *embed.FS) billy.Filesystem { fs := &Embed{ underlying: efs, } @@ -30,7 +29,11 @@ func New(efs *embed.FS, path string) billy.Filesystem { fs.underlying = &embed.FS{} } - return chroot.New(fs, path) + return fs +} + +func (fs *Embed) Root() string { + return "" } func (fs *Embed) Stat(filename string) (os.FileInfo, error) { @@ -74,14 +77,14 @@ func (fs *Embed) OpenFile(filename string, flag int, perm os.FileMode) (billy.Fi return toFile(lazyFunc, fi), nil } -// TODO: use memfs instead +// Join return a path with all elements joined by forward slashes. +// +// This behaviour is OS-agnostic. func (fs *Embed) Join(elem ...string) string { - // Function adapted from Go's filepath.Join for unix: - // https://github.com/golang/go/blob/1ed85ee228023d766b37db056311929c00091c9f/src/path/filepath/path_unix.go#L45 for i, el := range elem { if el != "" { - // reuses filepath.Clean, as it is OS agnostic. - return filepath.Clean(strings.Join(elem[i:], "/")) + clean := filepath.Clean(strings.Join(elem[i:], "/")) + return filepath.ToSlash(clean) } } return "" @@ -104,6 +107,41 @@ func (fs *Embed) ReadDir(path string) ([]os.FileInfo, error) { return entries, nil } +// Chroot is not supported. +// +// Calls will always return billy.ErrNotSupported. +func (fs *Embed) Chroot(path 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 +} + +// Readlink is not supported. +// +// Calls will always return billy.ErrNotSupported. +func (fs *Embed) Readlink(_ string) (string, error) { + return "", billy.ErrNotSupported +} + +// TempFile is not supported. +// +// Calls will always return billy.ErrNotSupported. +func (fs *Embed) TempFile(_, _ string) (billy.File, error) { + return nil, billy.ErrNotSupported +} + +// Symlink is not supported. +// +// Calls will always return billy.ErrReadOnly. +func (fs *Embed) Symlink(_, _ string) error { + return billy.ErrReadOnly +} + // Create is not supported. // // Calls will always return billy.ErrReadOnly. diff --git a/internal/embedfs/embed_test.go b/internal/embedfs/embed_test.go index 7e1dec5..ff3e3c2 100644 --- a/internal/embedfs/embed_test.go +++ b/internal/embedfs/embed_test.go @@ -2,6 +2,7 @@ package embedfs import ( "embed" + "fmt" "io" "os" "testing" @@ -25,12 +26,12 @@ func TestOpen(t *testing.T) { wantErr bool }{ { - name: "empty.txt", + name: "testdata/empty.txt", want: []byte(""), }, { - name: "empty2.txt", - want: []byte("test\n"), + name: "testdata/empty2.txt", + want: []byte("test"), }, { name: "non-existent", @@ -40,7 +41,7 @@ func TestOpen(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&testdataDir, "testdata") + fs := New(&testdataDir) var got []byte f, err := fs.Open(tc.name) @@ -48,12 +49,13 @@ func TestOpen(t *testing.T) { assert.Error(t, err) } else { assert.NoError(t, err) + assert.NotNil(t, f) got, err = io.ReadAll(f) assert.NoError(t, err) } - assert.Equal(t, got, tc.want) + assert.Equal(t, tc.want, got) }) } } @@ -67,48 +69,48 @@ func TestOpenFileFlags(t *testing.T) { }{ { name: "O_CREATE", - file: "empty.txt", + file: "testdata/empty.txt", flag: os.O_CREATE, wantErr: "read-only filesystem", }, { name: "O_WRONLY", - file: "empty.txt", + file: "testdata/empty.txt", flag: os.O_WRONLY, wantErr: "read-only filesystem", }, { name: "O_TRUNC", - file: "empty.txt", + file: "testdata/empty.txt", flag: os.O_TRUNC, wantErr: "read-only filesystem", }, { name: "O_RDWR", - file: "empty.txt", + file: "testdata/empty.txt", flag: os.O_RDWR, wantErr: "read-only filesystem", }, { name: "O_EXCL", - file: "empty.txt", + file: "testdata/empty.txt", flag: os.O_EXCL, wantErr: "read-only filesystem", }, { name: "O_RDONLY", - file: "empty.txt", + file: "testdata/empty.txt", flag: os.O_RDONLY, }, { name: "no flags", - file: "empty.txt", + file: "testdata/empty.txt", }, } for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&testdataDir, "testdata") + fs := New(&testdataDir) _, err := fs.OpenFile(tc.file, tc.flag, 0o700) if tc.wantErr != "" { @@ -140,7 +142,7 @@ func TestStat(t *testing.T) { wantErr: true, }, { - name: "testdata/", + name: "testdata", want: "testdata", isDir: true, }, @@ -148,13 +150,14 @@ func TestStat(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&testdataDir, "") + fs := New(&testdataDir) fi, err := fs.Stat(tc.name) if tc.wantErr { assert.Error(t, err) } else { assert.NoError(t, err) + assert.NotNil(t, fi) assert.Equal(t, tc.want, fi.Name()) assert.Equal(t, tc.isDir, fi.IsDir()) @@ -166,60 +169,32 @@ func TestStat(t *testing.T) { func TestReadDir(t *testing.T) { tests := []struct { name string - chroot string path string fs embed.FS want []string wantErr bool }{ { - name: "singleFile w/ chroot", - chroot: "testdata/", - path: "", - fs: singleFile, - want: []string{"empty.txt"}, - }, - { - name: "singleFile w/o chroot", - chroot: "", - path: "testdata", - fs: singleFile, - want: []string{"empty.txt"}, - }, - { - name: "singleFile return no dir names", - chroot: "", - path: "", - fs: singleFile, - want: []string{}, - wantErr: true, + name: "singleFile", + path: "testdata", + fs: singleFile, + want: []string{"empty.txt"}, }, { name: "empty", - chroot: "", path: "", fs: empty, want: []string{}, wantErr: true, }, - - { - name: "testdataDir w/ chroot", - chroot: "testdata", - path: "", - fs: testdataDir, - want: []string{"empty.txt", "empty2.txt"}, - }, { - name: "testdataDir w/o chroot", - chroot: "", - path: "testdata", - fs: testdataDir, - want: []string{"empty.txt", "empty2.txt"}, + name: "testdataDir w/ path", + path: "testdata", + fs: testdataDir, + want: []string{"empty.txt", "empty2.txt"}, }, { name: "testdataDir return no dir names", - chroot: "", path: "", fs: testdataDir, want: []string{}, @@ -229,7 +204,7 @@ func TestReadDir(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&tc.fs, tc.chroot) + fs := New(&tc.fs) fis, err := fs.ReadDir(tc.path) if tc.wantErr { @@ -255,7 +230,7 @@ func TestReadDir(t *testing.T) { } func TestUnsupported(t *testing.T) { - fs := New(&testdataDir, "") + fs := New(&testdataDir) _, err := fs.Create("test") assert.ErrorIs(t, err, billy.ErrReadOnly) @@ -271,9 +246,9 @@ func TestUnsupported(t *testing.T) { } func TestFileUnsupported(t *testing.T) { - fs := New(&testdataDir, "testdata") + fs := New(&testdataDir) - f, err := fs.Open("empty.txt") + f, err := fs.Open("testdata/empty.txt") assert.NoError(t, err) assert.NotNil(t, f) @@ -285,9 +260,9 @@ func TestFileUnsupported(t *testing.T) { } func TestFileSeek(t *testing.T) { - fs := New(&testdataDir, "testdata") + fs := New(&testdataDir) - f, err := fs.Open("empty2.txt") + f, err := fs.Open("testdata/empty2.txt") assert.NoError(t, err) assert.NotNil(t, f) @@ -296,18 +271,18 @@ func TestFileSeek(t *testing.T) { seekWhence int want string }{ - {seekOff: 4, seekWhence: io.SeekStart, want: "\n"}, - {seekOff: 3, seekWhence: io.SeekStart, want: "t\n"}, - {seekOff: 2, seekWhence: io.SeekStart, want: "st\n"}, - {seekOff: 1, seekWhence: io.SeekStart, want: "est\n"}, - {seekOff: 0, seekWhence: io.SeekStart, want: "test\n"}, + {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: "st\n"}, - {seekOff: -3, seekWhence: io.SeekEnd, want: "st\n"}, + {seekOff: 1, seekWhence: io.SeekCurrent, want: "s"}, + {seekOff: -2, seekWhence: io.SeekEnd, want: "st"}, } - for _, tc := range tests { - t.Run("", func(t *testing.T) { + for i, tc := range tests { + t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { _, err = f.Seek(tc.seekOff, tc.seekWhence) assert.NoError(t, err) @@ -320,3 +295,41 @@ func TestFileSeek(t *testing.T) { }) } } + +func TestJoin(t *testing.T) { + tests := []struct { + name string + path []string + want string + }{ + { + name: "no leading slash", + path: []string{"data", "foo/bar"}, + want: "data/foo/bar", + }, + { + name: "w/ leading slash", + path: []string{"/data", "foo/bar"}, + want: "/data/foo/bar", + }, + { + name: "dot dot", + path: []string{"/data", "../bar"}, + want: "/bar", + }, + { + name: "dot", + path: []string{"/data", "./bar"}, + want: "/data/bar", + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + fs := New(&empty) + + got := fs.Join(tc.path...) + assert.Equal(t, tc.want, got) + }) + } +} diff --git a/internal/embedfs/testdata/empty2.txt b/internal/embedfs/testdata/empty2.txt index 9daeafb..30d74d2 100644 --- a/internal/embedfs/testdata/empty2.txt +++ b/internal/embedfs/testdata/empty2.txt @@ -1 +1 @@ -test +test \ No newline at end of file diff --git a/internal/tgz/tgz.go b/internal/tgz/tgz.go index a0c8506..aa8a11f 100644 --- a/internal/tgz/tgz.go +++ b/internal/tgz/tgz.go @@ -8,60 +8,45 @@ import ( "os" "github.com/go-git/go-billy/v5" - "github.com/go-git/go-billy/v5/util" + "github.com/go-git/go-billy/v5/memfs" ) -const ( - useDefaultTempDir = "" - tmpPrefix = "tmp-tgz-" -) +var MemFactory = func() (billy.Filesystem, error) { + return memfs.New(), nil +} -// Extract decompress a gziped tarball into a new temporal directory -// created just for this purpose. +// Extract decompress a gziped tarball into a billy Filesystem. // // On success, the path of the newly created directory and a nil error // is returned. // // A non-nil error is returned if the method fails to complete. The -// returned path will be an empty string if no information was extracted -// before the error and the temporal directory has not been created. +// returned path will be an empty string if no information was extracted. // Otherwise, a non-empty string with the temporal directory holding // whatever information was extracted before the error is returned. -func Extract(fs billy.Filesystem, tgz string) (d billy.Filesystem, err error, cleanup func()) { - dirName := "" - cleanup = func() { - if dirName != "" { - _ = os.RemoveAll(dirName) - } - } - - f, err := fs.Open(tgz) - if err != nil { - return - } - +func Extract(tgz billy.File, newFS func() (billy.Filesystem, error)) (fs billy.Filesystem, err error) { defer func() { - errClose := f.Close() + errClose := tgz.Close() if err == nil { err = errClose } }() - dirName, err = util.TempDir(fs, useDefaultTempDir, tmpPrefix) + tar, err := zipTarReader(tgz) if err != nil { return } - tar, err := zipTarReader(f) + fs, err = newFS() if err != nil { return } - if err = unTar(fs, tar, dirName); err != nil { + err = unTar(fs, tar) + if err != nil { return } - d, err = fs.Chroot(dirName) return } @@ -74,7 +59,7 @@ func zipTarReader(r io.Reader) (*tar.Reader, error) { return tar.NewReader(zip), nil } -func unTar(fs billy.Filesystem, src *tar.Reader, dstPath string) error { +func unTar(fs billy.Filesystem, src *tar.Reader) error { for { header, err := src.Next() if err != nil { @@ -84,7 +69,7 @@ func unTar(fs billy.Filesystem, src *tar.Reader, dstPath string) error { return err } - dst := dstPath + "/" + header.Name + dst := header.Name mode := os.FileMode(header.Mode) switch header.Typeflag { case tar.TypeDir: @@ -98,7 +83,7 @@ func unTar(fs billy.Filesystem, src *tar.Reader, dstPath string) error { return err } default: - return fmt.Errorf("Unable to untar type : %c in file %s", + return fmt.Errorf("unable to untar type: %c in file %s", header.Typeflag, header.Name) } } diff --git a/internal/tgz/tgz_test.go b/internal/tgz/tgz_test.go index 5bb5e18..f995931 100644 --- a/internal/tgz/tgz_test.go +++ b/internal/tgz/tgz_test.go @@ -4,66 +4,69 @@ import ( "fmt" "os" "path/filepath" - "reflect" - "regexp" - "sort" "testing" + "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/osfs" + "github.com/stretchr/testify/assert" ) func TestExtractError(t *testing.T) { - for i, test := range [...]struct { - tgz string - errRgx *regexp.Regexp + tests := []struct { + tgz string + notFound bool + wantErr string }{ { - tgz: "not-found", - errRgx: regexp.MustCompile("open not-found: (The system cannot find the file specified|no such file).*"), - }, { - tgz: filepath.Join("fixtures", "invalid-gzip.tgz"), - errRgx: regexp.MustCompile("gzip: invalid header"), - }, { - tgz: filepath.Join("fixtures", "not-a-tar.tgz"), - errRgx: regexp.MustCompile("unexpected EOF"), + tgz: "not-found", + notFound: true, + }, + { + tgz: "invalid-gzip.tgz", + wantErr: "gzip: invalid header", + }, + { + tgz: "not-a-tar.tgz", + wantErr: "unexpected EOF", }, - } { - com := fmt.Sprintf("%d) tgz path = %s", i, test.tgz) - _, err, cleanup := Extract(osfs.New(""), test.tgz) - defer cleanup() // Clean up temporary dirs. - - if err == nil { - t.Errorf("%s: expect an error, but none was returned", com) - } else if errorNotMatch(err, test.errRgx) { - t.Errorf("%s:\n\treceived error: %s\n\texpected regexp: %s\n", - com, err, test.errRgx) - } } -} -func errorNotMatch(err error, regexp *regexp.Regexp) bool { - return !regexp.MatchString(err.Error()) + for _, tc := range tests { + t.Run(fmt.Sprintf("tgz path = %s", tc.tgz), func(t *testing.T) { + d, err := os.Getwd() + assert.NoError(t, err) + + source := osfs.New(d + "/fixtures") + f, err := source.Open(tc.tgz) + if tc.notFound { + assert.ErrorIs(t, err, os.ErrNotExist) + } else { + _, err = Extract(f, MemFactory) + assert.ErrorContains(t, err, tc.wantErr) + } + }) + } } func TestExtract(t *testing.T) { - for i, test := range [...]struct { + tests := []struct { tgz string tree []string }{ { - tgz: filepath.Join("fixtures", "test-01.tgz"), + tgz: "test-01.tgz", tree: []string{ "foo.txt", }, }, { - tgz: filepath.Join("fixtures", "test-02.tgz"), + tgz: "test-02.tgz", tree: []string{ "baz.txt", "bla.txt", "foo.txt", }, }, { - tgz: filepath.Join("fixtures", "test-03.tgz"), + tgz: "test-03.tgz", tree: []string{ "bar", filepath.Join("bar", "baz.txt"), @@ -77,54 +80,33 @@ func TestExtract(t *testing.T) { "foo.txt", }, }, - } { - com := fmt.Sprintf("%d) tgz path = %s", i, test.tgz) - - path, err, cleanup := Extract(osfs.New(""), test.tgz) - defer cleanup() // Clean up temporary dirs. - - if err != nil { - t.Fatalf("%s: unexpected error extracting: %s", test.tgz, err) - } - - obt, err := relativeTree(path.Root()) - if err != nil { - t.Errorf("%s: unexpected error calculating relative path: %s", com, err) - } - - sort.Strings(test.tree) - if !reflect.DeepEqual(obt, test.tree) { - t.Fatalf("%s:\n\tobtained: %v\n\t expected: %v", com, obt, test.tree) - } } -} -// relativeTree returns the list of relative paths to the files and -// directories inside a given directory, recursively. -func relativeTree(dir string) ([]string, error) { - dir = filepath.Clean(dir) - - absPaths := []string{} - walkFn := func(path string, _ os.FileInfo, _ error) error { - absPaths = append(absPaths, path) - return nil + factories := []struct { + name string + factory func() (billy.Filesystem, error) + }{ + {name: "mem", factory: MemFactory}, + {name: "osfs-temp", factory: func() (billy.Filesystem, error) { + return osfs.New(t.TempDir(), osfs.WithChrootOS()), nil + }}, } - _ = filepath.Walk(dir, walkFn) - - return toRelative(absPaths[1:], dir) -} - -// toRelative returns the relative paths (form b) of the list of paths in l. -func toRelative(l []string, b string) ([]string, error) { - r := []string{} - for _, p := range l { - rel, err := filepath.Rel(b, p) - if err != nil { - return nil, err + for _, ff := range factories { + for _, tc := range tests { + t.Run(fmt.Sprintf("[%s] tgz path = %s", ff.name, tc.tgz), func(t *testing.T) { + source := osfs.New("fixtures", osfs.WithChrootOS()) + f, err := source.Open(tc.tgz) + assert.NoError(t, err) + + fs, err := Extract(f, ff.factory) + assert.NoError(t, err, "%s: unexpected error extracting: %s", tc.tgz, err) + + for _, path := range tc.tree { + _, err = fs.Stat(path) + assert.NoError(t, err) + } + }) } - r = append(r, rel) } - - return r, nil } From ca306c47291e86a9a16bcc879b6758fa74d735cc Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 8 Apr 2024 22:58:05 +0100 Subject: [PATCH 4/7] Bump module version to v5 Signed-off-by: Paulo Gomes --- fixtures.go | 4 ++-- fixtures_options.go | 2 +- go.mod | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/fixtures.go b/fixtures.go index f3f9a9d..c827bee 100644 --- a/fixtures.go +++ b/fixtures.go @@ -9,8 +9,8 @@ import ( "testing" "github.com/go-git/go-billy/v5" - "github.com/go-git/go-git-fixtures/v4/internal/embedfs" - "github.com/go-git/go-git-fixtures/v4/internal/tgz" + "github.com/go-git/go-git-fixtures/v5/internal/embedfs" + "github.com/go-git/go-git-fixtures/v5/internal/tgz" ) var Filesystem = embedfs.New(&data) diff --git a/fixtures_options.go b/fixtures_options.go index 0867773..79d6ae3 100644 --- a/fixtures_options.go +++ b/fixtures_options.go @@ -3,7 +3,7 @@ package fixtures import ( "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/osfs" - "github.com/go-git/go-git-fixtures/v4/internal/tgz" + "github.com/go-git/go-git-fixtures/v5/internal/tgz" ) const ( diff --git a/go.mod b/go.mod index 4813223..c0b40fc 100644 --- a/go.mod +++ b/go.mod @@ -1,4 +1,4 @@ -module github.com/go-git/go-git-fixtures/v4 +module github.com/go-git/go-git-fixtures/v5 // go-git supports the last 3 stable Go versions. go 1.20 From 0e9e048566503785acedbb9c6df45fe31c68a61d Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Mon, 8 Apr 2024 23:04:36 +0100 Subject: [PATCH 5/7] build: Remove redundant target Signed-off-by: Paulo Gomes --- Makefile | 4 ---- README.md | 2 -- 2 files changed, 6 deletions(-) diff --git a/Makefile b/Makefile index e88eebd..9df829f 100644 --- a/Makefile +++ b/Makefile @@ -1,9 +1,5 @@ -# Go parameters GOCMD = go GOTEST = $(GOCMD) test test: $(GOTEST) ./... - -generate: $(esc) - $(GOCMD) generate diff --git a/README.md b/README.md index 314d7d6..f1a156f 100644 --- a/README.md +++ b/README.md @@ -26,5 +26,3 @@ ls .git/objects/pack/ PackfileHash: "", } ``` - -4. Run `make generate`. From 43baec7717f3b0c31d34537410f8f204b296d274 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 26 Apr 2024 07:39:42 +0100 Subject: [PATCH 6/7] build: Add make validate to run golangci linters Signed-off-by: Paulo Gomes --- .github/workflows/test.yml | 4 ++ .gitignore | 1 + .golangci.yaml | 74 ++++++++++++++++++++++++++++++++++ Makefile | 30 ++++++++++++++ fixtures.go | 3 +- fixtures_options.go | 5 --- fixtures_test.go | 15 +++++-- internal/embedfs/embed.go | 22 +++++----- internal/embedfs/embed_test.go | 68 ++++++++++++++++++------------- internal/tgz/tgz_test.go | 18 +++++---- 10 files changed, 183 insertions(+), 57 deletions(-) create mode 100644 .gitignore create mode 100644 .golangci.yaml diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index b519d72..d0a1173 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -22,5 +22,9 @@ jobs: with: go-version: ${{ matrix.go-version }} + - name: Validate + if: matrix.platform == 'ubuntu-latest' + run: make validate + - name: Test run: make test diff --git a/.gitignore b/.gitignore new file mode 100644 index 0000000..567609b --- /dev/null +++ b/.gitignore @@ -0,0 +1 @@ +build/ diff --git a/.golangci.yaml b/.golangci.yaml new file mode 100644 index 0000000..d4a4826 --- /dev/null +++ b/.golangci.yaml @@ -0,0 +1,74 @@ +linters: + disable-all: true + enable: + - asasalint + - asciicheck + - bidichk + - bodyclose + - containedctx + - contextcheck + - decorder + - dogsled + - dupl + - dupword + - durationcheck + - errcheck + - errchkjson + - errname + - errorlint + - execinquery + - exhaustive + - exportloopref + - forcetypeassert + - ginkgolinter + - gocheckcompilerdirectives + - gochecknoinits + - gochecksumtype + - goconst + - gofmt + - goheader + - goimports + - gomodguard + - goprintffuncname + - gosec + - gosimple + - gosmopolitan + - govet + - grouper + - importas + - ineffassign + - loggercheck + - makezero + - mirror + - misspell + - nakedret + - nestif + - nilerr + - nilnil + - noctx + - nolintlint + - nosprintfhostport + - prealloc + - predeclared + - promlinter + - reassign + - revive + - rowserrcheck + - sloglint + - spancheck + - sqlclosecheck + - stylecheck + - tagalign + - tagliatelle + - tenv + - testableexamples + - testifylint + - thelper + - typecheck + - unconvert + - unparam + - unused + - usestdlibvars + - wastedassign + - whitespace + - zerologlint diff --git a/Makefile b/Makefile index 9df829f..4c40cf3 100644 --- a/Makefile +++ b/Makefile @@ -1,5 +1,35 @@ GOCMD = go GOTEST = $(GOCMD) test +GOLANGCI_VERSION ?= v1.57.2 +TOOLS_BIN := $(shell mkdir -p build/tools && realpath build/tools) + +GOLANGCI = $(TOOLS_BIN)/golangci-lint-$(GOLANGCI_VERSION) +$(GOLANGCI): + rm -f $(TOOLS_BIN)/golangci-lint* + curl -sSfL https://raw.githubusercontent.com/golangci/golangci-lint/$(GOLANGCI_VERSION)/install.sh | sh -s -- -b $(TOOLS_BIN) $(GOLANGCI_VERSION) + mv $(TOOLS_BIN)/golangci-lint $(TOOLS_BIN)/golangci-lint-$(GOLANGCI_VERSION) + test: $(GOTEST) ./... + +validate: validate-lint validate-dirty ## Run validation checks. + +validate-lint: $(GOLANGCI) + $(GOLANGCI) run + +define go-install-tool +@[ -f $(1) ] || { \ +set -e ;\ +echo "Downloading $(2)" ;\ +GOBIN=$(TOOLS_BIN) go install $(2) ;\ +} +endef + +validate-dirty: +ifneq ($(shell git status --porcelain --untracked-files=no),) + @echo worktree is dirty + @git --no-pager status + @git --no-pager diff + @exit 1 +endif diff --git a/fixtures.go b/fixtures.go index c827bee..ff47bd8 100644 --- a/fixtures.go +++ b/fixtures.go @@ -264,7 +264,7 @@ func (f *Fixture) DotGit(opts ...Option) billy.Filesystem { if f.DotGitHash == "" && f.WorktreeHash != "" { fs, _ := f.Worktree(opts...).Chroot(".git") - return fs.(billy.Filesystem) + return fs } file, err := Filesystem.Open(fmt.Sprintf("data/git-%s.tgz", f.DotGitHash)) @@ -329,6 +329,7 @@ type Fixtures []*Fixture // Run calls test within a t.Run for each fixture in g. func (g Fixtures) Run(t *testing.T, test func(*testing.T, *Fixture)) { + t.Helper() for _, f := range g { name := fmt.Sprintf("fixture run (%q, %q)", f.URL, f.Tags) t.Run(name, func(t *testing.T) { diff --git a/fixtures_options.go b/fixtures_options.go index 79d6ae3..379e7fd 100644 --- a/fixtures_options.go +++ b/fixtures_options.go @@ -6,11 +6,6 @@ import ( "github.com/go-git/go-git-fixtures/v5/internal/tgz" ) -const ( - useDefaultTempDir = "" - tmpPrefix = "tmp-tgz-" -) - type Option func(*options) type options struct { diff --git a/fixtures_test.go b/fixtures_test.go index 799bcd9..b5ab1cd 100644 --- a/fixtures_test.go +++ b/fixtures_test.go @@ -4,21 +4,26 @@ import ( "testing" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestDotGit(t *testing.T) { + t.Parallel() + fs := Basic().One().DotGit(WithTargetDir(t.TempDir)) files, err := fs.ReadDir("/") - assert.NoError(t, err) - assert.True(t, len(files) > 1) + require.NoError(t, err) + assert.Greater(t, len(files), 1) fs = Basic().One().DotGit(WithMemFS()) files, err = fs.ReadDir("/") - assert.NoError(t, err) - assert.True(t, len(files) > 1) + require.NoError(t, err) + assert.Greater(t, len(files), 1) } func TestEmbeddedFiles(t *testing.T) { + t.Parallel() + for i, f := range fixtures { if f.PackfileHash != "" { if f.Packfile() == nil { @@ -53,6 +58,8 @@ func TestEmbeddedFiles(t *testing.T) { } func TestRevFiles(t *testing.T) { + t.Parallel() + f := ByTag("packfile-sha256").One() assert.NotNil(t, f) diff --git a/internal/embedfs/embed.go b/internal/embedfs/embed.go index be21ae9..0d924f2 100644 --- a/internal/embedfs/embed.go +++ b/internal/embedfs/embed.go @@ -48,7 +48,7 @@ func (fs *Embed) Open(filename string) (billy.File, error) { return fs.OpenFile(filename, os.O_RDONLY, 0) } -func (fs *Embed) OpenFile(filename string, flag int, perm os.FileMode) (billy.File, error) { +func (fs *Embed) OpenFile(filename string, flag int, _ os.FileMode) (billy.File, error) { if flag&(os.O_CREATE|os.O_WRONLY|os.O_APPEND|os.O_RDWR|os.O_EXCL|os.O_TRUNC) != 0 { return nil, billy.ErrReadOnly } @@ -96,7 +96,7 @@ func (fs *Embed) ReadDir(path string) ([]os.FileInfo, error) { return nil, err } - var entries []os.FileInfo + entries := make([]os.FileInfo, 0, len(e)) for _, f := range e { fi, _ := f.Info() entries = append(entries, fi) @@ -110,7 +110,7 @@ func (fs *Embed) ReadDir(path string) ([]os.FileInfo, error) { // Chroot is not supported. // // Calls will always return billy.ErrNotSupported. -func (fs *Embed) Chroot(path string) (billy.Filesystem, error) { +func (fs *Embed) Chroot(_ string) (billy.Filesystem, error) { return nil, billy.ErrNotSupported } @@ -145,38 +145,36 @@ func (fs *Embed) Symlink(_, _ string) error { // Create is not supported. // // Calls will always return billy.ErrReadOnly. -func (fs *Embed) Create(filename string) (billy.File, error) { +func (fs *Embed) Create(_ string) (billy.File, error) { return nil, billy.ErrReadOnly } // Rename is not supported. // // Calls will always return billy.ErrReadOnly. -func (fs *Embed) Rename(from, to string) error { +func (fs *Embed) Rename(_, _ string) error { return billy.ErrReadOnly } // Remove is not supported. // // Calls will always return billy.ErrReadOnly. -func (fs *Embed) Remove(filename string) error { +func (fs *Embed) Remove(_ string) error { return billy.ErrReadOnly } // MkdirAll is not supported. // // Calls will always return billy.ErrReadOnly. -func (fs *Embed) MkdirAll(filename string, perm os.FileMode) error { +func (fs *Embed) MkdirAll(_ string, _ os.FileMode) error { return billy.ErrReadOnly } func toFile(lazy func() *bytes.Reader, fi fs.FileInfo) billy.File { - new := &file{ + return &file{ lazy: lazy, fi: fi, } - - return new } type file struct { @@ -234,13 +232,13 @@ func (f *file) Unlock() error { // Truncate is not supported. // // Calls will always return billy.ErrReadOnly. -func (f *file) Truncate(size int64) error { +func (f *file) Truncate(_ int64) error { return billy.ErrReadOnly } // Write is not supported. // // Calls will always return billy.ErrReadOnly. -func (f *file) Write(p []byte) (int, error) { +func (f *file) Write(_ []byte) (int, error) { return 0, billy.ErrReadOnly } diff --git a/internal/embedfs/embed_test.go b/internal/embedfs/embed_test.go index ff3e3c2..a4fdd2b 100644 --- a/internal/embedfs/embed_test.go +++ b/internal/embedfs/embed_test.go @@ -9,6 +9,7 @@ import ( "github.com/go-git/go-billy/v5" "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) //go:embed testdata/empty.txt @@ -20,6 +21,8 @@ var testdataDir embed.FS var empty embed.FS func TestOpen(t *testing.T) { + t.Parallel() + tests := []struct { name string want []byte @@ -46,13 +49,13 @@ func TestOpen(t *testing.T) { var got []byte f, err := fs.Open(tc.name) if tc.wantErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, f) got, err = io.ReadAll(f) - assert.NoError(t, err) + require.NoError(t, err) } assert.Equal(t, tc.want, got) @@ -61,6 +64,8 @@ func TestOpen(t *testing.T) { } func TestOpenFileFlags(t *testing.T) { + t.Parallel() + tests := []struct { name string file string @@ -114,15 +119,17 @@ func TestOpenFileFlags(t *testing.T) { _, err := fs.OpenFile(tc.file, tc.flag, 0o700) if tc.wantErr != "" { - assert.ErrorContains(t, err, tc.wantErr) + require.ErrorContains(t, err, tc.wantErr) } else { - assert.NoError(t, err) + require.NoError(t, err) } }) } } func TestStat(t *testing.T) { + t.Parallel() + tests := []struct { name string want string @@ -154,9 +161,9 @@ func TestStat(t *testing.T) { fi, err := fs.Stat(tc.name) if tc.wantErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, fi) assert.Equal(t, tc.want, fi.Name()) @@ -167,36 +174,38 @@ func TestStat(t *testing.T) { } func TestReadDir(t *testing.T) { + t.Parallel() + tests := []struct { name string path string - fs embed.FS + fs *embed.FS want []string wantErr bool }{ { name: "singleFile", path: "testdata", - fs: singleFile, + fs: &singleFile, want: []string{"empty.txt"}, }, { name: "empty", path: "", - fs: empty, + fs: &empty, want: []string{}, wantErr: true, }, { name: "testdataDir w/ path", path: "testdata", - fs: testdataDir, + fs: &testdataDir, want: []string{"empty.txt", "empty2.txt"}, }, { name: "testdataDir return no dir names", path: "", - fs: testdataDir, + fs: &testdataDir, want: []string{}, wantErr: true, }, @@ -204,13 +213,13 @@ func TestReadDir(t *testing.T) { for _, tc := range tests { t.Run(tc.name, func(t *testing.T) { - fs := New(&tc.fs) + fs := New(tc.fs) fis, err := fs.ReadDir(tc.path) if tc.wantErr { - assert.Error(t, err) + require.Error(t, err) } else { - assert.NoError(t, err) + require.NoError(t, err) } assert.Len(t, fis, len(tc.want)) @@ -230,40 +239,44 @@ func TestReadDir(t *testing.T) { } func TestUnsupported(t *testing.T) { + t.Parallel() + fs := New(&testdataDir) _, err := fs.Create("test") - assert.ErrorIs(t, err, billy.ErrReadOnly) + require.ErrorIs(t, err, billy.ErrReadOnly) err = fs.Remove("test") - assert.ErrorIs(t, err, billy.ErrReadOnly) + require.ErrorIs(t, err, billy.ErrReadOnly) err = fs.Rename("test", "test") - assert.ErrorIs(t, err, billy.ErrReadOnly) + require.ErrorIs(t, err, billy.ErrReadOnly) err = fs.MkdirAll("test", 0o700) - assert.ErrorIs(t, err, billy.ErrReadOnly) + require.ErrorIs(t, err, billy.ErrReadOnly) } func TestFileUnsupported(t *testing.T) { + t.Parallel() + fs := New(&testdataDir) f, err := fs.Open("testdata/empty.txt") - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, f) _, err = f.Write([]byte("foo")) - assert.ErrorIs(t, err, billy.ErrReadOnly) + require.ErrorIs(t, err, billy.ErrReadOnly) err = f.Truncate(0) - assert.ErrorIs(t, err, billy.ErrReadOnly) + require.ErrorIs(t, err, billy.ErrReadOnly) } func TestFileSeek(t *testing.T) { fs := New(&testdataDir) f, err := fs.Open("testdata/empty2.txt") - assert.NoError(t, err) + require.NoError(t, err) assert.NotNil(t, f) tests := []struct { @@ -283,13 +296,12 @@ func TestFileSeek(t *testing.T) { for i, tc := range tests { t.Run(fmt.Sprintf("#%d", i), func(t *testing.T) { - _, err = f.Seek(tc.seekOff, tc.seekWhence) - assert.NoError(t, err) + require.NoError(t, err) data := make([]byte, len(tc.want)) n, err := f.Read(data) - assert.NoError(t, err) + require.NoError(t, err) assert.Equal(t, len(tc.want), n) assert.Equal(t, []byte(tc.want), data) }) @@ -313,12 +325,12 @@ func TestJoin(t *testing.T) { want: "/data/foo/bar", }, { - name: "dot dot", + name: "..", path: []string{"/data", "../bar"}, want: "/bar", }, { - name: "dot", + name: ".", path: []string{"/data", "./bar"}, want: "/data/bar", }, diff --git a/internal/tgz/tgz_test.go b/internal/tgz/tgz_test.go index f995931..9071f49 100644 --- a/internal/tgz/tgz_test.go +++ b/internal/tgz/tgz_test.go @@ -8,10 +8,12 @@ import ( "github.com/go-git/go-billy/v5" "github.com/go-git/go-billy/v5/osfs" - "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" ) func TestExtractError(t *testing.T) { + t.Parallel() + tests := []struct { tgz string notFound bool @@ -34,21 +36,23 @@ func TestExtractError(t *testing.T) { for _, tc := range tests { t.Run(fmt.Sprintf("tgz path = %s", tc.tgz), func(t *testing.T) { d, err := os.Getwd() - assert.NoError(t, err) + require.NoError(t, err) source := osfs.New(d + "/fixtures") f, err := source.Open(tc.tgz) if tc.notFound { - assert.ErrorIs(t, err, os.ErrNotExist) + require.ErrorIs(t, err, os.ErrNotExist) } else { _, err = Extract(f, MemFactory) - assert.ErrorContains(t, err, tc.wantErr) + require.ErrorContains(t, err, tc.wantErr) } }) } } func TestExtract(t *testing.T) { + t.Parallel() + tests := []struct { tgz string tree []string @@ -97,14 +101,14 @@ func TestExtract(t *testing.T) { t.Run(fmt.Sprintf("[%s] tgz path = %s", ff.name, tc.tgz), func(t *testing.T) { source := osfs.New("fixtures", osfs.WithChrootOS()) f, err := source.Open(tc.tgz) - assert.NoError(t, err) + require.NoError(t, err) fs, err := Extract(f, ff.factory) - assert.NoError(t, err, "%s: unexpected error extracting: %s", tc.tgz, err) + require.NoError(t, err, "%s: unexpected error extracting: %s", tc.tgz, err) for _, path := range tc.tree { _, err = fs.Stat(path) - assert.NoError(t, err) + require.NoError(t, err) } }) } From 0cecebcbbe7d7e415966e56a2ffacefc273b0c21 Mon Sep 17 00:00:00 2001 From: Paulo Gomes Date: Fri, 26 Apr 2024 07:48:32 +0100 Subject: [PATCH 7/7] Simplify tgz.Extract Signed-off-by: Paulo Gomes --- fixtures.go | 14 ++++++++++++-- internal/tgz/tgz.go | 17 +++-------------- internal/tgz/tgz_test.go | 14 ++++++++++++-- 3 files changed, 27 insertions(+), 18 deletions(-) diff --git a/fixtures.go b/fixtures.go index ff47bd8..2d85588 100644 --- a/fixtures.go +++ b/fixtures.go @@ -272,7 +272,12 @@ func (f *Fixture) DotGit(opts ...Option) billy.Filesystem { panic(err) } - fs, err := tgz.Extract(file, o.fsFactory) + fs, err := o.fsFactory() + if err != nil { + panic(err) + } + + err = tgz.Extract(file, fs) if err != nil { panic(err) } @@ -317,7 +322,12 @@ func (f *Fixture) Worktree(opts ...Option) billy.Filesystem { panic(err) } - fs, err := tgz.Extract(file, o.fsFactory) + fs, err := o.fsFactory() + if err != nil { + panic(err) + } + + err = tgz.Extract(file, fs) if err != nil { panic(err) } diff --git a/internal/tgz/tgz.go b/internal/tgz/tgz.go index aa8a11f..baa730b 100644 --- a/internal/tgz/tgz.go +++ b/internal/tgz/tgz.go @@ -15,16 +15,10 @@ var MemFactory = func() (billy.Filesystem, error) { return memfs.New(), nil } -// Extract decompress a gziped tarball into a billy Filesystem. +// Extract decompress a gziped tarball into the fs billy.Filesystem. // -// On success, the path of the newly created directory and a nil error -// is returned. -// -// A non-nil error is returned if the method fails to complete. The -// returned path will be an empty string if no information was extracted. -// Otherwise, a non-empty string with the temporal directory holding -// whatever information was extracted before the error is returned. -func Extract(tgz billy.File, newFS func() (billy.Filesystem, error)) (fs billy.Filesystem, err error) { +// A non-nil error is returned if the method fails to complete. +func Extract(tgz billy.File, fs billy.Filesystem) (err error) { defer func() { errClose := tgz.Close() if err == nil { @@ -37,11 +31,6 @@ func Extract(tgz billy.File, newFS func() (billy.Filesystem, error)) (fs billy.F return } - fs, err = newFS() - if err != nil { - return - } - err = unTar(fs, tar) if err != nil { return diff --git a/internal/tgz/tgz_test.go b/internal/tgz/tgz_test.go index 9071f49..5e70cc1 100644 --- a/internal/tgz/tgz_test.go +++ b/internal/tgz/tgz_test.go @@ -43,7 +43,12 @@ func TestExtractError(t *testing.T) { if tc.notFound { require.ErrorIs(t, err, os.ErrNotExist) } else { - _, err = Extract(f, MemFactory) + fs, err := MemFactory() + if err != nil { + panic(err) + } + + err = Extract(f, fs) require.ErrorContains(t, err, tc.wantErr) } }) @@ -103,7 +108,12 @@ func TestExtract(t *testing.T) { f, err := source.Open(tc.tgz) require.NoError(t, err) - fs, err := Extract(f, ff.factory) + fs, err := ff.factory() + if err != nil { + panic(err) + } + + err = Extract(f, fs) require.NoError(t, err, "%s: unexpected error extracting: %s", tc.tgz, err) for _, path := range tc.tree {