diff --git a/bundle/config/mutator/configure_wsfs.go b/bundle/config/mutator/configure_wsfs.go index 1d1bec582..110e1a381 100644 --- a/bundle/config/mutator/configure_wsfs.go +++ b/bundle/config/mutator/configure_wsfs.go @@ -5,14 +5,12 @@ import ( "strings" "github.com/databricks/cli/bundle" + "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/diag" - "github.com/databricks/cli/libs/env" "github.com/databricks/cli/libs/filer" "github.com/databricks/cli/libs/vfs" ) -const envDatabricksRuntimeVersion = "DATABRICKS_RUNTIME_VERSION" - type configureWSFS struct{} func ConfigureWSFS() bundle.Mutator { @@ -32,7 +30,7 @@ func (m *configureWSFS) Apply(ctx context.Context, b *bundle.Bundle) diag.Diagno } // The executable must be running on DBR. - if _, ok := env.Lookup(ctx, envDatabricksRuntimeVersion); !ok { + if !dbr.RunsOnRuntime(ctx) { return nil } diff --git a/bundle/config/mutator/configure_wsfs_test.go b/bundle/config/mutator/configure_wsfs_test.go new file mode 100644 index 000000000..6f76293e0 --- /dev/null +++ b/bundle/config/mutator/configure_wsfs_test.go @@ -0,0 +1,65 @@ +package mutator_test + +import ( + "context" + "runtime" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/libs/dbr" + "github.com/databricks/cli/libs/vfs" + "github.com/databricks/databricks-sdk-go/config" + "github.com/databricks/databricks-sdk-go/experimental/mocks" + "github.com/stretchr/testify/assert" +) + +func mockBundleForConfigureWSFS(t *testing.T, syncRootPath string) *bundle.Bundle { + // The native path of the sync root on Windows will never match the /Workspace prefix, + // so the test case for nominal behavior will always fail. + if runtime.GOOS == "windows" { + t.Skip("this test is not applicable on Windows") + } + + b := &bundle.Bundle{ + SyncRoot: vfs.MustNew(syncRootPath), + } + + w := mocks.NewMockWorkspaceClient(t) + w.WorkspaceClient.Config = &config.Config{} + b.SetWorkpaceClient(w.WorkspaceClient) + + return b +} + +func TestConfigureWSFS_SkipsIfNotWorkspacePrefix(t *testing.T) { + b := mockBundleForConfigureWSFS(t, "/foo") + originalSyncRoot := b.SyncRoot + + ctx := context.Background() + diags := bundle.Apply(ctx, b, mutator.ConfigureWSFS()) + assert.Empty(t, diags) + assert.Equal(t, originalSyncRoot, b.SyncRoot) +} + +func TestConfigureWSFS_SkipsIfNotRunningOnRuntime(t *testing.T) { + b := mockBundleForConfigureWSFS(t, "/Workspace/foo") + originalSyncRoot := b.SyncRoot + + ctx := context.Background() + ctx = dbr.MockRuntime(ctx, false) + diags := bundle.Apply(ctx, b, mutator.ConfigureWSFS()) + assert.Empty(t, diags) + assert.Equal(t, originalSyncRoot, b.SyncRoot) +} + +func TestConfigureWSFS_SwapSyncRoot(t *testing.T) { + b := mockBundleForConfigureWSFS(t, "/Workspace/foo") + originalSyncRoot := b.SyncRoot + + ctx := context.Background() + ctx = dbr.MockRuntime(ctx, true) + diags := bundle.Apply(ctx, b, mutator.ConfigureWSFS()) + assert.Empty(t, diags) + assert.NotEqual(t, originalSyncRoot, b.SyncRoot) +} diff --git a/bundle/config/mutator/initialize_urls_test.go b/bundle/config/mutator/initialize_urls_test.go index 16b67dac8..ec4e790c4 100644 --- a/bundle/config/mutator/initialize_urls_test.go +++ b/bundle/config/mutator/initialize_urls_test.go @@ -65,9 +65,8 @@ func TestInitializeURLs(t *testing.T) { }, QualityMonitors: map[string]*resources.QualityMonitor{ "qualityMonitor1": { - CreateMonitor: &catalog.CreateMonitor{ - TableName: "catalog.schema.qualityMonitor1", - }, + TableName: "catalog.schema.qualityMonitor1", + CreateMonitor: &catalog.CreateMonitor{}, }, }, Schemas: map[string]*resources.Schema{ diff --git a/bundle/config/mutator/prepend_workspace_prefix.go b/bundle/config/mutator/prepend_workspace_prefix.go index de71bf7fd..e0be2572d 100644 --- a/bundle/config/mutator/prepend_workspace_prefix.go +++ b/bundle/config/mutator/prepend_workspace_prefix.go @@ -44,6 +44,11 @@ func (m *prependWorkspacePrefix) Apply(ctx context.Context, b *bundle.Bundle) di return dyn.InvalidValue, fmt.Errorf("expected string, got %s", v.Kind()) } + // Skip prefixing if the path does not start with /, it might be variable reference or smth else. + if !strings.HasPrefix(path, "/") { + return pv, nil + } + for _, prefix := range skipPrefixes { if strings.HasPrefix(path, prefix) { return pv, nil diff --git a/bundle/config/mutator/prepend_workspace_prefix_test.go b/bundle/config/mutator/prepend_workspace_prefix_test.go index 6fbadec56..31393e6bd 100644 --- a/bundle/config/mutator/prepend_workspace_prefix_test.go +++ b/bundle/config/mutator/prepend_workspace_prefix_test.go @@ -31,6 +31,14 @@ func TestPrependWorkspacePrefix(t *testing.T) { path: "/Volumes/Users/test", expected: "/Volumes/Users/test", }, + { + path: "~/test", + expected: "~/test", + }, + { + path: "${workspace.file_path}/test", + expected: "${workspace.file_path}/test", + }, } for _, tc := range testCases { diff --git a/bundle/config/mutator/process_target_mode_test.go b/bundle/config/mutator/process_target_mode_test.go index b694f627a..4135d5fdf 100644 --- a/bundle/config/mutator/process_target_mode_test.go +++ b/bundle/config/mutator/process_target_mode_test.go @@ -102,16 +102,23 @@ func mockBundle(mode config.Mode) *bundle.Bundle { "registeredmodel1": {CreateRegisteredModelRequest: &catalog.CreateRegisteredModelRequest{Name: "registeredmodel1"}}, }, QualityMonitors: map[string]*resources.QualityMonitor{ - "qualityMonitor1": {CreateMonitor: &catalog.CreateMonitor{TableName: "qualityMonitor1"}}, - "qualityMonitor2": { + "qualityMonitor1": { + TableName: "qualityMonitor1", CreateMonitor: &catalog.CreateMonitor{ - TableName: "qualityMonitor2", - Schedule: &catalog.MonitorCronSchedule{}, + OutputSchemaName: "catalog.schema", + }, + }, + "qualityMonitor2": { + TableName: "qualityMonitor2", + CreateMonitor: &catalog.CreateMonitor{ + OutputSchemaName: "catalog.schema", + Schedule: &catalog.MonitorCronSchedule{}, }, }, "qualityMonitor3": { + TableName: "qualityMonitor3", CreateMonitor: &catalog.CreateMonitor{ - TableName: "qualityMonitor3", + OutputSchemaName: "catalog.schema", Schedule: &catalog.MonitorCronSchedule{ PauseStatus: catalog.MonitorCronSchedulePauseStatusUnpaused, }, diff --git a/bundle/config/resources/quality_monitor.go b/bundle/config/resources/quality_monitor.go index 3c823e625..30ec4f918 100644 --- a/bundle/config/resources/quality_monitor.go +++ b/bundle/config/resources/quality_monitor.go @@ -13,17 +13,15 @@ import ( ) type QualityMonitor struct { - // Represents the Input Arguments for Terraform and will get - // converted to a HCL representation for CRUD - *catalog.CreateMonitor - - // This represents the id which is the full name of the monitor - // (catalog_name.schema_name.table_name) that can be used - // as a reference in other resources. This value is returned by terraform. - ID string `json:"id,omitempty" bundle:"readonly"` - + ID string `json:"id,omitempty" bundle:"readonly"` ModifiedStatus ModifiedStatus `json:"modified_status,omitempty" bundle:"internal"` URL string `json:"url,omitempty" bundle:"internal"` + + // The table name is a required field but not included as a JSON field in [catalog.CreateMonitor]. + TableName string `json:"table_name"` + + // This struct defines the creation payload for a monitor. + *catalog.CreateMonitor } func (s *QualityMonitor) UnmarshalJSON(b []byte) error { diff --git a/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go b/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go index 50bfce7a0..f71abf43c 100644 --- a/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go +++ b/bundle/deploy/terraform/tfdyn/convert_quality_monitor_test.go @@ -15,8 +15,8 @@ import ( func TestConvertQualityMonitor(t *testing.T) { var src = resources.QualityMonitor{ + TableName: "test_table_name", CreateMonitor: &catalog.CreateMonitor{ - TableName: "test_table_name", AssetsDir: "assets_dir", OutputSchemaName: "output_schema_name", InferenceLog: &catalog.MonitorInferenceLog{ diff --git a/bundle/internal/schema/testdata/pass/quality_monitor.yml b/bundle/internal/schema/testdata/pass/quality_monitor.yml index a9be59329..79c4dd69b 100644 --- a/bundle/internal/schema/testdata/pass/quality_monitor.yml +++ b/bundle/internal/schema/testdata/pass/quality_monitor.yml @@ -4,6 +4,7 @@ bundle: resources: quality_monitors: myqualitymonitor: + table_name: catalog.schema.quality_monitor inference_log: granularities: - a diff --git a/bundle/schema/jsonschema.json b/bundle/schema/jsonschema.json index dc0d7f953..703daafeb 100644 --- a/bundle/schema/jsonschema.json +++ b/bundle/schema/jsonschema.json @@ -684,6 +684,9 @@ "description": "Configuration for monitoring snapshot tables.", "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/catalog.MonitorSnapshot" }, + "table_name": { + "$ref": "#/$defs/string" + }, "time_series": { "description": "Configuration for monitoring time series tables.", "$ref": "#/$defs/github.com/databricks/databricks-sdk-go/service/catalog.MonitorTimeSeries" @@ -695,6 +698,7 @@ }, "additionalProperties": false, "required": [ + "table_name", "assets_dir", "output_schema_name" ] diff --git a/cmd/fs/helpers_test.go b/cmd/fs/helpers_test.go index 10b4aa160..a01035cc7 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/cmd/root/root.go b/cmd/root/root.go index 7059586f3..e6f66f126 100644 --- a/cmd/root/root.go +++ b/cmd/root/root.go @@ -11,6 +11,7 @@ import ( "github.com/databricks/cli/internal/build" "github.com/databricks/cli/libs/cmdio" + "github.com/databricks/cli/libs/dbr" "github.com/databricks/cli/libs/log" "github.com/spf13/cobra" ) @@ -73,6 +74,9 @@ func New(ctx context.Context) *cobra.Command { // get the context back ctx = cmd.Context() + // Detect if the CLI is running on DBR and store this on the context. + ctx = dbr.DetectRuntime(ctx) + // Configure our user agent with the command that's about to be executed. ctx = withCommandInUserAgent(ctx, cmd) ctx = withCommandExecIdInUserAgent(ctx) diff --git a/libs/dbr/context.go b/libs/dbr/context.go new file mode 100644 index 000000000..7512c0fe2 --- /dev/null +++ b/libs/dbr/context.go @@ -0,0 +1,49 @@ +package dbr + +import "context" + +// key is a package-local type to use for context keys. +// +// Using an unexported type for context keys prevents key collisions across +// packages since external packages cannot create values of this type. +type key int + +const ( + // dbrKey is the context key for the detection result. + // The value of 1 is arbitrary and can be any number. + // Other keys in the same package must have different values. + dbrKey = key(1) +) + +// DetectRuntime detects whether or not the current +// process is running inside a Databricks Runtime environment. +// It return a new context with the detection result set. +func DetectRuntime(ctx context.Context) context.Context { + if v := ctx.Value(dbrKey); v != nil { + panic("dbr.DetectRuntime called twice on the same context") + } + return context.WithValue(ctx, dbrKey, detect(ctx)) +} + +// MockRuntime is a helper function to mock the detection result. +// It returns a new context with the detection result set. +func MockRuntime(ctx context.Context, b bool) context.Context { + if v := ctx.Value(dbrKey); v != nil { + panic("dbr.MockRuntime called twice on the same context") + } + return context.WithValue(ctx, dbrKey, b) +} + +// RunsOnRuntime returns the detection result from the context. +// It expects a context returned by [DetectRuntime] or [MockRuntime]. +// +// We store this value in a context to avoid having to use either +// a global variable, passing a boolean around everywhere, or +// performing the same detection multiple times. +func RunsOnRuntime(ctx context.Context) bool { + v := ctx.Value(dbrKey) + if v == nil { + panic("dbr.RunsOnRuntime called without calling dbr.DetectRuntime first") + } + return v.(bool) +} diff --git a/libs/dbr/context_test.go b/libs/dbr/context_test.go new file mode 100644 index 000000000..fc53cf130 --- /dev/null +++ b/libs/dbr/context_test.go @@ -0,0 +1,59 @@ +package dbr + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" +) + +func TestContext_DetectRuntimePanics(t *testing.T) { + ctx := context.Background() + + // Run detection. + ctx = DetectRuntime(ctx) + + // Expect a panic if the detection is run twice. + assert.Panics(t, func() { + ctx = DetectRuntime(ctx) + }) +} + +func TestContext_MockRuntimePanics(t *testing.T) { + ctx := context.Background() + + // Run detection. + ctx = MockRuntime(ctx, true) + + // Expect a panic if the mock function is run twice. + assert.Panics(t, func() { + MockRuntime(ctx, true) + }) +} + +func TestContext_RunsOnRuntimePanics(t *testing.T) { + ctx := context.Background() + + // Expect a panic if the detection is not run. + assert.Panics(t, func() { + RunsOnRuntime(ctx) + }) +} + +func TestContext_RunsOnRuntime(t *testing.T) { + ctx := context.Background() + + // Run detection. + ctx = DetectRuntime(ctx) + + // Expect no panic because detection has run. + assert.NotPanics(t, func() { + RunsOnRuntime(ctx) + }) +} + +func TestContext_RunsOnRuntimeWithMock(t *testing.T) { + ctx := context.Background() + assert.True(t, RunsOnRuntime(MockRuntime(ctx, true))) + assert.False(t, RunsOnRuntime(MockRuntime(ctx, false))) +} diff --git a/libs/dbr/detect.go b/libs/dbr/detect.go new file mode 100644 index 000000000..d8b4dfe20 --- /dev/null +++ b/libs/dbr/detect.go @@ -0,0 +1,35 @@ +package dbr + +import ( + "context" + "os" + "runtime" + + "github.com/databricks/cli/libs/env" +) + +// Dereference [os.Stat] to allow mocking in tests. +var statFunc = os.Stat + +// detect returns true if the current process is running on a Databricks Runtime. +// Its return value is meant to be cached in the context. +func detect(ctx context.Context) bool { + // Databricks Runtime implies Linux. + // Return early on other operating systems. + if runtime.GOOS != "linux" { + return false + } + + // Databricks Runtime always has the DATABRICKS_RUNTIME_VERSION environment variable set. + if value, ok := env.Lookup(ctx, "DATABRICKS_RUNTIME_VERSION"); !ok || value == "" { + return false + } + + // Expect to see a "/databricks" directory. + if fi, err := statFunc("/databricks"); err != nil || !fi.IsDir() { + return false + } + + // All checks passed. + return true +} diff --git a/libs/dbr/detect_test.go b/libs/dbr/detect_test.go new file mode 100644 index 000000000..3a4a43a73 --- /dev/null +++ b/libs/dbr/detect_test.go @@ -0,0 +1,83 @@ +package dbr + +import ( + "context" + "io/fs" + "runtime" + "testing" + + "github.com/databricks/cli/libs/env" + "github.com/databricks/cli/libs/fakefs" + "github.com/stretchr/testify/assert" +) + +func requireLinux(t *testing.T) { + if runtime.GOOS != "linux" { + t.Skipf("skipping test on %s", runtime.GOOS) + } +} + +func configureStatFunc(t *testing.T, fi fs.FileInfo, err error) { + originalFunc := statFunc + statFunc = func(name string) (fs.FileInfo, error) { + assert.Equal(t, "/databricks", name) + return fi, err + } + + t.Cleanup(func() { + statFunc = originalFunc + }) +} + +func TestDetect_NotLinux(t *testing.T) { + if runtime.GOOS == "linux" { + t.Skip("skipping test on Linux OS") + } + + ctx := context.Background() + assert.False(t, detect(ctx)) +} + +func TestDetect_Env(t *testing.T) { + requireLinux(t) + + // Configure other checks to pass. + configureStatFunc(t, fakefs.FileInfo{FakeDir: true}, nil) + + t.Run("empty", func(t *testing.T) { + ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "") + assert.False(t, detect(ctx)) + }) + + t.Run("non-empty cluster", func(t *testing.T) { + ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "15.4") + assert.True(t, detect(ctx)) + }) + + t.Run("non-empty serverless", func(t *testing.T) { + ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "client.1.13") + assert.True(t, detect(ctx)) + }) +} + +func TestDetect_Stat(t *testing.T) { + requireLinux(t) + + // Configure other checks to pass. + ctx := env.Set(context.Background(), "DATABRICKS_RUNTIME_VERSION", "non-empty") + + t.Run("error", func(t *testing.T) { + configureStatFunc(t, nil, fs.ErrNotExist) + assert.False(t, detect(ctx)) + }) + + t.Run("not a directory", func(t *testing.T) { + configureStatFunc(t, fakefs.FileInfo{}, nil) + assert.False(t, detect(ctx)) + }) + + t.Run("directory", func(t *testing.T) { + configureStatFunc(t, fakefs.FileInfo{FakeDir: true}, nil) + assert.True(t, detect(ctx)) + }) +} diff --git a/libs/dyn/convert/struct_info.go b/libs/dyn/convert/struct_info.go index 595e52edd..dc3ed4da4 100644 --- a/libs/dyn/convert/struct_info.go +++ b/libs/dyn/convert/struct_info.go @@ -6,7 +6,6 @@ import ( "sync" "github.com/databricks/cli/libs/dyn" - "github.com/databricks/cli/libs/textutil" ) // structInfo holds the type information we need to efficiently @@ -85,14 +84,6 @@ func buildStructInfo(typ reflect.Type) structInfo { } name, _, _ := strings.Cut(sf.Tag.Get("json"), ",") - if typ.Name() == "QualityMonitor" && name == "-" { - urlName, _, _ := strings.Cut(sf.Tag.Get("url"), ",") - if urlName == "" || urlName == "-" { - name = textutil.CamelToSnakeCase(sf.Name) - } else { - name = urlName - } - } if name == "" || name == "-" { continue } diff --git a/libs/fakefs/fakefs.go b/libs/fakefs/fakefs.go new file mode 100644 index 000000000..a8d5eb873 --- /dev/null +++ b/libs/fakefs/fakefs.go @@ -0,0 +1,87 @@ +package fakefs + +import ( + "fmt" + "io/fs" + "time" +) + +var ErrNotImplemented = fmt.Errorf("not implemented") + +// DirEntry is a fake implementation of [fs.DirEntry]. +type DirEntry struct { + fs.FileInfo +} + +func (entry DirEntry) Type() fs.FileMode { + typ := fs.ModePerm + if entry.IsDir() { + typ |= fs.ModeDir + } + return typ +} + +func (entry DirEntry) Info() (fs.FileInfo, error) { + return entry.FileInfo, nil +} + +// FileInfo is a fake implementation of [fs.FileInfo]. +type FileInfo struct { + FakeName string + FakeSize int64 + FakeDir bool + FakeMode fs.FileMode +} + +func (info FileInfo) Name() string { + return info.FakeName +} + +func (info FileInfo) Size() int64 { + return info.FakeSize +} + +func (info FileInfo) Mode() fs.FileMode { + return info.FakeMode +} + +func (info FileInfo) ModTime() time.Time { + return time.Now() +} + +func (info FileInfo) IsDir() bool { + return info.FakeDir +} + +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 000000000..b89190206 --- /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 c533f0b6c..d284447b9 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 0e650ff60..76b8bcd94 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 000000000..fb5364888 --- /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 a74c10f0b..849cf6f7c 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 ad89d6dd5..786c7e394 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 4ac135dd4..000000000 --- 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") -}