From abfd1713e088503eb869981b4f2f1aa6c9b810a5 Mon Sep 17 00:00:00 2001
From: Pieter Noordhuis <pieter.noordhuis@databricks.com>
Date: Thu, 21 Nov 2024 16:03:13 +0100
Subject: [PATCH] Skip sync warning if no sync paths are defined (#1926)

## Changes

Users can configure the bundle to not synchronize any files with:
```yaml
sync:
  paths: []
```

If it is explicitly configured as an empty list, the validate command
must not warn about not having any files to synchronize. The warning
exists to alert users who are unintentionally not synchronizing any
files (they might have a `.gitignore` pattern that matches everything).

Closes #1663.

## Tests

* New unit test.
---
 bundle/config/validate/files_to_sync.go      |   7 ++
 bundle/config/validate/files_to_sync_test.go | 105 +++++++++++++++++++
 2 files changed, 112 insertions(+)
 create mode 100644 bundle/config/validate/files_to_sync_test.go

diff --git a/bundle/config/validate/files_to_sync.go b/bundle/config/validate/files_to_sync.go
index 7cdad772..a1427848 100644
--- a/bundle/config/validate/files_to_sync.go
+++ b/bundle/config/validate/files_to_sync.go
@@ -21,6 +21,12 @@ func (v *filesToSync) Name() string {
 }
 
 func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.Diagnostics {
+	// The user may be intentional about not synchronizing any files.
+	// In this case, we should not show any warnings.
+	if len(rb.Config().Sync.Paths) == 0 {
+		return nil
+	}
+
 	sync, err := files.GetSync(ctx, rb)
 	if err != nil {
 		return diag.FromErr(err)
@@ -31,6 +37,7 @@ func (v *filesToSync) Apply(ctx context.Context, rb bundle.ReadOnlyBundle) diag.
 		return diag.FromErr(err)
 	}
 
+	// If there are files to sync, we don't need to show any warnings.
 	if len(fl) != 0 {
 		return nil
 	}
diff --git a/bundle/config/validate/files_to_sync_test.go b/bundle/config/validate/files_to_sync_test.go
new file mode 100644
index 00000000..2a598fa7
--- /dev/null
+++ b/bundle/config/validate/files_to_sync_test.go
@@ -0,0 +1,105 @@
+package validate
+
+import (
+	"context"
+	"testing"
+
+	"github.com/databricks/cli/bundle"
+	"github.com/databricks/cli/bundle/config"
+	"github.com/databricks/cli/internal/testutil"
+	"github.com/databricks/cli/libs/diag"
+	"github.com/databricks/cli/libs/vfs"
+	sdkconfig "github.com/databricks/databricks-sdk-go/config"
+	"github.com/databricks/databricks-sdk-go/experimental/mocks"
+	"github.com/databricks/databricks-sdk-go/service/iam"
+	"github.com/databricks/databricks-sdk-go/service/workspace"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
+)
+
+func TestFilesToSync_NoPaths(t *testing.T) {
+	b := &bundle.Bundle{
+		Config: config.Root{
+			Sync: config.Sync{
+				Paths: []string{},
+			},
+		},
+	}
+
+	ctx := context.Background()
+	rb := bundle.ReadOnly(b)
+	diags := bundle.ApplyReadOnly(ctx, rb, FilesToSync())
+	assert.Empty(t, diags)
+}
+
+func setupBundleForFilesToSyncTest(t *testing.T) *bundle.Bundle {
+	dir := t.TempDir()
+
+	testutil.Touch(t, dir, "file1")
+	testutil.Touch(t, dir, "file2")
+
+	b := &bundle.Bundle{
+		BundleRootPath: dir,
+		BundleRoot:     vfs.MustNew(dir),
+		SyncRootPath:   dir,
+		SyncRoot:       vfs.MustNew(dir),
+		Config: config.Root{
+			Bundle: config.Bundle{
+				Target: "default",
+			},
+			Workspace: config.Workspace{
+				FilePath: "/this/doesnt/matter",
+				CurrentUser: &config.User{
+					User: &iam.User{},
+				},
+			},
+			Sync: config.Sync{
+				// Paths are relative to [SyncRootPath].
+				Paths: []string{"."},
+			},
+		},
+	}
+
+	m := mocks.NewMockWorkspaceClient(t)
+	m.WorkspaceClient.Config = &sdkconfig.Config{
+		Host: "https://foo.com",
+	}
+
+	// The initialization logic in [sync.New] performs a check on the destination path.
+	// Removing this check at initialization time is tbd...
+	m.GetMockWorkspaceAPI().EXPECT().GetStatusByPath(mock.Anything, "/this/doesnt/matter").Return(&workspace.ObjectInfo{
+		ObjectType: workspace.ObjectTypeDirectory,
+	}, nil)
+
+	b.SetWorkpaceClient(m.WorkspaceClient)
+	return b
+}
+
+func TestFilesToSync_EverythingIgnored(t *testing.T) {
+	b := setupBundleForFilesToSyncTest(t)
+
+	// Ignore all files.
+	testutil.WriteFile(t, "*\n.*\n", b.BundleRootPath, ".gitignore")
+
+	ctx := context.Background()
+	rb := bundle.ReadOnly(b)
+	diags := bundle.ApplyReadOnly(ctx, rb, FilesToSync())
+	require.Equal(t, 1, len(diags))
+	assert.Equal(t, diag.Warning, diags[0].Severity)
+	assert.Equal(t, "There are no files to sync, please check your .gitignore", diags[0].Summary)
+}
+
+func TestFilesToSync_EverythingExcluded(t *testing.T) {
+	b := setupBundleForFilesToSyncTest(t)
+
+	// Exclude all files.
+	b.Config.Sync.Exclude = []string{"*"}
+
+	ctx := context.Background()
+	rb := bundle.ReadOnly(b)
+	diags := bundle.ApplyReadOnly(ctx, rb, FilesToSync())
+	require.Equal(t, 1, len(diags))
+	assert.Equal(t, diag.Warning, diags[0].Severity)
+	assert.Equal(t, "There are no files to sync, please check your .gitignore and sync.exclude configuration", diags[0].Summary)
+}