diff --git a/pkg/command/command.go b/pkg/command/command.go index e1ec71fe178..8acbef55ff4 100644 --- a/pkg/command/command.go +++ b/pkg/command/command.go @@ -40,16 +40,30 @@ type Status struct { output string } -// New creates a new command from provided string, which contains a whitespace -// sepearated shell command +// New creates a new command from the provided arguments. func New(cmd ...string) *Command { + return NewWithWorkDir("", cmd...) +} + +// New creates a new command from the provided workDir and the command +// arguments. +func NewWithWorkDir(workDir string, cmd ...string) *Command { args := strings.Fields(strings.Join(cmd, " ")) - if len(args) == 0 { - return &Command{exec.Command(cmd[0])} - } else if len(args) > 1 { - return &Command{exec.Command(args[0], args[1:]...)} + + command := func() *Command { + if len(args) == 0 { + return &Command{exec.Command(cmd[0])} + } else if len(args) > 1 { + return &Command{exec.Command(args[0], args[1:]...)} + } + return &Command{exec.Command(args[0])} + }() + + if workDir != "" { + command.cmd.Dir = workDir } - return &Command{exec.Command(args[0])} + + return command } // Run starts the command and waits for it to finish. It returns an error if @@ -59,6 +73,19 @@ func (c *Command) Run() (res *Status, err error) { return c.run(true) } +// Run starts the command and waits for it to finish. It returns an error if +// the command execution was not successful. +func (c *Command) RunSuccess() (err error) { + res, err := c.run(true) + if err != nil { + return err + } + if !res.Success() { + return errors.Errorf("command %v did not succeed", c.cmd) + } + return nil +} + // Run starts the command and waits for it to finish. It returns an error if // the command execution was not possible at all, otherwise the Status. // This method does not print the output of the command during its execution. @@ -66,6 +93,20 @@ func (c *Command) RunSilent() (res *Status, err error) { return c.run(false) } +// Run starts the command and waits for it to finish. It returns an error if +// the command execution was not successful. +// This method does not print the output of the command during its execution. +func (c *Command) RunSilentSuccess() (err error) { + res, err := c.run(false) + if err != nil { + return err + } + if !res.Success() { + return errors.Errorf("command %v did not succeed", c.cmd) + } + return nil +} + // run is the internal run method func (c *Command) run(print bool) (res *Status, err error) { log.Printf("Running command: %v", c.cmd) @@ -140,7 +181,7 @@ func (s *Status) Output() string { return s.output } -// Execute is a convinience function which creates a new Command, excutes it +// Execute is a convenience function which creates a new Command, executes it // and evaluates its status. func Execute(cmd ...string) error { status, err := New(cmd...).Run() diff --git a/pkg/command/command_test.go b/pkg/command/command_test.go index ebb55d156b4..dc910f35952 100644 --- a/pkg/command/command_test.go +++ b/pkg/command/command_test.go @@ -13,6 +13,18 @@ func TestSuccess(t *testing.T) { require.Zero(t, res.ExitCode()) } +func TestSuccessWithWorkingDir(t *testing.T) { + res, err := NewWithWorkDir("/", "ls -1").Run() + require.Nil(t, err) + require.True(t, res.Success()) + require.Zero(t, res.ExitCode()) +} + +func TestFailureWithWrongWorkingDir(t *testing.T) { + _, err := NewWithWorkDir("/should/not/exist", "ls -1").Run() + require.NotNil(t, err) +} + func TestSuccessSilent(t *testing.T) { res, err := New("echo hi").RunSilent() require.Nil(t, err) @@ -86,3 +98,19 @@ func TestAvailableFailure(t *testing.T) { res := Available("echo", "this-command-should-not-exist") require.False(t, res) } + +func TestSuccessRunSuccess(t *testing.T) { + require.Nil(t, New("echo hi").RunSuccess()) +} + +func TestFailureRunSuccess(t *testing.T) { + require.NotNil(t, New("cat /not/available").RunSuccess()) +} + +func TestSuccessRunSilentSuccess(t *testing.T) { + require.Nil(t, New("echo hi").RunSilentSuccess()) +} + +func TestFailureRunSuccessSilent(t *testing.T) { + require.NotNil(t, New("cat /not/available").RunSilentSuccess()) +} diff --git a/pkg/git/BUILD.bazel b/pkg/git/BUILD.bazel index d7a3a47925e..5e5666331d6 100644 --- a/pkg/git/BUILD.bazel +++ b/pkg/git/BUILD.bazel @@ -1,4 +1,4 @@ -load("@io_bazel_rules_go//go:def.bzl", "go_library") +load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test") go_library( name = "go_default_library", @@ -31,3 +31,16 @@ filegroup( tags = ["automanaged"], visibility = ["//visibility:public"], ) + +go_test( + name = "go_default_test", + srcs = ["git_test.go"], + embed = [":go_default_library"], + deps = [ + "//pkg/command:go_default_library", + "@com_github_stretchr_testify//require:go_default_library", + "@in_gopkg_src_d_go_git_v4//:go_default_library", + "@in_gopkg_src_d_go_git_v4//config:go_default_library", + "@in_gopkg_src_d_go_git_v4//plumbing/object:go_default_library", + ], +) diff --git a/pkg/git/git.go b/pkg/git/git.go index 56f6f0f1c38..f0e1cb18039 100644 --- a/pkg/git/git.go +++ b/pkg/git/git.go @@ -191,8 +191,8 @@ func (r *Repo) RevParse(rev string) (string, error) { return ref.String(), nil } -// RevParseShort parses a git revision and returns a SHA1 on success, otherwise -// an error. +// RevParseShort parses a git revision and returns a SHA1 trimmed to the length +// 10 on success, otherwise an error. func (r *Repo) RevParseShort(rev string) (string, error) { fullRev, err := r.RevParse(rev) if err != nil { @@ -319,7 +319,7 @@ func (r *Repo) CheckoutBranch(name string) error { func IsReleaseBranch(branch string) bool { re := regexp.MustCompile(branchRE) if !re.MatchString(branch) { - log.Fatalf("%s is not a release branch\n", branch) + log.Printf("%s is not a release branch", branch) return false } @@ -373,30 +373,14 @@ func Remotify(name string) string { return fmt.Sprintf("%s/%s", DefaultRemote, name) } -// switchToRepoDir changes into the repo dir and returning the old one for -// restoring it -func (r *Repo) switchToRepoDir() (string, error) { - oldWd, err := os.Getwd() - if err != nil { - return "", err - } - if err := os.Chdir(r.dir); err != nil { - return "", err - } - return oldWd, nil -} - // DescribeTag can be used to retrieve the latest tag for a provided revision func (r *Repo) DescribeTag(rev string) (string, error) { - oldWd, err := r.switchToRepoDir() - if err != nil { - return "", err - } - // go git seems to have no implementation for `git describe` // which means that we fallback to a shell command for sake of // simplicity - status, err := command.New("git describe --abbrev=0 --tags", rev).RunSilent() + status, err := command.NewWithWorkDir( + r.Dir(), "git describe --abbrev=0 --tags", rev, + ).RunSilent() if err != nil { return "", err } @@ -404,55 +388,28 @@ func (r *Repo) DescribeTag(rev string) (string, error) { return "", errors.New("git describe command failed") } - // Restore working directory - if err := os.Chdir(oldWd); err != nil { - return "", err - } - return strings.TrimSpace(status.Output()), nil } // Merge does a git merge into the current branch from the provided one func (r *Repo) Merge(from string) error { - oldWd, err := r.switchToRepoDir() - if err != nil { - return err - } - - status, err := command.New("git merge -X ours", from).Run() - if err != nil { - return err - } - if !status.Success() { - return errors.New("git merge command failed") - } - - return os.Chdir(oldWd) + return command.NewWithWorkDir( + r.Dir(), "git merge -X ours", from, + ).RunSuccess() } // Push does push the specified branch to the default remote, but only if the // repository is not in dry run mode func (r *Repo) Push(remoteBranch string) error { - oldWd, err := r.switchToRepoDir() - if err != nil { - return err - } - dryRunFlag := "" if r.dryRun { log.Println("Won't push due to dry run repository") dryRunFlag = "--dry-run" } - status, err := command.New("git push", dryRunFlag, DefaultRemote, remoteBranch).Run() - if err != nil { - return err - } - if !status.Success() { - return errors.New("git push command failed") - } - - return os.Chdir(oldWd) + return command.NewWithWorkDir( + r.Dir(), "git push", dryRunFlag, DefaultRemote, remoteBranch, + ).RunSuccess() } // Head retrieves the current repository HEAD as a string diff --git a/pkg/git/git_test.go b/pkg/git/git_test.go new file mode 100644 index 00000000000..e9ef721617b --- /dev/null +++ b/pkg/git/git_test.go @@ -0,0 +1,311 @@ +/* +Copyright 2019 The Kubernetes Authors. + +Licensed under the Apache License, Version 2.0 (the "License"); +you may not use this file except in compliance with the License. +You may obtain a copy of the License at + + http://www.apache.org/licenses/LICENSE-2.0 + +Unless required by applicable law or agreed to in writing, software +distributed under the License is distributed on an "AS IS" BASIS, +WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +See the License for the specific language governing permissions and +limitations under the License. +*/ + +package git + +import ( + "io/ioutil" + "os" + "path/filepath" + "testing" + "time" + + "k8s.io/release/pkg/command" + + "github.com/stretchr/testify/require" + "gopkg.in/src-d/go-git.v4" + "gopkg.in/src-d/go-git.v4/config" + "gopkg.in/src-d/go-git.v4/plumbing/object" +) + +const master = "master" + +type testRepo struct { + sut *Repo + dir string + firstCommit string + branchCommit string + branchName string + tagCommit string + tagName string +} + +// newTestRepo creates a test repo with the following structure: +// +// * commit `branchCommit` (HEAD -> first-branch, origin/first-branch) +// | Author: John Doe +// | +// | Second commit +// | +// * commit `firstCommit` (tag: v0.1.0, origin/master, origin/HEAD, master) +// Author: John Doe +// +// First commit +// +func newTestRepo(t *testing.T) *testRepo { + // Setup the bare repo as base + bareTempDir, err := ioutil.TempDir("", "k8s-test-bare-") + require.Nil(t, err) + + bareRepo, err := git.PlainInit(bareTempDir, true) + require.Nil(t, err) + require.NotNil(t, bareRepo) + + // Clone from the bare to be able to add our test data + cloneTempDir, err := ioutil.TempDir("", "k8s-test-clone-") + require.Nil(t, err) + cloneRepo, err := git.PlainInit(cloneTempDir, false) + require.Nil(t, err) + + // Add the test data set + const testFileName = "test-file" + require.Nil(t, ioutil.WriteFile( + filepath.Join(cloneTempDir, testFileName), + []byte("test-content"), + 0644, + )) + + worktree, err := cloneRepo.Worktree() + require.Nil(t, err) + _, err = worktree.Add(testFileName) + require.Nil(t, err) + + author := &object.Signature{ + Name: "John Doe", + Email: "john@doe.org", + When: time.Now(), + } + firstCommit, err := worktree.Commit("First commit", &git.CommitOptions{ + Author: author, + }) + require.Nil(t, err) + + tagName := "v0.1.0" + tagRef, err := cloneRepo.CreateTag(tagName, firstCommit, + &git.CreateTagOptions{ + Tagger: author, + Message: tagName, + }, + ) + require.Nil(t, err) + + // Create a test branch and a test commit on top + branchName := "first-branch" + require.Nil(t, command.NewWithWorkDir( + cloneTempDir, "git checkout -b", branchName, + ).RunSuccess()) + + const branchTestFileName = "branch-test-file" + require.Nil(t, ioutil.WriteFile( + filepath.Join(cloneTempDir, branchTestFileName), + []byte("test-content"), + 0644, + )) + _, err = worktree.Add(branchTestFileName) + require.Nil(t, err) + branchCommit, err := worktree.Commit("Second commit", &git.CommitOptions{ + Author: author, + }) + require.Nil(t, err) + + // Push the test content into the bare repo + _, err = cloneRepo.CreateRemote(&config.RemoteConfig{ + Name: DefaultRemote, + URLs: []string{bareTempDir}, + }) + require.Nil(t, err) + require.Nil(t, cloneRepo.Push(&git.PushOptions{ + RefSpecs: []config.RefSpec{"refs/*:refs/*"}, + })) + + require.Nil(t, os.RemoveAll(cloneTempDir)) + + // Provide a system under test inside the test repo + sut, err := CloneOrOpenRepo("", bareTempDir, false) + require.Nil(t, err) + + return &testRepo{ + sut: sut, + dir: bareTempDir, + firstCommit: firstCommit.String(), + branchCommit: branchCommit.String(), + branchName: branchName, + tagName: tagName, + tagCommit: tagRef.Hash().String(), + } +} + +func (r *testRepo) cleanup(t *testing.T) { + require.Nil(t, os.RemoveAll(r.dir)) + require.Nil(t, os.RemoveAll(r.sut.Dir())) +} + +func TestSuccessCloneOrOpen(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + secondRepo, err := CloneOrOpenRepo(testRepo.sut.Dir(), testRepo.dir, false) + require.Nil(t, err) + + require.Equal(t, testRepo.sut.Dir(), secondRepo.Dir()) + require.Nil(t, secondRepo.Cleanup()) +} + +func TestSuccessDescribeTag(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + tag, err := testRepo.sut.DescribeTag(testRepo.tagCommit) + require.Nil(t, err) + require.Equal(t, tag, testRepo.tagName) +} + +func TestFailureDescribeTag(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + _, err := testRepo.sut.DescribeTag("wrong") + require.NotNil(t, err) +} + +func TestSuccessHasRemoteBranch(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + require.Nil(t, testRepo.sut.HasRemoteBranch(testRepo.branchName)) + require.Nil(t, testRepo.sut.HasRemoteBranch(master)) +} + +func TestFailureHasRemoteBranch(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + err := testRepo.sut.HasRemoteBranch("wrong") + require.NotNil(t, err) +} + +func TestSuccessHead(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + head, err := testRepo.sut.Head() + require.Nil(t, err) + require.Equal(t, head, testRepo.firstCommit) +} + +func TestSuccessMerge(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + err := testRepo.sut.Merge(master) + require.Nil(t, err) +} + +func TestFailureMerge(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + err := testRepo.sut.Merge(testRepo.branchName) + require.NotNil(t, err) +} + +func TestSuccessMergeBase(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + mergeBase, err := testRepo.sut.MergeBase(master, testRepo.branchName) + require.Nil(t, err) + require.Equal(t, mergeBase, testRepo.firstCommit) +} + +func TestSuccessRevParse(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + masterRev, err := testRepo.sut.RevParse(master) + require.Nil(t, err) + require.Equal(t, masterRev, testRepo.firstCommit) + + branchRev, err := testRepo.sut.RevParse(testRepo.branchName) + require.Nil(t, err) + require.Equal(t, branchRev, testRepo.branchCommit) + + tagRev, err := testRepo.sut.RevParse(testRepo.tagName) + require.Nil(t, err) + require.Equal(t, tagRev, testRepo.firstCommit) +} + +func TestFailureRevParse(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + _, err := testRepo.sut.RevParse("wrong") + require.NotNil(t, err) +} + +func TestSuccessRevParseShort(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + masterRev, err := testRepo.sut.RevParseShort(master) + require.Nil(t, err) + require.Equal(t, masterRev, testRepo.firstCommit[:10]) + + branchRev, err := testRepo.sut.RevParseShort(testRepo.branchName) + require.Nil(t, err) + require.Equal(t, branchRev, testRepo.branchCommit[:10]) + + tagRev, err := testRepo.sut.RevParseShort(testRepo.tagName) + require.Nil(t, err) + require.Equal(t, tagRev, testRepo.firstCommit[:10]) +} + +func TestFailureRevParseShort(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + _, err := testRepo.sut.RevParseShort("wrong") + require.NotNil(t, err) +} + +func TestSuccessPush(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + err := testRepo.sut.Push(master) + require.Nil(t, err) +} + +func TestFailurePush(t *testing.T) { + testRepo := newTestRepo(t) + defer testRepo.cleanup(t) + + err := testRepo.sut.Push("wrong") + require.NotNil(t, err) +} + +func TestSuccessRemotify(t *testing.T) { + newRemote := Remotify(master) + require.Equal(t, newRemote, DefaultRemote+"/"+master) +} + +func TestSuccessIsReleaseBranch(t *testing.T) { + require.True(t, IsReleaseBranch("release-1.17")) +} + +func TestFailureIsReleaseBranch(t *testing.T) { + require.False(t, IsReleaseBranch("some-branch")) +}