From e1767d9ef8eb32af43b43582188fbf30f5ddc54a Mon Sep 17 00:00:00 2001 From: monalisa Date: Mon, 7 Aug 2023 14:23:47 +0200 Subject: [PATCH 1/4] Fix bundle git branch validation --- bundle/config/mutator/load_git_details.go | 23 +++++++------ bundle/tests/autoload_git_test.go | 20 ----------- .../git_branch_validation/databricks.yml | 4 +++ bundle/tests/git_test.go | 33 +++++++++++++++++++ 4 files changed, 50 insertions(+), 30 deletions(-) delete mode 100644 bundle/tests/autoload_git_test.go create mode 100644 bundle/tests/git_branch_validation/databricks.yml create mode 100644 bundle/tests/git_test.go diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index f22aafe01d..ecb4cd282f 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/bundle/config/mutator/load_git_details.go @@ -24,17 +24,20 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error { if err != nil { return err } - // load branch name if undefined - if b.Config.Bundle.Git.Branch == "" { - branch, err := repo.CurrentBranch() - if err != nil { - log.Warnf(ctx, "failed to load current branch: %s", err) - } else { - b.Config.Bundle.Git.Branch = branch - b.Config.Bundle.Git.ActualBranch = branch - b.Config.Bundle.Git.Inferred = true - } + + // Read branch name of current checkout + branch, err := repo.CurrentBranch() + if err == nil && b.Config.Bundle.Git.Branch == "" { + // Only load branch if there's no user defined value + b.Config.Bundle.Git.Inferred = true + b.Config.Bundle.Git.Branch = branch + } + if err == nil { + b.Config.Bundle.Git.ActualBranch = branch + } else { + log.Warnf(ctx, "failed to load current branch: %s", err) } + // load commit hash if undefined if b.Config.Bundle.Git.Commit == "" { commit, err := repo.LatestCommit() diff --git a/bundle/tests/autoload_git_test.go b/bundle/tests/autoload_git_test.go deleted file mode 100644 index a1075198fa..0000000000 --- a/bundle/tests/autoload_git_test.go +++ /dev/null @@ -1,20 +0,0 @@ -package config_tests - -import ( - "testing" - - "github.com/stretchr/testify/assert" -) - -func TestAutoLoad(t *testing.T) { - b := load(t, "./autoload_git") - assert.True(t, b.Config.Bundle.Git.Inferred) - assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli") -} - -func TestManuallySetBranch(t *testing.T) { - b := loadEnvironment(t, "./autoload_git", "production") - assert.False(t, b.Config.Bundle.Git.Inferred) - assert.Equal(t, "main", b.Config.Bundle.Git.Branch) - assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli") -} diff --git a/bundle/tests/git_branch_validation/databricks.yml b/bundle/tests/git_branch_validation/databricks.yml new file mode 100644 index 0000000000..6363fb7655 --- /dev/null +++ b/bundle/tests/git_branch_validation/databricks.yml @@ -0,0 +1,4 @@ +bundle: + name: "Dancing Feet" + git: + branch: "this-branch-is-for-sure-not-checked-out-123" diff --git a/bundle/tests/git_test.go b/bundle/tests/git_test.go new file mode 100644 index 0000000000..8682b7d578 --- /dev/null +++ b/bundle/tests/git_test.go @@ -0,0 +1,33 @@ +package config_tests + +import ( + "context" + "testing" + + "github.com/databricks/cli/bundle" + "github.com/databricks/cli/bundle/config/mutator" + "github.com/stretchr/testify/assert" +) + +func TestGitAutoLoad(t *testing.T) { + b := load(t, "./autoload_git") + assert.True(t, b.Config.Bundle.Git.Inferred) + assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli") +} + +func TestGitManuallySetBranch(t *testing.T) { + b := loadEnvironment(t, "./autoload_git", "production") + assert.False(t, b.Config.Bundle.Git.Inferred) + assert.Equal(t, "main", b.Config.Bundle.Git.Branch) + assert.Contains(t, b.Config.Bundle.Git.OriginURL, "/cli") +} + +func TestGitBundleBranchValidation(t *testing.T) { + b := load(t, "./git_branch_validation") + assert.False(t, b.Config.Bundle.Git.Inferred) + assert.Equal(t, "this-branch-is-for-sure-not-checked-out-123", b.Config.Bundle.Git.Branch) + assert.NotEqual(t, "this-branch-is-for-sure-not-checked-out-123", b.Config.Bundle.Git.ActualBranch) + + err := bundle.Apply(context.Background(), b, mutator.ValidateGitDetails()) + assert.ErrorContains(t, err, "not on the right Git branch:") +} From 814feb5f0cf4a547f58563af873b02afe254d6fd Mon Sep 17 00:00:00 2001 From: monalisa Date: Mon, 7 Aug 2023 14:41:46 +0200 Subject: [PATCH 2/4] fully mock the repo --- bundle/tests/git_branch_validation/.mock-git/HEAD | 1 + bundle/tests/git_branch_validation/databricks.yml | 2 +- bundle/tests/git_test.go | 10 ++++++++-- libs/git/repository.go | 9 +++++---- 4 files changed, 15 insertions(+), 7 deletions(-) create mode 100644 bundle/tests/git_branch_validation/.mock-git/HEAD diff --git a/bundle/tests/git_branch_validation/.mock-git/HEAD b/bundle/tests/git_branch_validation/.mock-git/HEAD new file mode 100644 index 0000000000..6c83ec9df9 --- /dev/null +++ b/bundle/tests/git_branch_validation/.mock-git/HEAD @@ -0,0 +1 @@ +ref: refs/heads/feature-b diff --git a/bundle/tests/git_branch_validation/databricks.yml b/bundle/tests/git_branch_validation/databricks.yml index 6363fb7655..8c7b96efc1 100644 --- a/bundle/tests/git_branch_validation/databricks.yml +++ b/bundle/tests/git_branch_validation/databricks.yml @@ -1,4 +1,4 @@ bundle: name: "Dancing Feet" git: - branch: "this-branch-is-for-sure-not-checked-out-123" + branch: "feature-a" diff --git a/bundle/tests/git_test.go b/bundle/tests/git_test.go index 8682b7d578..daab4d30a2 100644 --- a/bundle/tests/git_test.go +++ b/bundle/tests/git_test.go @@ -6,6 +6,7 @@ import ( "github.com/databricks/cli/bundle" "github.com/databricks/cli/bundle/config/mutator" + "github.com/databricks/cli/libs/git" "github.com/stretchr/testify/assert" ) @@ -23,10 +24,15 @@ func TestGitManuallySetBranch(t *testing.T) { } func TestGitBundleBranchValidation(t *testing.T) { + git.GitDirectoryName = ".mock-git" + t.Cleanup(func() { + git.GitDirectoryName = ".git" + }) + b := load(t, "./git_branch_validation") assert.False(t, b.Config.Bundle.Git.Inferred) - assert.Equal(t, "this-branch-is-for-sure-not-checked-out-123", b.Config.Bundle.Git.Branch) - assert.NotEqual(t, "this-branch-is-for-sure-not-checked-out-123", b.Config.Bundle.Git.ActualBranch) + assert.Equal(t, "feature-a", b.Config.Bundle.Git.Branch) + assert.Equal(t, "feature-b", b.Config.Bundle.Git.ActualBranch) err := bundle.Apply(context.Background(), b, mutator.ValidateGitDetails()) assert.ErrorContains(t, err, "not on the right Git branch:") diff --git a/libs/git/repository.go b/libs/git/repository.go index 3b93669aec..f55d9f9438 100644 --- a/libs/git/repository.go +++ b/libs/git/repository.go @@ -11,6 +11,7 @@ import ( ) const gitIgnoreFileName = ".gitignore" +var GitDirectoryName = ".git" // Repository represents a Git repository or a directory // that could later be initialized as Git repository. @@ -45,7 +46,7 @@ func (r *Repository) Root() string { func (r *Repository) CurrentBranch() (string, error) { // load .git/HEAD - ref, err := LoadReferenceFile(filepath.Join(r.rootPath, ".git", "HEAD")) + ref, err := LoadReferenceFile(filepath.Join(r.rootPath, GitDirectoryName, "HEAD")) if err != nil { return "", err } @@ -62,7 +63,7 @@ func (r *Repository) CurrentBranch() (string, error) { func (r *Repository) LatestCommit() (string, error) { // load .git/HEAD - ref, err := LoadReferenceFile(filepath.Join(r.rootPath, ".git", "HEAD")) + ref, err := LoadReferenceFile(filepath.Join(r.rootPath, GitDirectoryName, "HEAD")) if err != nil { return "", err } @@ -81,7 +82,7 @@ func (r *Repository) LatestCommit() (string, error) { if err != nil { return "", err } - branchHeadRef, err := LoadReferenceFile(filepath.Join(r.rootPath, ".git", branchHeadPath)) + branchHeadRef, err := LoadReferenceFile(filepath.Join(r.rootPath, GitDirectoryName, branchHeadPath)) if err != nil { return "", err } @@ -186,7 +187,7 @@ func NewRepository(path string) (*Repository, error) { } real := true - rootPath, err := folders.FindDirWithLeaf(path, ".git") + rootPath, err := folders.FindDirWithLeaf(path, GitDirectoryName) if err != nil { if !os.IsNotExist(err) { return nil, err From 6c5d86d719fcf9d300b6a72ad72c4fa2c4413659 Mon Sep 17 00:00:00 2001 From: monalisa Date: Mon, 7 Aug 2023 14:43:36 +0200 Subject: [PATCH 3/4] - --- libs/git/repository.go | 1 + 1 file changed, 1 insertion(+) diff --git a/libs/git/repository.go b/libs/git/repository.go index f55d9f9438..2f19cff98f 100644 --- a/libs/git/repository.go +++ b/libs/git/repository.go @@ -11,6 +11,7 @@ import ( ) const gitIgnoreFileName = ".gitignore" + var GitDirectoryName = ".git" // Repository represents a Git repository or a directory From bf9e8f5221de0c83ab07f187820598261f5fbdf1 Mon Sep 17 00:00:00 2001 From: monalisa Date: Mon, 7 Aug 2023 17:51:44 +0200 Subject: [PATCH 4/4] addressed comment --- bundle/config/mutator/load_git_details.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/bundle/config/mutator/load_git_details.go b/bundle/config/mutator/load_git_details.go index ecb4cd282f..ab47677dd3 100644 --- a/bundle/config/mutator/load_git_details.go +++ b/bundle/config/mutator/load_git_details.go @@ -27,13 +27,13 @@ func (m *loadGitDetails) Apply(ctx context.Context, b *bundle.Bundle) error { // Read branch name of current checkout branch, err := repo.CurrentBranch() - if err == nil && b.Config.Bundle.Git.Branch == "" { - // Only load branch if there's no user defined value - b.Config.Bundle.Git.Inferred = true - b.Config.Bundle.Git.Branch = branch - } if err == nil { b.Config.Bundle.Git.ActualBranch = branch + if b.Config.Bundle.Git.Branch == "" { + // Only load branch if there's no user defined value + b.Config.Bundle.Git.Inferred = true + b.Config.Bundle.Git.Branch = branch + } } else { log.Warnf(ctx, "failed to load current branch: %s", err) }