From b4d227b2719f8285fd8375a04c2865f98e05bec8 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 30 Jul 2024 15:53:49 +0200 Subject: [PATCH 1/6] Added test for negation pattern in sync include exclude section --- .../config/validate/validate_sync_patterns.go | 4 +- bundle/tests/sync/negate/databricks.yml | 12 +++++ bundle/tests/sync/negate/test.txt | 0 bundle/tests/sync/negate/test.yml | 0 .../sync_include_exclude_no_matches_test.go | 8 ++++ libs/sync/sync_test.go | 47 +++++++++++++++++-- 6 files changed, 67 insertions(+), 4 deletions(-) create mode 100644 bundle/tests/sync/negate/databricks.yml create mode 100644 bundle/tests/sync/negate/test.txt create mode 100644 bundle/tests/sync/negate/test.yml diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go index 573077b66e..84aa6b0e07 100644 --- a/bundle/config/validate/validate_sync_patterns.go +++ b/bundle/config/validate/validate_sync_patterns.go @@ -3,6 +3,7 @@ package validate import ( "context" "fmt" + "strings" "sync" "github.com/databricks/cli/bundle" @@ -49,7 +50,8 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di for i, pattern := range patterns { index := i - p := pattern + // If the pattern starts with !, remove it for the check below + p := strings.TrimPrefix(pattern, "!") errs.Go(func() error { fs, err := fileset.NewGlobSet(rb.BundleRoot(), []string{p}) if err != nil { diff --git a/bundle/tests/sync/negate/databricks.yml b/bundle/tests/sync/negate/databricks.yml new file mode 100644 index 0000000000..f04a84601c --- /dev/null +++ b/bundle/tests/sync/negate/databricks.yml @@ -0,0 +1,12 @@ +bundle: + name: sync_negte + +workspace: + host: https://acme.cloud.databricks.com/ + +sync: + exclude: + - ./* + - '!./*.txt' + include: + - ./*.txt diff --git a/bundle/tests/sync/negate/test.txt b/bundle/tests/sync/negate/test.txt new file mode 100644 index 0000000000..e69de29bb2 diff --git a/bundle/tests/sync/negate/test.yml b/bundle/tests/sync/negate/test.yml new file mode 100644 index 0000000000..e69de29bb2 diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go index 23f99b3a75..8d4d9a9cf8 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/bundle/tests/sync_include_exclude_no_matches_test.go @@ -42,3 +42,11 @@ func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) { require.Equal(t, diags[2].Severity, diag.Warning) require.Contains(t, summaries, diags[2].Summary) } + +func TestSyncIncludeWithNegate(t *testing.T) { + b := load(t, "./sync/negate") + + diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), validate.ValidateSyncPatterns()) + require.Len(t, diags, 0) + require.NoError(t, diags.Error()) +} diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index 292586e8d1..b0a6fd4989 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -67,6 +67,9 @@ func setupFiles(t *testing.T) string { err = createFile(sub2, "g.go") require.NoError(t, err) + err = createFile(sub2, "h.txt") + require.NoError(t, err) + return dir } @@ -97,7 +100,7 @@ func TestGetFileSet(t *testing.T) { fileList, err := s.GetFileList(ctx) require.NoError(t, err) - require.Equal(t, len(fileList), 9) + require.Equal(t, len(fileList), 10) inc, err = fileset.NewGlobSet(root, []string{}) require.NoError(t, err) @@ -115,7 +118,7 @@ func TestGetFileSet(t *testing.T) { fileList, err = s.GetFileList(ctx) require.NoError(t, err) - require.Equal(t, len(fileList), 1) + require.Equal(t, len(fileList), 2) inc, err = fileset.NewGlobSet(root, []string{".databricks/*"}) require.NoError(t, err) @@ -133,7 +136,7 @@ func TestGetFileSet(t *testing.T) { fileList, err = s.GetFileList(ctx) require.NoError(t, err) - require.Equal(t, len(fileList), 10) + require.Equal(t, len(fileList), 11) } func TestRecursiveExclude(t *testing.T) { @@ -165,3 +168,41 @@ func TestRecursiveExclude(t *testing.T) { require.NoError(t, err) require.Equal(t, len(fileList), 7) } + +func TestNegateExclude(t *testing.T) { + ctx := context.Background() + + dir := setupFiles(t) + root := vfs.MustNew(dir) + fileSet, err := git.NewFileSet(root) + require.NoError(t, err) + + err = fileSet.EnsureValidGitIgnoreExists() + require.NoError(t, err) + + inc, err := fileset.NewGlobSet(root, []string{}) + require.NoError(t, err) + + excl, err := fileset.NewGlobSet(root, []string{"./*", "!*.txt"}) + require.NoError(t, err) + + s := &Sync{ + SyncOptions: &SyncOptions{}, + + fileSet: fileSet, + includeFileSet: inc, + excludeFileSet: excl, + } + + fileList, err := s.GetFileList(ctx) + require.NoError(t, err) + require.Equal(t, len(fileList), 1) + found := false + for _, f := range fileList { + if f.Relative == "test/sub1/sub2/h.txt" { + found = true + break + } + } + require.True(t, found) +} From 0d8074517d27a48cae5e80092f9ecb40e870579b Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Tue, 30 Jul 2024 16:06:48 +0200 Subject: [PATCH 2/6] changed include pattern --- libs/sync/sync_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index b0a6fd4989..10e0a10010 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -120,7 +120,7 @@ func TestGetFileSet(t *testing.T) { require.NoError(t, err) require.Equal(t, len(fileList), 2) - inc, err = fileset.NewGlobSet(root, []string{".databricks/*"}) + inc, err = fileset.NewGlobSet(root, []string{"./.databricks/*.go"}) require.NoError(t, err) excl, err = fileset.NewGlobSet(root, []string{}) From 146ad4d2175ed59155f1018108b956f0f3ecb686 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 31 Jul 2024 10:19:28 +0200 Subject: [PATCH 3/6] pr feedback --- bundle/config/validate/validate_sync_patterns.go | 9 ++++++--- bundle/tests/sync/negate/databricks.yml | 10 ++++++++++ .../tests/sync_include_exclude_no_matches_test.go | 13 ++++++++++++- libs/sync/sync_test.go | 9 +-------- 4 files changed, 29 insertions(+), 12 deletions(-) diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go index 84aa6b0e07..f2233fc837 100644 --- a/bundle/config/validate/validate_sync_patterns.go +++ b/bundle/config/validate/validate_sync_patterns.go @@ -50,8 +50,11 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di for i, pattern := range patterns { index := i - // If the pattern starts with !, remove it for the check below - p := strings.TrimPrefix(pattern, "!") + fullPattern := pattern + // If the pattern is negated, strip the negation prefix + // and check if the pattern matches any files. + // If it doesn't, it means that negation pattern is not matching any files. + p := strings.TrimPrefix(fullPattern, "!") errs.Go(func() error { fs, err := fileset.NewGlobSet(rb.BundleRoot(), []string{p}) if err != nil { @@ -68,7 +71,7 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di mu.Lock() diags = diags.Append(diag.Diagnostic{ Severity: diag.Warning, - Summary: fmt.Sprintf("Pattern %s does not match any files", p), + Summary: fmt.Sprintf("Pattern %s does not match any files", fullPattern), Locations: []dyn.Location{loc.Location()}, Paths: []dyn.Path{loc.Path()}, }) diff --git a/bundle/tests/sync/negate/databricks.yml b/bundle/tests/sync/negate/databricks.yml index f04a84601c..cfecfd5bb8 100644 --- a/bundle/tests/sync/negate/databricks.yml +++ b/bundle/tests/sync/negate/databricks.yml @@ -10,3 +10,13 @@ sync: - '!./*.txt' include: - ./*.txt + +targets: + default: + dev: + sync: + exclude: + - ./* + - '!./*.txt2' + include: + - ./*.txt diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go index 8d4d9a9cf8..ab42ddd7e3 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/bundle/tests/sync_include_exclude_no_matches_test.go @@ -44,9 +44,20 @@ func TestSyncIncludeExcludeNoMatchesTest(t *testing.T) { } func TestSyncIncludeWithNegate(t *testing.T) { - b := load(t, "./sync/negate") + b := loadTarget(t, "./sync/negate", "default") diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), validate.ValidateSyncPatterns()) require.Len(t, diags, 0) require.NoError(t, diags.Error()) } + +func TestSyncIncludeWithNegateNoMatches(t *testing.T) { + b := loadTarget(t, "./sync/negate", "dev") + + diags := bundle.ApplyReadOnly(context.Background(), bundle.ReadOnly(b), validate.ValidateSyncPatterns()) + require.Len(t, diags, 1) + require.NoError(t, diags.Error()) + + require.Equal(t, diags[0].Severity, diag.Warning) + require.Equal(t, diags[0].Summary, "Pattern !./*.txt2 does not match any files") +} diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index 10e0a10010..185c225bcf 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -197,12 +197,5 @@ func TestNegateExclude(t *testing.T) { fileList, err := s.GetFileList(ctx) require.NoError(t, err) require.Equal(t, len(fileList), 1) - found := false - for _, f := range fileList { - if f.Relative == "test/sub1/sub2/h.txt" { - found = true - break - } - } - require.True(t, found) + require.Equal(t, fileList[0].Relative, "test/sub1/sub2/h.txt") } From 030f251f0ec36feb7308e9a734258698fbe8b9b3 Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 31 Jul 2024 11:51:54 +0200 Subject: [PATCH 4/6] fix typo --- bundle/tests/sync/negate/databricks.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bundle/tests/sync/negate/databricks.yml b/bundle/tests/sync/negate/databricks.yml index cfecfd5bb8..f1e021e57c 100644 --- a/bundle/tests/sync/negate/databricks.yml +++ b/bundle/tests/sync/negate/databricks.yml @@ -1,5 +1,5 @@ bundle: - name: sync_negte + name: sync_negate workspace: host: https://acme.cloud.databricks.com/ From 310e2074e98594127bcbcdf1975abe2ab1f1a28b Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 31 Jul 2024 12:05:18 +0200 Subject: [PATCH 5/6] fix test --- bundle/tests/sync/negate/databricks.yml | 8 ++++---- bundle/tests/sync_include_exclude_no_matches_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bundle/tests/sync/negate/databricks.yml b/bundle/tests/sync/negate/databricks.yml index f1e021e57c..3d591d19b3 100644 --- a/bundle/tests/sync/negate/databricks.yml +++ b/bundle/tests/sync/negate/databricks.yml @@ -7,9 +7,9 @@ workspace: sync: exclude: - ./* - - '!./*.txt' + - '!*.txt' include: - - ./*.txt + - '*.txt' targets: default: @@ -17,6 +17,6 @@ targets: sync: exclude: - ./* - - '!./*.txt2' + - '!*.txt2' include: - - ./*.txt + - '*.txt' diff --git a/bundle/tests/sync_include_exclude_no_matches_test.go b/bundle/tests/sync_include_exclude_no_matches_test.go index ab42ddd7e3..0192b61e65 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/bundle/tests/sync_include_exclude_no_matches_test.go @@ -59,5 +59,5 @@ func TestSyncIncludeWithNegateNoMatches(t *testing.T) { require.NoError(t, diags.Error()) require.Equal(t, diags[0].Severity, diag.Warning) - require.Equal(t, diags[0].Summary, "Pattern !./*.txt2 does not match any files") + require.Equal(t, diags[0].Summary, "Pattern !*.txt2 does not match any files") } From d9db50affd250e51b3beaedf5cbcc8c364d2309c Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 31 Jul 2024 15:33:13 +0200 Subject: [PATCH 6/6] addressed feedback --- .../config/validate/validate_sync_patterns.go | 4 +- libs/sync/sync_test.go | 71 ++++--------------- 2 files changed, 18 insertions(+), 57 deletions(-) diff --git a/bundle/config/validate/validate_sync_patterns.go b/bundle/config/validate/validate_sync_patterns.go index f2233fc837..fd011bf780 100644 --- a/bundle/config/validate/validate_sync_patterns.go +++ b/bundle/config/validate/validate_sync_patterns.go @@ -53,7 +53,9 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di fullPattern := pattern // If the pattern is negated, strip the negation prefix // and check if the pattern matches any files. - // If it doesn't, it means that negation pattern is not matching any files. + // Negation in gitignore syntax means "don't look at this path' + // So if p matches nothing it's useless negation, but if there are matches, + // it means: do not include these files into result set p := strings.TrimPrefix(fullPattern, "!") errs.Go(func() error { fs, err := fileset.NewGlobSet(rb.BundleRoot(), []string{p}) diff --git a/libs/sync/sync_test.go b/libs/sync/sync_test.go index 185c225bcf..2d800f4668 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -2,73 +2,32 @@ package sync import ( "context" - "os" - "path/filepath" "testing" + "github.com/databricks/cli/internal/testutil" "github.com/databricks/cli/libs/fileset" "github.com/databricks/cli/libs/git" "github.com/databricks/cli/libs/vfs" "github.com/stretchr/testify/require" ) -func createFile(dir string, name string) error { - f, err := os.Create(filepath.Join(dir, name)) - if err != nil { - return err - } - - return f.Close() -} - func setupFiles(t *testing.T) string { dir := t.TempDir() - err := createFile(dir, "a.go") - require.NoError(t, err) - - err = createFile(dir, "b.go") - require.NoError(t, err) - - err = createFile(dir, "ab.go") - require.NoError(t, err) - - err = createFile(dir, "abc.go") - require.NoError(t, err) - - err = createFile(dir, "c.go") - require.NoError(t, err) - - err = createFile(dir, "d.go") - require.NoError(t, err) - - dbDir := filepath.Join(dir, ".databricks") - err = os.Mkdir(dbDir, 0755) - require.NoError(t, err) - - err = createFile(dbDir, "e.go") - require.NoError(t, err) - - testDir := filepath.Join(dir, "test") - err = os.Mkdir(testDir, 0755) - require.NoError(t, err) - - sub1 := filepath.Join(testDir, "sub1") - err = os.Mkdir(sub1, 0755) - require.NoError(t, err) - - err = createFile(sub1, "f.go") - require.NoError(t, err) - - sub2 := filepath.Join(sub1, "sub2") - err = os.Mkdir(sub2, 0755) - require.NoError(t, err) - - err = createFile(sub2, "g.go") - require.NoError(t, err) - - err = createFile(sub2, "h.txt") - require.NoError(t, err) + for _, f := range []([]string){ + []string{dir, "a.go"}, + []string{dir, "b.go"}, + []string{dir, "ab.go"}, + []string{dir, "abc.go"}, + []string{dir, "c.go"}, + []string{dir, "d.go"}, + []string{dir, ".databricks", "e.go"}, + []string{dir, "test", "sub1", "f.go"}, + []string{dir, "test", "sub1", "sub2", "g.go"}, + []string{dir, "test", "sub1", "sub2", "h.txt"}, + } { + testutil.Touch(t, f...) + } return dir }