From fde30ff1ab5bf504e326f58adfd9f429d88b3b6d Mon Sep 17 00:00:00 2001 From: Denis Bilenko Date: Wed, 22 Jan 2025 11:17:45 +0100 Subject: [PATCH] Add a test for sync root outside of git root (#2202) - Move acceptance/bundle/sync-paths-dotdot test to acceptance/bundle/syncroot/dotdot-notgit - Add new test acceptance/bundle/syncroot/dotdot-git Fix replacer to work with this test and on Windows: - Make PATH work on Windows by using EvalSymlinks. - Make concatenated path match within JSON but stripping quotes. --- acceptance/acceptance_test.go | 12 +++--- acceptance/bundle/sync-paths-dotdot/script | 1 - .../dotdot-git}/databricks.yml | 0 .../dotdot-git}/output.txt | 2 +- acceptance/bundle/syncroot/dotdot-git/script | 6 +++ .../syncroot/dotdot-nogit/databricks.yml | 5 +++ .../bundle/syncroot/dotdot-nogit/output.txt | 11 ++++++ .../bundle/syncroot/dotdot-nogit/script | 2 + libs/testdiff/replacement.go | 39 ++++++++++++++++++- 9 files changed, 69 insertions(+), 9 deletions(-) delete mode 100644 acceptance/bundle/sync-paths-dotdot/script rename acceptance/bundle/{sync-paths-dotdot => syncroot/dotdot-git}/databricks.yml (100%) rename acceptance/bundle/{sync-paths-dotdot => syncroot/dotdot-git}/output.txt (69%) create mode 100644 acceptance/bundle/syncroot/dotdot-git/script create mode 100644 acceptance/bundle/syncroot/dotdot-nogit/databricks.yml create mode 100644 acceptance/bundle/syncroot/dotdot-nogit/output.txt create mode 100644 acceptance/bundle/syncroot/dotdot-nogit/script diff --git a/acceptance/acceptance_test.go b/acceptance/acceptance_test.go index cfcb0d29fa..9a4564ffae 100644 --- a/acceptance/acceptance_test.go +++ b/acceptance/acceptance_test.go @@ -188,12 +188,12 @@ func runTest(t *testing.T, dir, coverDir string, repls testdiff.ReplacementsCont tmpDir = t.TempDir() } - repls.Set("/private"+tmpDir, "$TMPDIR") - repls.Set("/private"+filepath.Dir(tmpDir), "$TMPPARENT") - repls.Set("/private"+filepath.Dir(filepath.Dir(tmpDir)), "$TMPGPARENT") - repls.Set(tmpDir, "$TMPDIR") - repls.Set(filepath.Dir(tmpDir), "$TMPPARENT") - repls.Set(filepath.Dir(filepath.Dir(tmpDir)), "$TMPGPARENT") + // Converts C:\Users\DENIS~1.BIL -> C:\Users\denis.bilenko + tmpDirEvalled, err1 := filepath.EvalSymlinks(tmpDir) + if err1 == nil && tmpDirEvalled != tmpDir { + repls.SetPathWithParents(tmpDirEvalled, "$TMPDIR") + } + repls.SetPathWithParents(tmpDir, "$TMPDIR") scriptContents := readMergedScriptContents(t, dir) testutil.WriteFile(t, filepath.Join(tmpDir, EntryPointScript), scriptContents) diff --git a/acceptance/bundle/sync-paths-dotdot/script b/acceptance/bundle/sync-paths-dotdot/script deleted file mode 100644 index 72555b332a..0000000000 --- a/acceptance/bundle/sync-paths-dotdot/script +++ /dev/null @@ -1 +0,0 @@ -$CLI bundle validate diff --git a/acceptance/bundle/sync-paths-dotdot/databricks.yml b/acceptance/bundle/syncroot/dotdot-git/databricks.yml similarity index 100% rename from acceptance/bundle/sync-paths-dotdot/databricks.yml rename to acceptance/bundle/syncroot/dotdot-git/databricks.yml diff --git a/acceptance/bundle/sync-paths-dotdot/output.txt b/acceptance/bundle/syncroot/dotdot-git/output.txt similarity index 69% rename from acceptance/bundle/sync-paths-dotdot/output.txt rename to acceptance/bundle/syncroot/dotdot-git/output.txt index 11db3e9ee0..f1dc5fb014 100644 --- a/acceptance/bundle/sync-paths-dotdot/output.txt +++ b/acceptance/bundle/syncroot/dotdot-git/output.txt @@ -1,4 +1,4 @@ -Error: path "$TMPPARENT" is not within repository root "$TMPDIR" +Error: path "$TMPDIR" is not within repository root "$TMPDIR/myrepo" Name: test-bundle Target: default diff --git a/acceptance/bundle/syncroot/dotdot-git/script b/acceptance/bundle/syncroot/dotdot-git/script new file mode 100644 index 0000000000..0706a1d5e5 --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-git/script @@ -0,0 +1,6 @@ +# This should error, we do not allow syncroot outside of git repo. +mkdir myrepo +cd myrepo +cp ../databricks.yml . +git-repo-init +$CLI bundle validate | sed 's/\\\\/\//g' diff --git a/acceptance/bundle/syncroot/dotdot-nogit/databricks.yml b/acceptance/bundle/syncroot/dotdot-nogit/databricks.yml new file mode 100644 index 0000000000..7215ffea29 --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-nogit/databricks.yml @@ -0,0 +1,5 @@ +bundle: + name: test-bundle +sync: + paths: + - .. diff --git a/acceptance/bundle/syncroot/dotdot-nogit/output.txt b/acceptance/bundle/syncroot/dotdot-nogit/output.txt new file mode 100644 index 0000000000..34059e2769 --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-nogit/output.txt @@ -0,0 +1,11 @@ +Error: path "$TMPDIR_PARENT" is not within repository root "$TMPDIR" + +Name: test-bundle +Target: default +Workspace: + User: $USERNAME + Path: /Workspace/Users/$USERNAME/.bundle/test-bundle/default + +Found 1 error + +Exit code: 1 diff --git a/acceptance/bundle/syncroot/dotdot-nogit/script b/acceptance/bundle/syncroot/dotdot-nogit/script new file mode 100644 index 0000000000..d3388903e1 --- /dev/null +++ b/acceptance/bundle/syncroot/dotdot-nogit/script @@ -0,0 +1,2 @@ +# This should not error, syncroot can be outside bundle root. +$CLI bundle validate diff --git a/libs/testdiff/replacement.go b/libs/testdiff/replacement.go index 207f425aae..ca76b159cf 100644 --- a/libs/testdiff/replacement.go +++ b/libs/testdiff/replacement.go @@ -3,7 +3,9 @@ package testdiff import ( "encoding/json" "fmt" + "path/filepath" "regexp" + "runtime" "slices" "strings" @@ -74,13 +76,48 @@ func (r *ReplacementsContext) Set(old, new string) { if err == nil { encodedOld, err := json.Marshal(old) if err == nil { - r.appendLiteral(string(encodedOld), string(encodedNew)) + r.appendLiteral(trimQuotes(string(encodedOld)), trimQuotes(string(encodedNew))) } } r.appendLiteral(old, new) } +func trimQuotes(s string) string { + if len(s) > 0 && s[0] == '"' { + s = s[1:] + } + if len(s) > 0 && s[len(s)-1] == '"' { + s = s[:len(s)-1] + } + return s +} + +func (r *ReplacementsContext) SetPath(old, new string) { + r.Set(old, new) + + if runtime.GOOS != "windows" { + return + } + + // Support both forward and backward slashes + m1 := strings.ReplaceAll(old, "\\", "/") + if m1 != old { + r.Set(m1, new) + } + + m2 := strings.ReplaceAll(old, "/", "\\") + if m2 != old && m2 != m1 { + r.Set(m2, new) + } +} + +func (r *ReplacementsContext) SetPathWithParents(old, new string) { + r.SetPath(old, new) + r.SetPath(filepath.Dir(old), new+"_PARENT") + r.SetPath(filepath.Dir(filepath.Dir(old)), new+"_GPARENT") +} + func PrepareReplacementsWorkspaceClient(t testutil.TestingT, r *ReplacementsContext, w *databricks.WorkspaceClient) { t.Helper() // in some clouds (gcp) w.Config.Host includes "https://" prefix in others it's really just a host (azure)