From d8679ea03c62aae8417f8f2d4d3552afaf7ed228 Mon Sep 17 00:00:00 2001 From: John Dewey Date: Sun, 5 Aug 2018 18:14:21 -0700 Subject: [PATCH] Mocked out the network Moved mockRunCommand to the git package. This feels very wrong, but maybe this is the golang way, eh? However, this patch cleans up a ton of duplication in our tests, which I like more than the parts I dislike. --- git/git_public_test.go | 49 +++++----------- git/git_test.go | 52 ++++++----------- git/mocked.go | 73 ++++++++++++++++++++++++ repositories/repositories_public_test.go | 60 +++++++++++++------ repositories/repositories_test.go | 1 + 5 files changed, 147 insertions(+), 88 deletions(-) create mode 100644 git/mocked.go diff --git a/git/git_public_test.go b/git/git_public_test.go index b651a54..06ec1d0 100644 --- a/git/git_public_test.go +++ b/git/git_public_test.go @@ -24,9 +24,7 @@ import ( "errors" "fmt" "os" - "os/exec" "path/filepath" - "strings" "testing" "github.com/retr0h/go-gilt/git" @@ -62,33 +60,6 @@ func (suite *GitTestSuite) TearDownTest() { testutil.RemoveTempDirectory(suite.r.GiltDir) } -func (suite *GitTestSuite) mockRunCommand(f func()) []string { - var got []string - - originalRunCommand := git.RunCommand - git.RunCommand = func(debug bool, name string, args ...string) error { - cmd := exec.Command(name, args...) - got = append(got, strings.Join(cmd.Args, " ")) - - return nil - } - defer func() { git.RunCommand = originalRunCommand }() - - f() - - return got -} - -func (suite *GitTestSuite) mockRunCommandErrors(f func() error) { - originalRunCommand := git.RunCommand - git.RunCommand = func(debug bool, name string, args ...string) error { - return errors.New("RunCommand had an error") - } - defer func() { git.RunCommand = originalRunCommand }() - - f() -} - func (suite *GitTestSuite) TestCloneAlreadyExists() { cloneDir := filepath.Join(suite.r.GiltDir, "https---example.com-user-repo.git-abc1234") if _, err := os.Stat(cloneDir); os.IsNotExist(err) { @@ -141,9 +112,14 @@ func (suite *GitTestSuite) TestCloneErrorsOnResetReturnsError() { } func (suite *GitTestSuite) TestClone() { - anon := func() { suite.c.Clone(suite.r) } + anon := func() error { + err := suite.c.Clone(suite.r) + assert.NoError(suite.T(), err) + + return err + } - got := suite.mockRunCommand(anon) + got := git.MockRunCommand(anon) want := []string{ fmt.Sprintf("git clone https://example.com/user/repo.git %s/https---example.com-user-repo.git-abc1234", suite.r.GiltDir), @@ -173,14 +149,19 @@ func (suite *GitTestSuite) TestCheckoutIndexFailsCheckoutIndexReturnsError() { return err } - suite.mockRunCommandErrors(anon) + git.MockRunCommandErrorsOn("git", anon) } func (suite *GitTestSuite) TestCheckoutIndex() { - anon := func() { suite.g.CheckoutIndex(suite.r) } + anon := func() error { + err := suite.g.CheckoutIndex(suite.r) + assert.NoError(suite.T(), err) + + return err + } dstDir, _ := git.FilePathAbs(suite.r.Dst) - got := suite.mockRunCommand(anon) + got := git.MockRunCommand(anon) want := []string{ fmt.Sprintf("git -C %s/https---example.com-user-repo.git-abc1234 checkout-index --force --all --prefix %s", suite.r.GiltDir, (dstDir + string(os.PathSeparator))), diff --git a/git/git_test.go b/git/git_test.go index 28453ec..e1bd88b 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -21,10 +21,7 @@ package git import ( - "errors" "fmt" - "os/exec" - "strings" "testing" "github.com/retr0h/go-gilt/repository" @@ -59,33 +56,6 @@ func (suite *GitTestSuite) TearDownTest() { testutil.RemoveTempDirectory(suite.r.GiltDir) } -func (suite *GitTestSuite) mockRunCommand(f func()) []string { - var got []string - - originalRunCommand := RunCommand - RunCommand = func(debug bool, name string, args ...string) error { - cmd := exec.Command(name, args...) - got = append(got, strings.Join(cmd.Args, " ")) - - return nil - } - defer func() { RunCommand = originalRunCommand }() - - f() - - return got -} - -func (suite *GitTestSuite) mockRunCommandErrors(f func() error) { - originalRunCommand := RunCommand - RunCommand = func(debug bool, name string, args ...string) error { - return errors.New("RunCommand had an error") - } - defer func() { RunCommand = originalRunCommand }() - - f() -} - func (suite *GitTestSuite) TestCloneReturnsError() { anon := func() error { err := suite.g.clone(suite.r) @@ -94,13 +64,18 @@ func (suite *GitTestSuite) TestCloneReturnsError() { return err } - suite.mockRunCommandErrors(anon) + MockRunCommandErrorsOn("git", anon) } func (suite *GitTestSuite) TestClone() { - anon := func() { suite.g.clone(suite.r) } + anon := func() error { + err := suite.g.clone(suite.r) + assert.NoError(suite.T(), err) + + return err + } - got := suite.mockRunCommand(anon) + got := MockRunCommand(anon) want := []string{ fmt.Sprintf("git clone https://example.com/user/repo.git %s/https---example.com-user-repo.git-abc1234", suite.r.GiltDir), @@ -117,13 +92,18 @@ func (suite *GitTestSuite) TestResetReturnsError() { return err } - suite.mockRunCommandErrors(anon) + MockRunCommandErrorsOn("git", anon) } func (suite *GitTestSuite) TestReset() { - anon := func() { suite.g.reset(suite.r) } + anon := func() error { + err := suite.g.reset(suite.r) + assert.NoError(suite.T(), err) + + return err + } - got := suite.mockRunCommand(anon) + got := MockRunCommand(anon) want := []string{ fmt.Sprintf("git -C %s/https---example.com-user-repo.git-abc1234 reset --hard abc1234", suite.r.GiltDir), diff --git a/git/mocked.go b/git/mocked.go new file mode 100644 index 0000000..98ee4c0 --- /dev/null +++ b/git/mocked.go @@ -0,0 +1,73 @@ +// Copyright (c) 2018 John Dewey + +// Permission is hereby granted, free of charge, to any person obtaining a copy +// of this software and associated documentation files (the "Software"), to +// deal in the Software without restriction, including without limitation the +// rights to use, copy, modify, merge, publish, distribute, sublicense, and/or +// sell copies of the Software, and to permit persons to whom the Software is +// furnished to do so, subject to the following conditions: + +// The above copyright notice and this permission notice shall be included in +// all copies or substantial portions of the Software. + +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR +// IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, +// FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE +// AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER +// LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING +// FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. + +package git + +import ( + "errors" + "os/exec" + "strings" +) + +// MockRunCommandImpl records and return the arguments passed to RunCommand as +// a string. An errString may be passed to force the command to fail with +// an error, if the command executed contains the errString. Useful, when +// mocking functions with multiple calls to RunCommand. +// This seems super wrong to have a test function living in the `git` package. +func MockRunCommandImpl(errString string, f func() error) []string { + var got []string + + originalRunCommand := RunCommand + RunCommand = func(debug bool, name string, args ...string) error { + cmd := exec.Command(name, args...) + cmdString := strings.Join(cmd.Args, " ") + + if errString == "" { + got = append(got, cmdString) + return nil + } + + if strings.Contains(cmdString, errString) { + return errors.New("RunCommand had an error") + } + + // NOTE(retr0h): Never hit this path since this is only used by unit tests + // and we keep an eye on our returns. However, the lack of this path causes + // our code coverage to drop. + return nil + } + defer func() { RunCommand = originalRunCommand }() + + f() + + return got +} + +// MockedRunCommand is sugar around MockRunCommandImpl, and returns +// a string with the arguments passed to RunCommand. +func MockRunCommand(f func() error) []string { + return MockRunCommandImpl("", f) +} + +// MockedRunCommandErrors is sugar around MockedRunCommandImpl and +// returns an error when invoked. +func MockRunCommandErrorsOn(errCmd string, f func() error) { + MockRunCommandImpl(errCmd, f) +} diff --git a/repositories/repositories_public_test.go b/repositories/repositories_public_test.go index 0affe2e..cc41553 100644 --- a/repositories/repositories_public_test.go +++ b/repositories/repositories_public_test.go @@ -22,10 +22,12 @@ package repositories_test import ( "errors" + "fmt" + "os" "path" - "path/filepath" "testing" + "github.com/retr0h/go-gilt/git" "github.com/retr0h/go-gilt/repositories" "github.com/retr0h/go-gilt/test/testutil" "github.com/stretchr/testify/assert" @@ -43,6 +45,7 @@ func (suite *RepositoriesTestSuite) SetupTest() { } func (suite *RepositoriesTestSuite) TearDownTest() { + testutil.RemoveTempDirectory(repositories.GiltDir) } func (suite *RepositoriesTestSuite) TestUnmarshalYAMLDoesNotParseYAMLAndReturnsError() { @@ -101,7 +104,6 @@ func (suite *RepositoriesTestSuite) TestUnmarshalYAMLFile() { assert.NotNil(suite.T(), suite.r.Items[0].Dst) } -// TODO: Mock out runCommand. func (suite *RepositoriesTestSuite) TestOverlayFailsCloneReturnsError() { data := ` --- @@ -110,39 +112,61 @@ func (suite *RepositoriesTestSuite) TestOverlayFailsCloneReturnsError() { dst: path/user.repo ` suite.r.UnmarshalYAML([]byte(data)) - err := suite.r.Overlay() + anon := func() error { + err := suite.r.Overlay() + assert.Error(suite.T(), err) - assert.Error(suite.T(), err) + return err + } + + git.MockRunCommandErrorsOn("git", anon) } -// TODO: Mock out runCommand. func (suite *RepositoriesTestSuite) TestOverlayFailsCheckoutIndexReturnsError() { data := ` --- -- url: https://github.com/retr0h/ansible-etcd.git - version: 77a95b7 +- url: https://example.com/user/repo.git + version: abc1234 dst: /invalid/directory ` suite.r.UnmarshalYAML([]byte(data)) - err := suite.r.Overlay() + anon := func() error { + err := suite.r.Overlay() + assert.Error(suite.T(), err) - assert.Error(suite.T(), err) + return err + } + + git.MockRunCommandErrorsOn("checkout-index", anon) } -// TODO: Mock out runCommand. func (suite *RepositoriesTestSuite) TestOverlay() { data := ` --- -- url: https://github.com/retr0h/ansible-etcd.git - version: 77a95b7 - dst: /tmp/user.repo +- url: https://example.com/user/repo.git + version: abc1234 + dst: path/user.repo ` - cloneDir := filepath.Join(repositories.GiltDir, "https---github.com-retr0h-ansible-etcd.git-77a95b7") suite.r.UnmarshalYAML([]byte(data)) - err := suite.r.Overlay() - - assert.DirExists(suite.T(), cloneDir) - assert.NoError(suite.T(), err) + anon := func() error { + err := suite.r.Overlay() + assert.NoError(suite.T(), err) + + return err + } + + dstDir, _ := git.FilePathAbs(suite.r.Items[0].Dst) + got := git.MockRunCommand(anon) + want := []string{ + fmt.Sprintf("git clone https://example.com/user/repo.git %s/https---example.com-user-repo.git-abc1234", + repositories.GiltDir), + fmt.Sprintf("git -C %s/https---example.com-user-repo.git-abc1234 reset --hard abc1234", + repositories.GiltDir), + fmt.Sprintf("git -C %s/https---example.com-user-repo.git-abc1234 checkout-index --force --all --prefix %s", + repositories.GiltDir, (dstDir + string(os.PathSeparator))), + } + + assert.Equal(suite.T(), want, got) } // In order for `go test` to run this suite, we need to create diff --git a/repositories/repositories_test.go b/repositories/repositories_test.go index e04b4b3..3c580cf 100644 --- a/repositories/repositories_test.go +++ b/repositories/repositories_test.go @@ -42,6 +42,7 @@ func (suite *RepositoriesTestSuite) SetupTest() { } func (suite *RepositoriesTestSuite) TearDownTest() { + testutil.RemoveTempDirectory(GiltDir) } func (suite *RepositoriesTestSuite) TestValidateSchemaHasErrorReturnsError() {