From e0939eefbef8d8d2b03b532401cd9bf8acf12da6 Mon Sep 17 00:00:00 2001 From: Nicolas Simonds Date: Sat, 17 Feb 2024 18:38:38 -0800 Subject: [PATCH] chore: coverage++ Get more error conditions under explicit test, so we can be sure we're doing what we think we are doing. Fixes a sneaky bug where a `RemoveAll` call was not going through Afero, and unit tests could potentially destroy data on the host filesystem --- examples/go-client/go.mod | 10 +-- examples/go-client/go.sum | 31 +++------ internal/repositories/repositories.go | 2 +- .../repositories/repositories_public_test.go | 63 ++++++++++++++++++- internal/repositories/repositories_test.go | 10 +++ internal/repository/copy.go | 6 +- internal/repository/copy_public_test.go | 18 ++++++ 7 files changed, 106 insertions(+), 34 deletions(-) diff --git a/examples/go-client/go.mod b/examples/go-client/go.mod index 4e92a71..41e2658 100644 --- a/examples/go-client/go.mod +++ b/examples/go-client/go.mod @@ -11,14 +11,14 @@ require ( require ( github.com/danjacques/gofslock v0.0.0-20230728142113-ae8f59f9e88b // indirect - github.com/gabriel-vasile/mimetype v1.4.2 // indirect + github.com/gabriel-vasile/mimetype v1.4.3 // indirect github.com/go-playground/locales v0.14.1 // indirect github.com/go-playground/universal-translator v0.18.1 // indirect - github.com/go-playground/validator/v10 v10.17.0 // indirect - github.com/leodido/go-urn v1.2.4 // indirect + github.com/go-playground/validator/v10 v10.18.0 // indirect + github.com/leodido/go-urn v1.4.0 // indirect github.com/spf13/afero v1.11.0 // indirect - golang.org/x/crypto v0.17.0 // indirect - golang.org/x/net v0.19.0 // indirect + golang.org/x/crypto v0.19.0 // indirect + golang.org/x/net v0.21.0 // indirect golang.org/x/sys v0.17.0 // indirect golang.org/x/text v0.14.0 // indirect ) diff --git a/examples/go-client/go.sum b/examples/go-client/go.sum index a7d206a..1b974ec 100644 --- a/examples/go-client/go.sum +++ b/examples/go-client/go.sum @@ -1,48 +1,37 @@ github.com/danjacques/gofslock v0.0.0-20230728142113-ae8f59f9e88b h1:BBkZ6LZYtzMQ2Oo5LkovMmUp0gxAD+AnXzfknZlFTBo= github.com/danjacques/gofslock v0.0.0-20230728142113-ae8f59f9e88b/go.mod h1:9LABMmUSkKzt6FVQNEWdUTM0bz8Bt8MPyEcuZe0Sr8c= -github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc h1:U9qPSI2PIWSS1VwoXQT9A3Wy9MM3WgvqSxFWenqJduM= github.com/davecgh/go-spew v1.1.2-0.20180830191138-d8f796af33cc/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38= -github.com/gabriel-vasile/mimetype v1.4.2 h1:w5qFW6JKBz9Y393Y4q372O9A7cUSequkh1Q7OhCmWKU= -github.com/gabriel-vasile/mimetype v1.4.2/go.mod h1:zApsH/mKG4w07erKIaJPFiX0Tsq9BFQgN3qGY5GnNgA= +github.com/gabriel-vasile/mimetype v1.4.3 h1:in2uUcidCuFcDKtdcBxlR0rJ1+fsokWf+uqxgUFjbI0= +github.com/gabriel-vasile/mimetype v1.4.3/go.mod h1:d8uq/6HKRL6CGdk+aubisF/M5GcPfT7nKyLpA0lbSSk= github.com/go-playground/assert/v2 v2.2.0 h1:JvknZsQTYeFEAhQwI4qEt9cyV5ONwRHC+lYKSsYSR8s= github.com/go-playground/assert/v2 v2.2.0/go.mod h1:VDjEfimB/XKnb+ZQfWdccd7VUvScMdVu0Titje2rxJ4= github.com/go-playground/locales v0.14.1 h1:EWaQ/wswjilfKLTECiXz7Rh+3BjFhfDFKv/oXslEjJA= github.com/go-playground/locales v0.14.1/go.mod h1:hxrqLVvrK65+Rwrd5Fc6F2O76J/NuW9t0sjnWqG1slY= github.com/go-playground/universal-translator v0.18.1 h1:Bcnm0ZwsGyWbCzImXv+pAJnYK9S473LQFuzCbDbfSFY= github.com/go-playground/universal-translator v0.18.1/go.mod h1:xekY+UJKNuX9WP91TpwSH2VMlDf28Uj24BCp08ZFTUY= -github.com/go-playground/validator/v10 v10.17.0 h1:SmVVlfAOtlZncTxRuinDPomC2DkXJ4E5T9gDA0AIH74= -github.com/go-playground/validator/v10 v10.17.0/go.mod h1:9iXMNT7sEkjXb0I+enO7QXmzG6QCsPWY4zveKFVRSyU= +github.com/go-playground/validator/v10 v10.18.0 h1:BvolUXjp4zuvkZ5YN5t7ebzbhlUtPsPm2S9NAZ5nl9U= +github.com/go-playground/validator/v10 v10.18.0/go.mod h1:dbuPbCMFw/DrkbEynArYaCwl3amGuJotoKCe95atGMM= github.com/golang/mock v1.6.0 h1:ErTB+efbowRARo13NNdxyJji2egdxLGQhRaY+DUumQc= github.com/golang/mock v1.6.0/go.mod h1:p6yTPP+5HYm5mzsMV8JkE6ZKdX+/wYM6Hr+LicevLPs= -github.com/leodido/go-urn v1.2.4 h1:XlAE/cm/ms7TE/VMVoduSpNBoyc2dOxHs5MZSwAN63Q= -github.com/leodido/go-urn v1.2.4/go.mod h1:7ZrI8mTSeBSHl/UaRyKQW1qZeMgak41ANeCNaVckg+4= +github.com/leodido/go-urn v1.4.0 h1:WT9HwE9SGECu3lg4d/dIA+jxlljEa1/ffXKmRjqdmIQ= +github.com/leodido/go-urn v1.4.0/go.mod h1:bvxc+MVxLKB4z00jd1z+Dvzr47oO32F/QSNjSBOlFxI= github.com/lmittmann/tint v1.0.4 h1:LeYihpJ9hyGvE0w+K2okPTGUdVLfng1+nDNVR4vWISc= github.com/lmittmann/tint v1.0.4/go.mod h1:HIS3gSy7qNwGCj+5oRjAutErFBl4BzdQP6cJZ0NfMwE= -github.com/pmezard/go-difflib v1.0.0/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2 h1:Jamvg5psRIccs7FGNTlIRMkT8wgtp5eCXdBlqhYGL6U= github.com/pmezard/go-difflib v1.0.1-0.20181226105442-5d4384ee4fb2/go.mod h1:iKH77koFhYxTK1pcRnkKkqfTogsbg7gZNVY4sRDYZ/4= github.com/spf13/afero v1.11.0 h1:WJQKhtpdm3v2IzqG8VMqrr6Rf3UYpEF239Jy9wNepM8= github.com/spf13/afero v1.11.0/go.mod h1:GH9Y3pIexgf1MTIWtNGyogA5MwRIDXGUr+hbWNoBjkY= -github.com/stretchr/objx v0.1.0/go.mod h1:HFkY916IF+rwdDfMAkV7OtwuqBVzrE8GR6GFx+wExME= -github.com/stretchr/objx v0.4.0/go.mod h1:YvHI0jy2hoMjB+UWwv71VJQ9isScKT/TqJzVSSt89Yw= -github.com/stretchr/objx v0.5.0/go.mod h1:Yh+to48EsGEfYuaHDzXPcE3xhTkx73EhmCGUpEOglKo= -github.com/stretchr/testify v1.7.1/go.mod h1:6Fq8oRcR53rry900zMqJjRRixrwX3KX962/h/Wwjteg= -github.com/stretchr/testify v1.8.0/go.mod h1:yNjHg4UonilssWZ8iaSj1OCr/vHnekPRkoO+kdMU+MU= -github.com/stretchr/testify v1.8.2/go.mod h1:w2LPCIKwWwSfY2zedu0+kehJoqGctiVI29o6fzry7u4= github.com/stretchr/testify v1.8.4 h1:CcVxjf3Q8PM0mHUKJCdn+eZZtm5yQwehR5yeSVQQcUk= github.com/stretchr/testify v1.8.4/go.mod h1:sz/lmYIOXD/1dqDmKjjqLyZ2RngseejIcXlSw2iwfAo= -golang.org/x/crypto v0.17.0 h1:r8bRNjWL3GshPW3gkd+RpvzWrZAwPS49OmTGZ/uhM4k= -golang.org/x/crypto v0.17.0/go.mod h1:gCAAfMLgwOJRpTjQ2zCCt2OcSfYMTeZVSRtQlPC7Nq4= -golang.org/x/net v0.19.0 h1:zTwKpTd2XuCqf8huc7Fo2iSy+4RHPd10s4KzeTnVr1c= -golang.org/x/net v0.19.0/go.mod h1:CfAk/cbD4CthTvqiEl8NpboMuiuOYsAr/7NOjZJtv1U= +golang.org/x/crypto v0.19.0 h1:ENy+Az/9Y1vSrlrvBSyna3PITt4tiZLf7sgCjZBX7Wo= +golang.org/x/crypto v0.19.0/go.mod h1:Iy9bg/ha4yyC70EfRS8jz+B6ybOBKMaSxLj6P6oBDfU= +golang.org/x/net v0.21.0 h1:AQyQV4dYCvJ7vGmJyKki9+PBdyvhkSd8EIx/qb0AYv4= +golang.org/x/net v0.21.0/go.mod h1:bIjVDfnllIU7BJ2DNgfnXvpSvtn8VRwhlsaeUTyUS44= golang.org/x/sys v0.1.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg= golang.org/x/sys v0.17.0 h1:25cE3gD+tdBA7lp7QfhuV+rJiE9YXTcS3VG1SqssI/Y= golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA= golang.org/x/text v0.14.0 h1:ScX5w1eTa3QqT8oi6+ziP7dTV1S2+ALU0bI+0zXKWiQ= golang.org/x/text v0.14.0/go.mod h1:18ZOQIKpY8NJVqYksKHtTdi31H5itFRjB5/qKTNYzSU= -gopkg.in/check.v1 v0.0.0-20161208181325-20d25e280405/go.mod h1:Co6ibVJAznAaIkqp8huTwlJQCZ016jof/cbN4VW5Yz0= -gopkg.in/yaml.v3 v3.0.0-20200313102051-9f266ea9e77c/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= gopkg.in/yaml.v3 v3.0.1 h1:fxVm/GzAzEWqLHuvctI91KS9hhNmmWOoWu0XTYJS7CA= gopkg.in/yaml.v3 v3.0.1/go.mod h1:K4uyk7z7BCEPqu6E+C64Yfv1cQ7kz7rIZviUmN+EgEM= diff --git a/internal/repositories/repositories.go b/internal/repositories/repositories.go index a751a1d..b59298a 100644 --- a/internal/repositories/repositories.go +++ b/internal/repositories/repositories.go @@ -95,7 +95,7 @@ func (r *Repositories) Overlay() error { if c.DstDir != "" { // delete dstDir since `git worktree add` will not replace existing directories if info, err := r.appFs.Stat(c.DstDir); err == nil && info.Mode().IsDir() { - if err := os.RemoveAll(c.DstDir); err != nil { + if err := r.appFs.RemoveAll(c.DstDir); err != nil { return err } } diff --git a/internal/repositories/repositories_public_test.go b/internal/repositories/repositories_public_test.go index 1eb3ae1..d354c00 100644 --- a/internal/repositories/repositories_public_test.go +++ b/internal/repositories/repositories_public_test.go @@ -101,16 +101,40 @@ func (suite *RepositoriesPublicTestSuite) TestOverlayOkWhenDstDir() { expected := filepath.Join(suite.giltDir, "cache") suite.mockRepo.EXPECT(). - Clone(suite.repoConfigDstDir[0], filepath.Join(suite.giltDir, "cache")). + Clone(suite.repoConfigDstDir[0], expected). Return(expected, nil) suite.mockRepo.EXPECT(). - Worktree(suite.repoConfigDstDir[0], filepath.Join(suite.giltDir, "cache"), suite.dstDir). + Worktree(suite.repoConfigDstDir[0], expected, suite.dstDir). Return(nil) err := repos.Overlay() assert.NoError(suite.T(), err) } +func (suite *RepositoriesPublicTestSuite) TestOverlayErrorWhenDstDir() { + repos := suite.NewTestRepositoriesManager(suite.repoConfigDstDir) + expected := filepath.Join(suite.giltDir, "cache") + errors := errors.New("tests error") + + suite.mockRepo.EXPECT(). + Clone(suite.repoConfigDstDir[0], expected). + Return(expected, nil) + suite.mockRepo.EXPECT(). + Worktree(suite.repoConfigDstDir[0], expected, suite.dstDir). + Return(errors) + + err := repos.Overlay() + assert.Error(suite.T(), err) +} + +func (suite *RepositoriesPublicTestSuite) TestOverlayCacheDirCreateError() { + // Replace the test FS with a read-only copy + suite.appFs = afero.NewReadOnlyFs(suite.appFs) + repos := suite.NewTestRepositoriesManager(suite.repoConfigDstDir) + err := repos.Overlay() + assert.Error(suite.T(), err) +} + func (suite *RepositoriesPublicTestSuite) TestOverlayDstDirExists() { repos := suite.NewTestRepositoriesManager(suite.repoConfigDstDir) @@ -122,6 +146,17 @@ func (suite *RepositoriesPublicTestSuite) TestOverlayDstDirExists() { assert.NoError(suite.T(), err) } +func (suite *RepositoriesPublicTestSuite) TestOverlayErrorRemovingDstDir() { + suite.mockRepo.EXPECT().Clone(gomock.Any(), gomock.Any()).Return("", nil) + _ = suite.appFs.MkdirAll(filepath.Join(suite.giltDir, "cache"), 0o755) + _ = suite.appFs.MkdirAll(suite.dstDir, 0o755) + // Replace the test FS with a read-only copy + suite.appFs = afero.NewReadOnlyFs(suite.appFs) + repos := suite.NewTestRepositoriesManager(suite.repoConfigDstDir) + err := repos.Overlay() + assert.Error(suite.T(), err) +} + func (suite *RepositoriesPublicTestSuite) TestOverlayReturnsErrorWhenDstDirDeleteFails() { suite.T().Skip("implement") } @@ -186,6 +221,30 @@ func (suite *RepositoriesPublicTestSuite) TestOverlayReturnsErrorWhenCopySources assert.Error(suite.T(), err) } +func (suite *RepositoriesPublicTestSuite) TestOverlayErrorCreatingCopySourcesWorktree() { + repoConfig := []config.Repository{ + { + Git: suite.gitURL, + Version: suite.gitVersion, + Sources: []config.Source{ + { + Src: "srcDir", + DstDir: suite.dstDir, + }, + }, + }, + } + repos := suite.NewTestRepositoriesManager(repoConfig) + errors := errors.New("tests error") + + suite.mockRepo.EXPECT().Clone(gomock.Any(), gomock.Any()).Return("", nil) + suite.mockExec.EXPECT().RunInTempDir(gomock.Any(), gomock.Any(), gomock.Any()).Return(nil) + suite.mockRepo.EXPECT().Worktree(repoConfig[0], gomock.Any(), gomock.Any()).Return(errors) + + err := repos.Overlay() + assert.Error(suite.T(), err) +} + func (suite *RepositoriesPublicTestSuite) TestOverlayOkWhenCommands() { repoConfig := []config.Repository{ { diff --git a/internal/repositories/repositories_test.go b/internal/repositories/repositories_test.go index bb7af73..acead66 100644 --- a/internal/repositories/repositories_test.go +++ b/internal/repositories/repositories_test.go @@ -91,6 +91,16 @@ func (suite *RepositoriesTestSuite) TestgetCacheDir() { assert.True(suite.T(), exists) } +func (suite *RepositoriesTestSuite) TestgetCacheDirCannotCreateError() { + // Replace the test FS with a read-only copy + suite.appFs = afero.NewReadOnlyFs(suite.appFs) + repos := suite.NewTestRepositories(suite.giltDir) + + got, err := repos.getCacheDir() + assert.Error(suite.T(), err) + assert.Equal(suite.T(), "", got) +} + // In order for `go test` to run this suite, we need to create // a normal test function and pass our suite to suite.Run. func TestRepositoriesTestSuite(t *testing.T) { diff --git a/internal/repository/copy.go b/internal/repository/copy.go index 09a49de..e6e914f 100644 --- a/internal/repository/copy.go +++ b/internal/repository/copy.go @@ -68,11 +68,7 @@ func (r *Copy) CopyFile( if err != nil { return err } - defer func() { - if e := out.Close(); e != nil { - err = e - } - }() + defer func() { _ = out.Close() }() _, err = io.Copy(out, in) if err != nil { diff --git a/internal/repository/copy_public_test.go b/internal/repository/copy_public_test.go index a72861f..dd46b5a 100644 --- a/internal/repository/copy_public_test.go +++ b/internal/repository/copy_public_test.go @@ -85,6 +85,24 @@ func (suite *CopyPublicTestSuite) TestCopyFileReturnsError() { assert.False(suite.T(), got) } +func (suite *CopyPublicTestSuite) TestCopyFileReturnsErrorEPERM() { + specs := []FileSpec{ + { + appFs: suite.appFs, + srcDir: filepath.Join(suite.cloneDir, "srcDir"), + srcFile: filepath.Join(suite.cloneDir, "srcDir", "1.txt"), + }, + } + createFileSpecs(specs) + + assertFile := filepath.Join(suite.dstDir, "1.txt") + // Replace the test FS with a read-only copy + suite.appFs = afero.NewReadOnlyFs(suite.appFs) + cm := suite.NewTestCopyManager() + err := cm.CopyFile(specs[0].srcFile, assertFile) + assert.Error(suite.T(), err) +} + func (suite *CopyPublicTestSuite) TestCopyDirOk() { cm := suite.NewTestCopyManager()