From 442ddfc3176f50ef463faecb0152397dc861df38 Mon Sep 17 00:00:00 2001 From: Hardy Ferentschik Date: Thu, 20 Jun 2019 00:19:46 +0200 Subject: [PATCH] fix: making sure the right Run function is called for step git merge (#4336) fixes issue #4334 Signed-off-by: Hardy Ferentschik --- go.mod | 1 + pkg/cmd/opts/step.go | 11 -- pkg/cmd/step/create/step_create_task.go | 3 +- pkg/cmd/step/git/step_git_merge.go | 17 ++- pkg/cmd/step/git/step_git_merge_test.go | 131 ++++++++++++++++++++++++ pkg/gits/testhelpers.go | 99 ++++++++++++++++++ 6 files changed, 245 insertions(+), 17 deletions(-) create mode 100644 pkg/cmd/step/git/step_git_merge_test.go create mode 100644 pkg/gits/testhelpers.go diff --git a/go.mod b/go.mod index b86506aa58..3d9f1c4e4e 100644 --- a/go.mod +++ b/go.mod @@ -87,6 +87,7 @@ require ( github.com/nlopes/slack v0.0.0-20180721202243-347a74b1ea30 github.com/nwaples/rardecode v1.0.0 // indirect github.com/onsi/ginkgo v1.7.0 + github.com/onsi/gomega v1.4.3 github.com/opencontainers/runc v0.1.1 // indirect github.com/ory/dockertest v3.3.4+incompatible // indirect github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c // indirect diff --git a/pkg/cmd/opts/step.go b/pkg/cmd/opts/step.go index ecd3de5977..bbf50f71d8 100644 --- a/pkg/cmd/opts/step.go +++ b/pkg/cmd/opts/step.go @@ -13,14 +13,3 @@ type StepOptions struct { func (o *StepOptions) Run() error { return o.Cmd.Help() } - -// StepGitMergeOptions contains the command line flags -type StepGitMergeOptions struct { - StepOptions - - SHAs []string - Remote string - Dir string - BaseBranch string - BaseSHA string -} diff --git a/pkg/cmd/step/create/step_create_task.go b/pkg/cmd/step/create/step_create_task.go index 5ea2f9ced3..e3427732e2 100644 --- a/pkg/cmd/step/create/step_create_task.go +++ b/pkg/cmd/step/create/step_create_task.go @@ -2,6 +2,7 @@ package create import ( "fmt" + "github.com/jenkins-x/jx/pkg/cmd/step/git" "io/ioutil" "os" "path/filepath" @@ -948,7 +949,7 @@ func (o *StepCreateTaskOptions) mergePullRefs(cloneDir string) (*prow.PullRefs, shas = append(shas, sha) } - mergeOpts := opts.StepGitMergeOptions{ + mergeOpts := git.StepGitMergeOptions{ StepOptions: opts.StepOptions{ CommonOptions: o.CommonOptions, }, diff --git a/pkg/cmd/step/git/step_git_merge.go b/pkg/cmd/step/git/step_git_merge.go index 045b501336..1a97742f38 100644 --- a/pkg/cmd/step/git/step_git_merge.go +++ b/pkg/cmd/step/git/step_git_merge.go @@ -41,13 +41,20 @@ master:ef08a6cd194c2687d4bc12df6bb8a86f53c348ba,2739:5b351f4eae3c4afbb90dd7787f8 `) ) -type StepGitMergeCommand struct { - opts.StepGitMergeOptions +// StepGitMergeOptions contains the command line flags +type StepGitMergeOptions struct { + opts.StepOptions + + SHAs []string + Remote string + Dir string + BaseBranch string + BaseSHA string } // NewCmdStepGitMerge create the 'step git envs' command func NewCmdStepGitMerge(commonOpts *opts.CommonOptions) *cobra.Command { - options := opts.StepGitMergeOptions{ + options := StepGitMergeOptions{ StepOptions: opts.StepOptions{ CommonOptions: commonOpts, }, @@ -78,7 +85,7 @@ func NewCmdStepGitMerge(commonOpts *opts.CommonOptions) *cobra.Command { } // Run implements the command -func (o *StepGitMergeCommand) Run() error { +func (o *StepGitMergeOptions) Run() error { if o.Remote == "" { o.Remote = "origin" } @@ -117,7 +124,7 @@ func (o *StepGitMergeCommand) Run() error { return gits.FetchAndMergeSHAs(o.SHAs, o.BaseBranch, o.BaseSHA, o.Remote, o.Dir, o.Git(), o.Verbose) } -func (o *StepGitMergeCommand) setGitConfig() error { +func (o *StepGitMergeOptions) setGitConfig() error { user, err := o.GetCommandOutput(o.Dir, "git", "config", "user.name") if err != nil || user == "" { err := o.RunCommandFromDir(o.Dir, "git", "config", "user.name", "jenkins-x") diff --git a/pkg/cmd/step/git/step_git_merge_test.go b/pkg/cmd/step/git/step_git_merge_test.go new file mode 100644 index 0000000000..9606ebc473 --- /dev/null +++ b/pkg/cmd/step/git/step_git_merge_test.go @@ -0,0 +1,131 @@ +package git + +import ( + "fmt" + "github.com/jenkins-x/jx/pkg/cmd/opts" + "github.com/jenkins-x/jx/pkg/gits" + "github.com/jenkins-x/jx/pkg/log" + . "github.com/onsi/ginkgo" + . "github.com/onsi/gomega" + "io/ioutil" + "os" + "testing" +) + +func TestStepGitMerge(t *testing.T) { + RegisterFailHandler(Fail) + RunSpecs(t, "Step Git Merge Suite") +} + +var _ = Describe("step git merge", func() { + var ( + masterSha string + branchSha string + repoDir string + err error + ) + + BeforeEach(func() { + repoDir, err = ioutil.TempDir("", "jenkins-x-git-test-repo-") + if err != nil { + Fail("unable to create test repo dir") + } + log.Logger().Debugf("created temporary git repo under %s", repoDir) + gits.GitCmd(Fail, repoDir, "init") + + gits.WriteFile(Fail, repoDir, "a.txt", "a") + gits.Add(Fail, repoDir) + masterSha = gits.Commit(Fail, repoDir, "a commit") + + gits.Branch(Fail, repoDir, "foo") + gits.WriteFile(Fail, repoDir, "b.txt", "b") + gits.Add(Fail, repoDir) + branchSha = gits.Commit(Fail, repoDir, "b commit") + + gits.Checkout(Fail, repoDir, "master") + }) + + AfterEach(func() { + _ = os.RemoveAll(repoDir) + log.Logger().Debugf("deleted temporary git repo under %s", repoDir) + }) + + Context("with command line options", func() { + It("succeeds", func() { + currentHeadSha := gits.HeadSha(Fail, repoDir) + Expect(currentHeadSha).Should(Equal(masterSha)) + + options := StepGitMergeOptions{ + StepOptions: opts.StepOptions{ + CommonOptions: &opts.CommonOptions{}, + }, + SHAs: []string{branchSha}, + Dir: repoDir, + BaseBranch: "master", + BaseSHA: masterSha, + } + + err := options.Run() + Expect(err).NotTo(HaveOccurred()) + + currentHeadSha = gits.HeadSha(Fail, repoDir) + Expect(currentHeadSha).Should(Equal(branchSha)) + }) + }) + + Context("with PULL_REFS", func() { + BeforeEach(func() { + err := os.Setenv("PULL_REFS", fmt.Sprintf("master:%s,foo:%s", masterSha, branchSha)) + if err != nil { + Fail("unable to set PULL_REFS") + } + + }) + + AfterEach(func() { + err := os.Unsetenv("PULL_REFS") + if err != nil { + Fail("unable to unset PULL_REFS") + } + }) + + It("succeeds", func() { + currentHeadSha := gits.HeadSha(Fail, repoDir) + Expect(currentHeadSha).Should(Equal(masterSha)) + + options := StepGitMergeOptions{ + StepOptions: opts.StepOptions{ + CommonOptions: &opts.CommonOptions{}, + }, + Dir: repoDir, + } + + err := options.Run() + Expect(err).NotTo(HaveOccurred()) + + currentHeadSha = gits.HeadSha(Fail, repoDir) + Expect(currentHeadSha).Should(Equal(branchSha)) + }) + }) + + Context("with no options and no PULL_REFS", func() { + It("logs warning", func() { + options := StepGitMergeOptions{ + StepOptions: opts.StepOptions{ + CommonOptions: &opts.CommonOptions{}, + }, + Dir: repoDir, + } + + out := log.CaptureOutput(func() { + err := options.Run() + Expect(err).NotTo(HaveOccurred()) + + currentHeadSha := gits.HeadSha(Fail, repoDir) + Expect(currentHeadSha).Should(Equal(masterSha)) + }) + + Expect(out).Should(ContainSubstring("no SHAs to merge")) + }) + }) +}) diff --git a/pkg/gits/testhelpers.go b/pkg/gits/testhelpers.go new file mode 100644 index 0000000000..b46fa7f2ff --- /dev/null +++ b/pkg/gits/testhelpers.go @@ -0,0 +1,99 @@ +package gits + +import ( + "github.com/jenkins-x/jx/pkg/log" + "github.com/jenkins-x/jx/pkg/util" + "io/ioutil" + "os" + "path/filepath" + "strings" +) + +// WriteFile creates a file with the specified name underneath the gitRepo directory adding the specified content. +// The file name can be path as well and intermediate directories are created. +func WriteFile(fail func(string, ...int), repoDir string, name string, contents string) { + path := filepath.Join(repoDir, name) + err := os.MkdirAll(filepath.Dir(path), 0755) + if err != nil { + log.Logger().Error(err.Error()) + fail("unable to create directory") + } + + b := []byte(contents) + err = ioutil.WriteFile(path, b, 0644) + if err != nil { + log.Logger().Error(err.Error()) + fail("unable to write file content") + } +} + +// HeadSha returns the commit SHA of the current HEAD commit within the specified git directory +func HeadSha(fail func(string, ...int), repoDir string) string { + data, err := ioutil.ReadFile(filepath.Join(repoDir, ".git", "HEAD")) + if err != nil { + log.Logger().Error(err.Error()) + fail("unable to read file") + } + + headRef := strings.TrimPrefix(string(data), "ref: ") + headRef = strings.Trim(headRef, "\n") + + return ReadRef(fail, repoDir, headRef) +} + +// ReadRef reads the commit SHA of the specified ref. Needs to be of the form /refs/heads/. +func ReadRef(fail func(string, ...int), repoDir string, name string) string { + data, err := ioutil.ReadFile(filepath.Join(repoDir, ".git", name)) + if err != nil { + log.Logger().Error(err.Error()) + fail("unable to read file") + } + return strings.Trim(string(data), "\n") +} + +// Add adds all unstaged changes to the index. +func Add(fail func(string, ...int), repoDir string) { + GitCmd(fail, repoDir, "add", ".") +} + +// Commit commits all staged changes with the specified commit message. +func Commit(fail func(string, ...int), repoDir string, message string) string { + GitCmd(fail, repoDir, "commit", "-m", message) + return HeadSha(fail, repoDir) +} + +// Tag creates an annotated tag. +func Tag(fail func(string, ...int), repoDir string, tag string, message string) string { + GitCmd(fail, repoDir, "tag", "-a", "-m", message, tag) + return HeadSha(fail, repoDir) +} + +// Checkout switches to the specified branch. +func Checkout(fail func(string, ...int), repoDir string, branch string) { + GitCmd(fail, repoDir, "checkout", branch) +} + +// Branch creates a new branch with the specified name. +func Branch(fail func(string, ...int), repoDir string, branch string) { + GitCmd(fail, repoDir, "checkout", "-b", branch) +} + +// DetachHead puts the repository in a detached head mode. +func DetachHead(fail func(string, ...int), repoDir string) { + head := HeadSha(fail, repoDir) + GitCmd(fail, repoDir, "checkout", head) +} + +// GitCmd runs a git command with arguments in the specified git repository +func GitCmd(fail func(string, ...int), repoDir string, args ...string) { + cmd := util.Command{ + Dir: repoDir, + Name: "git", + Args: args, + } + _, err := cmd.RunWithoutRetry() + if err != nil { + log.Logger().Error(err.Error()) + fail("unable to write file content") + } +}