From 1508d65c4cec06c697060b5ea143a835f5ec0f24 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Thu, 14 Nov 2024 17:10:45 +0100
Subject: [PATCH 1/4] Extract functionality to detect if the CLI is running on
 DBR (#1889)

## Changes

Whether or not the CLI is running on DBR can be detected once and stored
in the command's context.

By storing it in the context, it can easily be mocked for testing.

This builds on the simpler approach and conversation in #1744. It
unblocks testing of the DBR-specific paths while not compromising on the
checks we can perform to test if the CLI is running on DBR.

## Tests

* Unit tests for the new `dbr` package
* New unit test for the `ConfigureWSFS` mutator
---
 bundle/config/mutator/configure_wsfs.go      |  6 +-
 bundle/config/mutator/configure_wsfs_test.go | 65 +++++++++++++++
 cmd/root/root.go                             |  4 +
 libs/dbr/context.go                          | 49 ++++++++++++
 libs/dbr/context_test.go                     | 59 ++++++++++++++
 libs/dbr/detect.go                           | 35 +++++++++
 libs/dbr/detect_test.go                      | 83 ++++++++++++++++++++
 libs/fakefs/fakefs.go                        | 55 +++++++++++++
 8 files changed, 352 insertions(+), 4 deletions(-)
 create mode 100644 bundle/config/mutator/configure_wsfs_test.go
 create mode 100644 libs/dbr/context.go
 create mode 100644 libs/dbr/context_test.go
 create mode 100644 libs/dbr/detect.go
 create mode 100644 libs/dbr/detect_test.go
 create mode 100644 libs/fakefs/fakefs.go

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/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/fakefs/fakefs.go b/libs/fakefs/fakefs.go
new file mode 100644
index 000000000..2f4756970
--- /dev/null
+++ b/libs/fakefs/fakefs.go
@@ -0,0 +1,55 @@
+package fakefs
+
+import (
+	"io/fs"
+	"time"
+)
+
+// DirEntry is a fake implementation of [fs.DirEntry].
+type DirEntry struct {
+	FileInfo
+}
+
+func (entry DirEntry) Type() fs.FileMode {
+	typ := fs.ModePerm
+	if entry.FakeDir {
+		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
+}

From 1db384018c5efc6c7b1a9a43d5f1268c97ddd58d Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Thu, 14 Nov 2024 18:39:38 +0100
Subject: [PATCH 2/4] Make `TableName` field part of quality monitor schema
 (#1903)

## Changes

This field was special-cased in #1307 because it's not part of the JSON
payload in the SDK struct.

This approach, while pragmatic, meant it didn't show up in the JSON
schema. While debugging an issue with quality monitors in #1900, I
couldn't figure out why I was getting schema errors on this field, or
how it was passed through to the TF representation. This commit removes
the special case and makes it behave like everything else.

## Tests

* Unit tests pass.
* Confirmed that the updated schema failed validation before this
change.
---
 bundle/config/mutator/initialize_urls_test.go   |  5 ++---
 .../config/mutator/process_target_mode_test.go  | 17 ++++++++++++-----
 bundle/config/resources/quality_monitor.go      | 16 +++++++---------
 .../tfdyn/convert_quality_monitor_test.go       |  2 +-
 .../schema/testdata/pass/quality_monitor.yml    |  1 +
 bundle/schema/jsonschema.json                   |  4 ++++
 libs/dyn/convert/struct_info.go                 |  9 ---------
 7 files changed, 27 insertions(+), 27 deletions(-)

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/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/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
 			}

From 7f3fb10c4ac92d4e53b6e313dc87c31873c3c8ea Mon Sep 17 00:00:00 2001
From: Andrew Nester <andrew.nester@databricks.com>
Date: Fri, 15 Nov 2024 16:03:59 +0100
Subject: [PATCH 3/4] Do not prepend paths starting with ~ or variable
 reference (#1905)

## Changes
Fixes #1904

## Tests
Added regression test
---
 bundle/config/mutator/prepend_workspace_prefix.go      | 5 +++++
 bundle/config/mutator/prepend_workspace_prefix_test.go | 8 ++++++++
 2 files changed, 13 insertions(+)

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 {

From 7d732ceba8c5229f5228762db0d123764e290ddd Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Fri, 15 Nov 2024 16:37:21 +0100
Subject: [PATCH 4/4] 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 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/libs/fakefs/fakefs.go b/libs/fakefs/fakefs.go
index 2f4756970..a8d5eb873 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 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")
-}