From 1fb8e324d5646ce3320ea4c64d79de332f17c53b Mon Sep 17 00:00:00 2001 From: Andrew Nester Date: Wed, 31 Jul 2024 15:42:23 +0200 Subject: [PATCH] Added test for negation pattern in sync include exclude section (#1637) ## Changes Added test for negation pattern in sync include exclude section --- .../config/validate/validate_sync_patterns.go | 11 +- bundle/tests/sync/negate/databricks.yml | 22 ++++ bundle/tests/sync/negate/test.txt | 0 bundle/tests/sync/negate/test.yml | 0 .../sync_include_exclude_no_matches_test.go | 19 ++++ libs/sync/sync_test.go | 107 ++++++++---------- 6 files changed, 100 insertions(+), 59 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..fd011bf780 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,13 @@ func checkPatterns(patterns []string, path string, rb bundle.ReadOnlyBundle) (di for i, pattern := range patterns { index := i - p := pattern + fullPattern := pattern + // If the pattern is negated, strip the negation prefix + // and check if the pattern matches 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}) if err != nil { @@ -66,7 +73,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 new file mode 100644 index 0000000000..3d591d19b3 --- /dev/null +++ b/bundle/tests/sync/negate/databricks.yml @@ -0,0 +1,22 @@ +bundle: + name: sync_negate + +workspace: + host: https://acme.cloud.databricks.com/ + +sync: + exclude: + - ./* + - '!*.txt' + include: + - '*.txt' + +targets: + default: + dev: + sync: + exclude: + - ./* + - '!*.txt2' + 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..0192b61e65 100644 --- a/bundle/tests/sync_include_exclude_no_matches_test.go +++ b/bundle/tests/sync_include_exclude_no_matches_test.go @@ -42,3 +42,22 @@ 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 := 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 292586e8d1..2d800f4668 100644 --- a/libs/sync/sync_test.go +++ b/libs/sync/sync_test.go @@ -2,70 +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) + 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 } @@ -97,7 +59,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,9 +77,9 @@ 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/*"}) + inc, err = fileset.NewGlobSet(root, []string{"./.databricks/*.go"}) require.NoError(t, err) excl, err = fileset.NewGlobSet(root, []string{}) @@ -133,7 +95,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 +127,34 @@ 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) + require.Equal(t, fileList[0].Relative, "test/sub1/sub2/h.txt") +}