From 7d732ceba8c5229f5228762db0d123764e290ddd Mon Sep 17 00:00:00 2001 From: Pieter Noordhuis Date: Fri, 15 Nov 2024 16:37:21 +0100 Subject: [PATCH] Consolidate test helpers for `io/fs` (#1906) ## Changes We had a number of copies of test helpers for `io/fs` in the repository. This change consolidates all of them to use the `libs/fakefs` package. ## Tests Unit tests pass. --- cmd/fs/helpers_test.go | 3 +- libs/fakefs/fakefs.go | 36 +++++++++- libs/fakefs/fakefs_test.go | 38 ++++++++++ libs/filer/completer/completer_test.go | 3 +- libs/filer/fake_filer.go | 60 +++------------- libs/filer/fake_filer_test.go | 98 ++++++++++++++++++++++++++ libs/filer/fs_test.go | 3 +- libs/notebook/detect_test.go | 21 ++++-- libs/notebook/fakefs_test.go | 77 -------------------- 9 files changed, 201 insertions(+), 138 deletions(-) create mode 100644 libs/fakefs/fakefs_test.go create mode 100644 libs/filer/fake_filer_test.go delete mode 100644 libs/notebook/fakefs_test.go diff --git a/cmd/fs/helpers_test.go b/cmd/fs/helpers_test.go index 10b4aa16..a01035cc 100644 --- a/cmd/fs/helpers_test.go +++ b/cmd/fs/helpers_test.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/fakefs" "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/spf13/cobra" @@ -84,7 +85,7 @@ func setupTest(t *testing.T) (*validArgs, *cobra.Command, *mocks.MockWorkspaceCl cmd, m := setupCommand(t) fakeFilerForPath := func(ctx context.Context, fullPath string) (filer.Filer, string, error) { - fakeFiler := filer.NewFakeFiler(map[string]filer.FakeFileInfo{ + fakeFiler := filer.NewFakeFiler(map[string]fakefs.FileInfo{ "dir": {FakeName: "root", FakeDir: true}, "dir/dirA": {FakeDir: true}, "dir/dirB": {FakeDir: true}, diff --git a/libs/fakefs/fakefs.go b/libs/fakefs/fakefs.go index 2f475697..a8d5eb87 100644 --- a/libs/fakefs/fakefs.go +++ b/libs/fakefs/fakefs.go @@ -1,18 +1,21 @@ package fakefs import ( + "fmt" "io/fs" "time" ) +var ErrNotImplemented = fmt.Errorf("not implemented") + // DirEntry is a fake implementation of [fs.DirEntry]. type DirEntry struct { - FileInfo + fs.FileInfo } func (entry DirEntry) Type() fs.FileMode { typ := fs.ModePerm - if entry.FakeDir { + if entry.IsDir() { typ |= fs.ModeDir } return typ @@ -53,3 +56,32 @@ func (info FileInfo) IsDir() bool { func (info FileInfo) Sys() any { return nil } + +// File is a fake implementation of [fs.File]. +type File struct { + FileInfo fs.FileInfo +} + +func (f File) Close() error { + return nil +} + +func (f File) Read(p []byte) (n int, err error) { + return 0, ErrNotImplemented +} + +func (f File) Stat() (fs.FileInfo, error) { + return f.FileInfo, nil +} + +// FS is a fake implementation of [fs.FS]. +type FS map[string]fs.File + +func (f FS) Open(name string) (fs.File, error) { + e, ok := f[name] + if !ok { + return nil, fs.ErrNotExist + } + + return e, nil +} diff --git a/libs/fakefs/fakefs_test.go b/libs/fakefs/fakefs_test.go new file mode 100644 index 00000000..b8919020 --- /dev/null +++ b/libs/fakefs/fakefs_test.go @@ -0,0 +1,38 @@ +package fakefs + +import ( + "io/fs" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestFile(t *testing.T) { + var fakefile fs.File = File{ + FileInfo: FileInfo{ + FakeName: "file", + }, + } + + _, err := fakefile.Read([]byte{}) + assert.ErrorIs(t, err, ErrNotImplemented) + + fi, err := fakefile.Stat() + assert.NoError(t, err) + assert.Equal(t, "file", fi.Name()) + + err = fakefile.Close() + assert.NoError(t, err) +} + +func TestFS(t *testing.T) { + var fakefs fs.FS = FS{ + "file": File{}, + } + + _, err := fakefs.Open("doesntexist") + assert.ErrorIs(t, err, fs.ErrNotExist) + + _, err = fakefs.Open("file") + assert.NoError(t, err) +} diff --git a/libs/filer/completer/completer_test.go b/libs/filer/completer/completer_test.go index c533f0b6..d284447b 100644 --- a/libs/filer/completer/completer_test.go +++ b/libs/filer/completer/completer_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/databricks/cli/cmd/root" + "github.com/databricks/cli/libs/fakefs" "github.com/databricks/cli/libs/filer" "github.com/databricks/databricks-sdk-go/experimental/mocks" "github.com/spf13/cobra" @@ -17,7 +18,7 @@ func setupCompleter(t *testing.T, onlyDirs bool) *completer { // Needed to make type context.valueCtx for mockFilerForPath ctx = root.SetWorkspaceClient(ctx, mocks.NewMockWorkspaceClient(t).WorkspaceClient) - fakeFiler := filer.NewFakeFiler(map[string]filer.FakeFileInfo{ + fakeFiler := filer.NewFakeFiler(map[string]fakefs.FileInfo{ "dir": {FakeName: "root", FakeDir: true}, "dir/dirA": {FakeDir: true}, "dir/dirB": {FakeDir: true}, diff --git a/libs/filer/fake_filer.go b/libs/filer/fake_filer.go index 0e650ff6..76b8bcd9 100644 --- a/libs/filer/fake_filer.go +++ b/libs/filer/fake_filer.go @@ -8,58 +8,12 @@ import ( "path" "sort" "strings" - "time" + + "github.com/databricks/cli/libs/fakefs" ) -type FakeDirEntry struct { - FakeFileInfo -} - -func (entry FakeDirEntry) Type() fs.FileMode { - typ := fs.ModePerm - if entry.FakeDir { - typ |= fs.ModeDir - } - return typ -} - -func (entry FakeDirEntry) Info() (fs.FileInfo, error) { - return entry.FakeFileInfo, nil -} - -type FakeFileInfo struct { - FakeName string - FakeSize int64 - FakeDir bool - FakeMode fs.FileMode -} - -func (info FakeFileInfo) Name() string { - return info.FakeName -} - -func (info FakeFileInfo) Size() int64 { - return info.FakeSize -} - -func (info FakeFileInfo) Mode() fs.FileMode { - return info.FakeMode -} - -func (info FakeFileInfo) ModTime() time.Time { - return time.Now() -} - -func (info FakeFileInfo) IsDir() bool { - return info.FakeDir -} - -func (info FakeFileInfo) Sys() any { - return nil -} - type FakeFiler struct { - entries map[string]FakeFileInfo + entries map[string]fakefs.FileInfo } func (f *FakeFiler) Write(ctx context.Context, p string, reader io.Reader, mode ...WriteMode) error { @@ -97,7 +51,7 @@ func (f *FakeFiler) ReadDir(ctx context.Context, p string) ([]fs.DirEntry, error continue } - out = append(out, FakeDirEntry{v}) + out = append(out, fakefs.DirEntry{FileInfo: v}) } sort.Slice(out, func(i, j int) bool { return out[i].Name() < out[j].Name() }) @@ -117,7 +71,11 @@ func (f *FakeFiler) Stat(ctx context.Context, path string) (fs.FileInfo, error) return entry, nil } -func NewFakeFiler(entries map[string]FakeFileInfo) *FakeFiler { +// NewFakeFiler creates a new fake [Filer] instance with the given entries. +// It sets the [Name] field of each entry to the base name of the path. +// +// This is meant to be used in tests. +func NewFakeFiler(entries map[string]fakefs.FileInfo) *FakeFiler { fakeFiler := &FakeFiler{ entries: entries, } diff --git a/libs/filer/fake_filer_test.go b/libs/filer/fake_filer_test.go new file mode 100644 index 00000000..fb536488 --- /dev/null +++ b/libs/filer/fake_filer_test.go @@ -0,0 +1,98 @@ +package filer + +import ( + "context" + "io" + "io/fs" + "testing" + + "github.com/databricks/cli/libs/fakefs" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +func TestFakeFiler_Read(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "file": {}, + }) + + ctx := context.Background() + r, err := f.Read(ctx, "file") + require.NoError(t, err) + contents, err := io.ReadAll(r) + require.NoError(t, err) + + // Contents of every file is "foo". + assert.Equal(t, "foo", string(contents)) +} + +func TestFakeFiler_Read_NotFound(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "foo": {}, + }) + + ctx := context.Background() + _, err := f.Read(ctx, "bar") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestFakeFiler_ReadDir_NotFound(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "dir1": {FakeDir: true}, + }) + + ctx := context.Background() + _, err := f.ReadDir(ctx, "dir2") + assert.ErrorIs(t, err, fs.ErrNotExist) +} + +func TestFakeFiler_ReadDir_NotADirectory(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "file": {}, + }) + + ctx := context.Background() + _, err := f.ReadDir(ctx, "file") + assert.ErrorIs(t, err, fs.ErrInvalid) +} + +func TestFakeFiler_ReadDir(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "dir1": {FakeDir: true}, + "dir1/file2": {}, + "dir1/dir2": {FakeDir: true}, + }) + + ctx := context.Background() + entries, err := f.ReadDir(ctx, "dir1/") + require.NoError(t, err) + require.Len(t, entries, 2) + + // The entries are sorted by name. + assert.Equal(t, "dir2", entries[0].Name()) + assert.True(t, entries[0].IsDir()) + assert.Equal(t, "file2", entries[1].Name()) + assert.False(t, entries[1].IsDir()) +} + +func TestFakeFiler_Stat(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "file": {}, + }) + + ctx := context.Background() + info, err := f.Stat(ctx, "file") + require.NoError(t, err) + + assert.Equal(t, "file", info.Name()) +} + +func TestFakeFiler_Stat_NotFound(t *testing.T) { + f := NewFakeFiler(map[string]fakefs.FileInfo{ + "foo": {}, + }) + + ctx := context.Background() + _, err := f.Stat(ctx, "bar") + assert.ErrorIs(t, err, fs.ErrNotExist) +} diff --git a/libs/filer/fs_test.go b/libs/filer/fs_test.go index a74c10f0..849cf6f7 100644 --- a/libs/filer/fs_test.go +++ b/libs/filer/fs_test.go @@ -6,6 +6,7 @@ import ( "io/fs" "testing" + "github.com/databricks/cli/libs/fakefs" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" ) @@ -35,7 +36,7 @@ func TestFsDirImplementsFsReadDirFile(t *testing.T) { } func fakeFS() fs.FS { - fakeFiler := NewFakeFiler(map[string]FakeFileInfo{ + fakeFiler := NewFakeFiler(map[string]fakefs.FileInfo{ ".": {FakeName: "root", FakeDir: true}, "dirA": {FakeDir: true}, "dirB": {FakeDir: true}, diff --git a/libs/notebook/detect_test.go b/libs/notebook/detect_test.go index ad89d6dd..786c7e39 100644 --- a/libs/notebook/detect_test.go +++ b/libs/notebook/detect_test.go @@ -7,6 +7,7 @@ import ( "path/filepath" "testing" + "github.com/databricks/cli/libs/fakefs" "github.com/databricks/databricks-sdk-go/service/workspace" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -100,11 +101,21 @@ func TestDetectFileWithLongHeader(t *testing.T) { assert.False(t, nb) } +type fileInfoWithWorkspaceInfo struct { + fakefs.FileInfo + + oi workspace.ObjectInfo +} + +func (f fileInfoWithWorkspaceInfo) WorkspaceObjectInfo() workspace.ObjectInfo { + return f.oi +} + func TestDetectWithObjectInfo(t *testing.T) { - fakeFS := &fakeFS{ - fakeFile{ - fakeFileInfo{ - workspace.ObjectInfo{ + fakefs := fakefs.FS{ + "file.py": fakefs.File{ + FileInfo: fileInfoWithWorkspaceInfo{ + oi: workspace.ObjectInfo{ ObjectType: workspace.ObjectTypeNotebook, Language: workspace.LanguagePython, }, @@ -112,7 +123,7 @@ func TestDetectWithObjectInfo(t *testing.T) { }, } - nb, lang, err := DetectWithFS(fakeFS, "doesntmatter") + nb, lang, err := DetectWithFS(fakefs, "file.py") require.NoError(t, err) assert.True(t, nb) assert.Equal(t, workspace.LanguagePython, lang) diff --git a/libs/notebook/fakefs_test.go b/libs/notebook/fakefs_test.go deleted file mode 100644 index 4ac135dd..00000000 --- a/libs/notebook/fakefs_test.go +++ /dev/null @@ -1,77 +0,0 @@ -package notebook - -import ( - "fmt" - "io/fs" - "time" - - "github.com/databricks/databricks-sdk-go/service/workspace" -) - -type fakeFS struct { - fakeFile -} - -type fakeFile struct { - fakeFileInfo -} - -func (f fakeFile) Close() error { - return nil -} - -func (f fakeFile) Read(p []byte) (n int, err error) { - return 0, fmt.Errorf("not implemented") -} - -func (f fakeFile) Stat() (fs.FileInfo, error) { - return f.fakeFileInfo, nil -} - -type fakeFileInfo struct { - oi workspace.ObjectInfo -} - -func (f fakeFileInfo) WorkspaceObjectInfo() workspace.ObjectInfo { - return f.oi -} - -func (f fakeFileInfo) Name() string { - return "" -} - -func (f fakeFileInfo) Size() int64 { - return 0 -} - -func (f fakeFileInfo) Mode() fs.FileMode { - return 0 -} - -func (f fakeFileInfo) ModTime() time.Time { - return time.Time{} -} - -func (f fakeFileInfo) IsDir() bool { - return false -} - -func (f fakeFileInfo) Sys() any { - return nil -} - -func (f fakeFS) Open(name string) (fs.File, error) { - return f.fakeFile, nil -} - -func (f fakeFS) Stat(name string) (fs.FileInfo, error) { - panic("not implemented") -} - -func (f fakeFS) ReadDir(name string) ([]fs.DirEntry, error) { - panic("not implemented") -} - -func (f fakeFS) ReadFile(name string) ([]byte, error) { - panic("not implemented") -}