Skip to content

Commit

Permalink
Fix bundle git branch validation (#645)
Browse files Browse the repository at this point in the history
## Changes
This PR:
1. Fixes the computation logic for `ActualBranch`. An error in the
earlier logic caused the validation mutator to be a no-op.
2. Makes the `.git` string a global var. This is useful to configure in
tests.
3. Adds e2e test for the validation mutator.

## Tests
Unit test
  • Loading branch information
shreyas-goenka authored Aug 7, 2023
1 parent 81ee031 commit d6f6269
Show file tree
Hide file tree
Showing 6 changed files with 61 additions and 32 deletions.
19 changes: 11 additions & 8 deletions bundle/config/mutator/load_git_details.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

// Read branch name of current checkout
branch, err := repo.CurrentBranch()
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)
}

// load commit hash if undefined
if b.Config.Bundle.Git.Commit == "" {
commit, err := repo.LatestCommit()
Expand Down
20 changes: 0 additions & 20 deletions bundle/tests/autoload_git_test.go

This file was deleted.

1 change: 1 addition & 0 deletions bundle/tests/git_branch_validation/.mock-git/HEAD
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
ref: refs/heads/feature-b
4 changes: 4 additions & 0 deletions bundle/tests/git_branch_validation/databricks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
bundle:
name: "Dancing Feet"
git:
branch: "feature-a"
39 changes: 39 additions & 0 deletions bundle/tests/git_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package config_tests

import (
"context"
"testing"

"github.com/databricks/cli/bundle"
"github.com/databricks/cli/bundle/config/mutator"
"github.com/databricks/cli/libs/git"
"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) {
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, "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:")
}
10 changes: 6 additions & 4 deletions libs/git/repository.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ import (

const gitIgnoreFileName = ".gitignore"

var GitDirectoryName = ".git"

// Repository represents a Git repository or a directory
// that could later be initialized as Git repository.
type Repository struct {
Expand Down Expand Up @@ -45,7 +47,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
}
Expand All @@ -62,7 +64,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
}
Expand All @@ -81,7 +83,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
}
Expand Down Expand Up @@ -186,7 +188,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
Expand Down

0 comments on commit d6f6269

Please sign in to comment.