From 091fa09a23facf0a6fd20e70c149458e305ecfbb Mon Sep 17 00:00:00 2001 From: Ian Howell Date: Mon, 17 Feb 2020 14:29:26 -0600 Subject: [PATCH] [#50] Clean up temp files from unit tests This commit adds the utility testing function TempDir, which provides a tester with a temporary directory as well as a means of cleaning up that directory. The new function is implemented everywhere that makes sense throughout the code base. This also cleans up the directories left behind by go-git's testing fixtures. Some light refactoring was also performed in this change. Change-Id: I754484934660487140f57671bacb5463cf669e3e --- cmd/config/set_authinfo_test.go | 56 +++++++------ cmd/config/set_cluster_test.go | 114 +++++++++++++++------------ cmd/config/set_context_test.go | 77 ++++++++++-------- cmd/document/pull_test.go | 12 +-- pkg/bootstrap/isogen/command_test.go | 14 +++- pkg/config/config_test.go | 110 +++++++++++++++----------- pkg/config/test_utils.go | 18 +++-- pkg/document/pull/pull_test.go | 13 ++- pkg/document/repo/repo_test.go | 12 +++ pkg/environment/settings_test.go | 30 +++---- testutil/testdatafs.go | 25 ++++++ tools/gomod_check | 1 + 12 files changed, 292 insertions(+), 190 deletions(-) diff --git a/cmd/config/set_authinfo_test.go b/cmd/config/set_authinfo_test.go index 6c1a915fc..2b5a87883 100644 --- a/cmd/config/set_authinfo_test.go +++ b/cmd/config/set_authinfo_test.go @@ -41,10 +41,10 @@ const ( type setAuthInfoTest struct { description string - config *config.Config + givenConfig *config.Config args []string flags []string - expected string + expectedOutput string expectedConfig *config.Config } @@ -74,58 +74,68 @@ func TestConfigSetAuthInfo(t *testing.T) { } } -func initConfig(t *testing.T, withUser bool, testname string) (*config.Config, *config.Config) { - conf := config.InitConfig(t) +// initConfig creates an input config and an associated expected config +// Each of these config objects are associated with real files. Those files can be +// cleaned up by calling cleanup +func initConfig(t *testing.T, withUser bool, testname string) ( + given, expected *config.Config, cleanup func(*testing.T)) { + given, givenCleanup := config.InitConfig(t) if withUser { kAuthInfo := kubeconfig.NewAuthInfo() kAuthInfo.Username = testUsername kAuthInfo.Password = testPassword - conf.KubeConfig().AuthInfos[testname] = kAuthInfo - conf.AuthInfos[testname].SetKubeAuthInfo(kAuthInfo) + given.KubeConfig().AuthInfos[testname] = kAuthInfo + given.AuthInfos[testname].SetKubeAuthInfo(kAuthInfo) } - expconf := config.InitConfig(t) - expconf.AuthInfos[testname] = config.NewAuthInfo() + expected, expectedCleanup := config.InitConfig(t) + expected.AuthInfos[testname] = config.NewAuthInfo() expkAuthInfo := kubeconfig.NewAuthInfo() expkAuthInfo.Username = testUsername expkAuthInfo.Password = testPassword - expconf.KubeConfig().AuthInfos[testname] = expkAuthInfo - expconf.AuthInfos[testname].SetKubeAuthInfo(expkAuthInfo) + expected.KubeConfig().AuthInfos[testname] = expkAuthInfo + expected.AuthInfos[testname].SetKubeAuthInfo(expkAuthInfo) - return conf, expconf + return given, expected, func(tt *testing.T) { + givenCleanup(tt) + expectedCleanup(tt) + } } func TestSetAuthInfo(t *testing.T) { - conf, expconf := initConfig(t, false, testNewname) + given, expected, cleanup := initConfig(t, false, testNewname) + defer cleanup(t) test := setAuthInfoTest{ description: "Testing 'airshipctl config set-credential' with a new user", - config: conf, + givenConfig: given, args: []string{testNewname}, flags: []string{ "--" + config.FlagUsername + "=" + testUsername, "--" + config.FlagPassword + "=" + testPassword, }, - expected: `User information "` + testNewname + `" created.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("User information %q created.\n", testNewname), + expectedConfig: expected, } test.run(t) } func TestModifyAuthInfo(t *testing.T) { - conf, expconf := initConfig(t, true, testOldname) - expconf.AuthInfos[testOldname].KubeAuthInfo().Password = testPassword + pwdDelta + given, expected, cleanup := initConfig(t, true, testOldname) + defer cleanup(t) + + expected.AuthInfos[testOldname].KubeAuthInfo().Password = testPassword + pwdDelta test := setAuthInfoTest{ description: "Testing 'airshipctl config set-credential' with an existing user", - config: conf, + givenConfig: given, args: []string{testOldname}, flags: []string{ "--" + config.FlagPassword + "=" + testPassword + pwdDelta, }, - expected: `User information "` + testOldname + `" modified.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("User information %q modified.\n", testOldname), + expectedConfig: expected, } test.run(t) } @@ -133,7 +143,7 @@ func TestModifyAuthInfo(t *testing.T) { func (test setAuthInfoTest) run(t *testing.T) { // Get the Environment settings := &environment.AirshipCTLSettings{} - settings.SetConfig(test.config) + settings.SetConfig(test.givenConfig) buf := bytes.NewBuffer([]byte{}) @@ -165,7 +175,7 @@ func (test setAuthInfoTest) run(t *testing.T) { assert.EqualValues(t, testKauthinfo.Password, afterKauthinfo.Password) // Test that the Return Message looks correct - if len(test.expected) != 0 { - assert.EqualValues(t, test.expected, buf.String()) + if len(test.expectedOutput) != 0 { + assert.EqualValues(t, test.expectedOutput, buf.String()) } } diff --git a/cmd/config/set_cluster_test.go b/cmd/config/set_cluster_test.go index dc74df3c1..964edb8f3 100644 --- a/cmd/config/set_cluster_test.go +++ b/cmd/config/set_cluster_test.go @@ -18,6 +18,7 @@ package config import ( "bytes" + "fmt" "io/ioutil" "testing" @@ -32,10 +33,10 @@ import ( type setClusterTest struct { description string - config *config.Config + givenConfig *config.Config args []string flags []string - expected string + expectedOutput string expectedConfig *config.Config } @@ -44,27 +45,31 @@ const ( ) func TestSetClusterWithCAFile(t *testing.T) { - conf := config.InitConfig(t) + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) + certFile := "../../pkg/config/testdata/ca.crt" tname := testCluster tctype := config.Ephemeral - expconf := config.InitConfig(t) - expconf.Clusters[tname] = config.NewClusterPurpose() - expconf.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.Clusters[tname] = config.NewClusterPurpose() + expected.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() clusterName := config.NewClusterComplexName() clusterName.WithType(tname, tctype) - expconf.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() + expected.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() expkCluster := kubeconfig.NewCluster() expkCluster.CertificateAuthority = certFile expkCluster.InsecureSkipTLSVerify = false - expconf.KubeConfig().Clusters[clusterName.Name()] = expkCluster + expected.KubeConfig().Clusters[clusterName.Name()] = expkCluster test := setClusterTest{ description: "Testing 'airshipctl config set-cluster' with a new cluster", - config: conf, + givenConfig: given, args: []string{tname}, flags: []string{ "--" + config.FlagClusterType + "=" + config.Ephemeral, @@ -72,24 +77,28 @@ func TestSetClusterWithCAFile(t *testing.T) { "--" + config.FlagCAFile + "=" + certFile, "--" + config.FlagInsecure + "=false", }, - expected: `Cluster "` + tname + `" of type "` + config.Ephemeral + `" created.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Cluster %q of type %q created.\n", testCluster, config.Ephemeral), + expectedConfig: expected, } test.run(t) } func TestSetClusterWithCAFileData(t *testing.T) { - conf := config.InitConfig(t) + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) + certFile := "../../pkg/config/testdata/ca.crt" tname := testCluster tctype := config.Ephemeral - expconf := config.InitConfig(t) - expconf.Clusters[tname] = config.NewClusterPurpose() - expconf.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.Clusters[tname] = config.NewClusterPurpose() + expected.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() clusterName := config.NewClusterComplexName() clusterName.WithType(tname, tctype) - expconf.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() + expected.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() expkCluster := kubeconfig.NewCluster() readData, err := ioutil.ReadFile(certFile) @@ -97,11 +106,11 @@ func TestSetClusterWithCAFileData(t *testing.T) { expkCluster.CertificateAuthorityData = readData expkCluster.InsecureSkipTLSVerify = false - expconf.KubeConfig().Clusters[clusterName.Name()] = expkCluster + expected.KubeConfig().Clusters[clusterName.Name()] = expkCluster test := setClusterTest{ description: "Testing 'airshipctl config set-cluster' with a new cluster", - config: conf, + givenConfig: given, args: []string{tname}, flags: []string{ "--" + config.FlagClusterType + "=" + config.Ephemeral, @@ -109,41 +118,44 @@ func TestSetClusterWithCAFileData(t *testing.T) { "--" + config.FlagCAFile + "=" + certFile, "--" + config.FlagInsecure + "=false", }, - expected: `Cluster "` + tname + `" of type "` + config.Ephemeral + `" created.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Cluster %q of type %q created.\n", tname, config.Ephemeral), + expectedConfig: expected, } test.run(t) } func TestSetCluster(t *testing.T) { - conf := config.InitConfig(t) + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) tname := testCluster tctype := config.Ephemeral - expconf := config.InitConfig(t) - expconf.Clusters[tname] = config.NewClusterPurpose() - expconf.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.Clusters[tname] = config.NewClusterPurpose() + expected.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() clusterName := config.NewClusterComplexName() clusterName.WithType(tname, tctype) - expconf.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() + expected.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() expkCluster := kubeconfig.NewCluster() expkCluster.Server = "https://192.168.0.11" expkCluster.InsecureSkipTLSVerify = false - expconf.KubeConfig().Clusters[clusterName.Name()] = expkCluster + expected.KubeConfig().Clusters[clusterName.Name()] = expkCluster test := setClusterTest{ description: "Testing 'airshipctl config set-cluster' with a new cluster", - config: conf, + givenConfig: given, args: []string{tname}, flags: []string{ "--" + config.FlagClusterType + "=" + config.Ephemeral, "--" + config.FlagAPIServer + "=https://192.168.0.11", "--" + config.FlagInsecure + "=false", }, - expected: `Cluster "` + tname + `" of type "` + config.Ephemeral + `" created.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Cluster %q of type %q created.\n", tname, config.Ephemeral), + expectedConfig: expected, } test.run(t) } @@ -152,36 +164,40 @@ func TestModifyCluster(t *testing.T) { tname := testClusterName tctype := config.Ephemeral - conf := config.InitConfig(t) - conf.Clusters[tname] = config.NewClusterPurpose() + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) + + given.Clusters[tname] = config.NewClusterPurpose() clusterName := config.NewClusterComplexName() clusterName.WithType(tname, tctype) - conf.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() - conf.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() + given.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() + given.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() kCluster := kubeconfig.NewCluster() kCluster.Server = "https://192.168.0.10" - conf.KubeConfig().Clusters[clusterName.Name()] = kCluster - conf.Clusters[tname].ClusterTypes[tctype].SetKubeCluster(kCluster) + given.KubeConfig().Clusters[clusterName.Name()] = kCluster + given.Clusters[tname].ClusterTypes[tctype].SetKubeCluster(kCluster) - expconf := config.InitConfig(t) - expconf.Clusters[tname] = config.NewClusterPurpose() - expconf.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() - expconf.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.Clusters[tname] = config.NewClusterPurpose() + expected.Clusters[tname].ClusterTypes[tctype] = config.NewCluster() + expected.Clusters[tname].ClusterTypes[tctype].NameInKubeconf = clusterName.Name() expkCluster := kubeconfig.NewCluster() expkCluster.Server = "https://192.168.0.10" - expconf.KubeConfig().Clusters[clusterName.Name()] = expkCluster - expconf.Clusters[tname].ClusterTypes[tctype].SetKubeCluster(expkCluster) + expected.KubeConfig().Clusters[clusterName.Name()] = expkCluster + expected.Clusters[tname].ClusterTypes[tctype].SetKubeCluster(expkCluster) test := setClusterTest{ description: "Testing 'airshipctl config set-cluster' with an existing cluster", - config: conf, + givenConfig: given, args: []string{tname}, flags: []string{ "--" + config.FlagClusterType + "=" + config.Ephemeral, "--" + config.FlagAPIServer + "=https://192.168.0.99", }, - expected: `Cluster "` + tname + `" of type "` + tctype + `" modified.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Cluster %q of type %q modified.\n", tname, tctype), + expectedConfig: expected, } test.run(t) } @@ -189,7 +205,7 @@ func TestModifyCluster(t *testing.T) { func (test setClusterTest) run(t *testing.T) { // Get the Environment settings := &environment.AirshipCTLSettings{} - settings.SetConfig(test.config) + settings.SetConfig(test.givenConfig) buf := bytes.NewBuffer([]byte{}) @@ -219,13 +235,13 @@ func (test setClusterTest) run(t *testing.T) { afterKcluster := afterRunCluster.KubeCluster() require.NotNil(t, afterKcluster) - testKcluster := test.config.KubeConfig(). - Clusters[test.config.Clusters[test.args[0]].ClusterTypes[tctype].NameInKubeconf] + testKcluster := test.givenConfig.KubeConfig(). + Clusters[test.givenConfig.Clusters[test.args[0]].ClusterTypes[tctype].NameInKubeconf] assert.EqualValues(t, afterKcluster.Server, testKcluster.Server) // Test that the Return Message looks correct - if len(test.expected) != 0 { - assert.EqualValues(t, test.expected, buf.String()) + if len(test.expectedOutput) != 0 { + assert.EqualValues(t, test.expectedOutput, buf.String()) } } diff --git a/cmd/config/set_context_test.go b/cmd/config/set_context_test.go index 687f2026e..b6c9d88a6 100644 --- a/cmd/config/set_context_test.go +++ b/cmd/config/set_context_test.go @@ -37,10 +37,10 @@ const ( type setContextTest struct { description string - config *config.Config + givenConfig *config.Config args []string flags []string - expected string + expectedOutput string expectedConfig *config.Config } @@ -71,26 +71,29 @@ func TestConfigSetContext(t *testing.T) { } func TestSetContext(t *testing.T) { - conf := config.InitConfig(t) + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) tname := "dummycontext" tctype := config.Ephemeral - expconf := config.InitConfig(t) - expconf.Contexts[tname] = config.NewContext() + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.Contexts[tname] = config.NewContext() clusterName := config.NewClusterComplexName() clusterName.WithType(tname, tctype) - expconf.Contexts[tname].NameInKubeconf = clusterName.Name() - expconf.Contexts[tname].Manifest = "edge_cloud" + expected.Contexts[tname].NameInKubeconf = clusterName.Name() + expected.Contexts[tname].Manifest = "edge_cloud" expkContext := kubeconfig.NewContext() expkContext.AuthInfo = testUser expkContext.Namespace = "kube-system" - expconf.KubeConfig().Contexts[expconf.Contexts[tname].NameInKubeconf] = expkContext + expected.KubeConfig().Contexts[expected.Contexts[tname].NameInKubeconf] = expkContext test := setContextTest{ description: "Testing 'airshipctl config set-context' with a new context", - config: conf, + givenConfig: given, args: []string{tname}, flags: []string{ "--" + config.FlagClusterType + "=" + config.Target, @@ -98,60 +101,68 @@ func TestSetContext(t *testing.T) { "--" + config.FlagManifest + "=edge_cloud", "--" + config.FlagNamespace + "=kube-system", }, - expected: `Context "` + tname + `" created.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Context %q created.\n", tname), + expectedConfig: expected, } test.run(t) } func TestSetCurrentContext(t *testing.T) { tname := "def_target" - conf := config.InitConfig(t) + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) - expconf := config.InitConfig(t) - expconf.CurrentContext = "def_target" + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.CurrentContext = "def_target" test := setContextTest{ description: "Testing 'airshipctl config set-context' with a new current context", - config: conf, + givenConfig: given, args: []string{tname}, - expected: `Context "` + tname + `" modified.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Context %q modified.\n", tname), + expectedConfig: expected, } test.run(t) } + func TestModifyContext(t *testing.T) { tname := testCluster tctype := config.Ephemeral - conf := config.InitConfig(t) - conf.Contexts[tname] = config.NewContext() + given, cleanupGiven := config.InitConfig(t) + defer cleanupGiven(t) + + given.Contexts[testCluster] = config.NewContext() clusterName := config.NewClusterComplexName() clusterName.WithType(tname, tctype) - conf.Contexts[tname].NameInKubeconf = clusterName.Name() + given.Contexts[tname].NameInKubeconf = clusterName.Name() kContext := kubeconfig.NewContext() kContext.AuthInfo = testUser - conf.KubeConfig().Contexts[clusterName.Name()] = kContext - conf.Contexts[tname].SetKubeContext(kContext) + given.KubeConfig().Contexts[clusterName.Name()] = kContext + given.Contexts[tname].SetKubeContext(kContext) - expconf := config.InitConfig(t) - expconf.Contexts[tname] = config.NewContext() - expconf.Contexts[tname].NameInKubeconf = clusterName.Name() + expected, cleanupExpected := config.InitConfig(t) + defer cleanupExpected(t) + + expected.Contexts[tname] = config.NewContext() + expected.Contexts[tname].NameInKubeconf = clusterName.Name() expkContext := kubeconfig.NewContext() expkContext.AuthInfo = testUser - expconf.KubeConfig().Contexts[clusterName.Name()] = expkContext - expconf.Contexts[tname].SetKubeContext(expkContext) + expected.KubeConfig().Contexts[clusterName.Name()] = expkContext + expected.Contexts[tname].SetKubeContext(expkContext) test := setContextTest{ description: "Testing 'airshipctl config set-context' with an existing context", - config: conf, + givenConfig: given, args: []string{tname}, flags: []string{ "--" + config.FlagAuthInfoName + "=" + testUser, }, - expected: `Context "` + tname + `" modified.` + "\n", - expectedConfig: expconf, + expectedOutput: fmt.Sprintf("Context %q modified.\n", tname), + expectedConfig: expected, } test.run(t) } @@ -159,7 +170,7 @@ func TestModifyContext(t *testing.T) { func (test setContextTest) run(t *testing.T) { // Get the Environment settings := &environment.AirshipCTLSettings{} - settings.SetConfig(test.config) + settings.SetConfig(test.givenConfig) buf := bytes.NewBuffer([]byte{}) @@ -190,7 +201,7 @@ func (test setContextTest) run(t *testing.T) { assert.EqualValues(t, afterKcontext.AuthInfo, testKcontext.AuthInfo) // Test that the Return Message looks correct - if len(test.expected) != 0 { - assert.EqualValuesf(t, buf.String(), test.expected, "expected %v, but got %v", test.expected, buf.String()) + if len(test.expectedOutput) != 0 { + assert.EqualValues(t, test.expectedOutput, buf.String()) } } diff --git a/cmd/document/pull_test.go b/cmd/document/pull_test.go index e92448c42..c16d5cb2b 100644 --- a/cmd/document/pull_test.go +++ b/cmd/document/pull_test.go @@ -3,6 +3,7 @@ package document import ( "testing" + "github.com/stretchr/testify/require" fixtures "gopkg.in/src-d/go-git-fixtures.v3" "opendev.org/airship/airshipctl/pkg/config" @@ -11,15 +12,14 @@ import ( "opendev.org/airship/airshipctl/testutil" ) -func getDummyAirshipSettings() *environment.AirshipCTLSettings { +func getDummyAirshipSettings(t *testing.T) *environment.AirshipCTLSettings { settings := new(environment.AirshipCTLSettings) conf := config.DummyConfig() mfst := conf.Manifests["dummy_manifest"] err := fixtures.Init() - if err != nil { - panic(err) - } + require.NoError(t, err) + fx := fixtures.Basic().One() mfst.Repository = &config.Repository{ @@ -41,7 +41,7 @@ func TestPull(t *testing.T) { { Name: "document-pull-cmd-with-defaults", CmdLine: "", - Cmd: NewDocumentPullCommand(getDummyAirshipSettings()), + Cmd: NewDocumentPullCommand(getDummyAirshipSettings(t)), }, { Name: "document-pull-cmd-with-help", @@ -53,4 +53,6 @@ func TestPull(t *testing.T) { for _, tt := range cmdTests { testutil.RunTest(t, tt) } + + testutil.CleanUpGitFixtures(t) } diff --git a/pkg/bootstrap/isogen/command_test.go b/pkg/bootstrap/isogen/command_test.go index 36de7bdd6..be6ca95a0 100644 --- a/pkg/bootstrap/isogen/command_test.go +++ b/pkg/bootstrap/isogen/command_test.go @@ -49,7 +49,10 @@ func TestBootstrapIso(t *testing.T) { bundle, err := document.NewBundle(fSys, "/", "/") require.NoError(t, err, "Building Bundle Failed") - volBind := "/tmp:/dst" + tempVol, cleanup := testutil.TempDir(t, "bootstrap-test") + defer cleanup(t) + + volBind := tempVol + ":/dst" testErr := fmt.Errorf("TestErr") testCfg := &config.Bootstrap{ Container: &config.Container{ @@ -123,6 +126,9 @@ func TestBootstrapIso(t *testing.T) { } func TestVerifyInputs(t *testing.T) { + tempVol, cleanup := testutil.TempDir(t, "bootstrap-test") + defer cleanup(t) + tests := []struct { cfg *config.Bootstrap args []string @@ -137,7 +143,7 @@ func TestVerifyInputs(t *testing.T) { { cfg: &config.Bootstrap{ Container: &config.Container{ - Volume: "/tmp:/dst", + Volume: tempVol + ":/dst", }, Builder: &config.Builder{}, }, @@ -146,7 +152,7 @@ func TestVerifyInputs(t *testing.T) { { cfg: &config.Bootstrap{ Container: &config.Container{ - Volume: "/tmp", + Volume: tempVol, }, Builder: &config.Builder{ UserDataFileName: "user-data", @@ -158,7 +164,7 @@ func TestVerifyInputs(t *testing.T) { { cfg: &config.Bootstrap{ Container: &config.Container{ - Volume: "/tmp:/dst:/dst1", + Volume: tempVol + ":/dst:/dst1", }, Builder: &config.Builder{ UserDataFileName: "user-data", diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go index b9237457f..1793a3f6c 100644 --- a/pkg/config/config_test.go +++ b/pkg/config/config_test.go @@ -18,9 +18,7 @@ package config_test import ( "fmt" - "io/ioutil" "os" - "path/filepath" "testing" "github.com/stretchr/testify/assert" @@ -232,8 +230,8 @@ func TestEqual(t *testing.T) { } func TestLoadConfig(t *testing.T) { - conf := config.InitConfig(t) - require.NotEmpty(t, conf.String()) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) assert.Len(t, conf.Clusters, 5) require.Contains(t, conf.Clusters, "def") @@ -243,22 +241,23 @@ func TestLoadConfig(t *testing.T) { } func TestPersistConfig(t *testing.T) { - cfg := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) - err := cfg.PersistConfig() + err := conf.PersistConfig() require.NoError(t, err) // Check that the files were created - assert.FileExists(t, cfg.LoadedConfigPath()) - assert.FileExists(t, cfg.KubeConfigPath()) + assert.FileExists(t, conf.LoadedConfigPath()) + assert.FileExists(t, conf.KubeConfigPath()) // Check that the invalid name was changed to a valid one - assert.Contains(t, cfg.KubeConfig().Clusters, "invalidName_target") + assert.Contains(t, conf.KubeConfig().Clusters, "invalidName_target") // Check that the missing cluster was added to the airshipconfig - assert.Contains(t, cfg.Clusters, "onlyinkubeconf") + assert.Contains(t, conf.Clusters, "onlyinkubeconf") // Check that the "stragglers" were removed from the airshipconfig - assert.NotContains(t, cfg.Clusters, "straggler") + assert.NotContains(t, conf.Clusters, "straggler") } func TestEnsureComplete(t *testing.T) { @@ -371,38 +370,31 @@ func TestEnsureComplete(t *testing.T) { } func TestPurge(t *testing.T) { - cfg := config.InitConfig(t) - - // Point the config objects at a temporary directory - tempDir, err := ioutil.TempDir("", "airship-test-purge") - require.NoError(t, err) - - airConfigFile := filepath.Join(tempDir, config.AirshipConfig) - cfg.SetLoadedConfigPath(airConfigFile) - - kConfigFile := filepath.Join(tempDir, config.AirshipKubeConfig) - cfg.SetKubeConfigPath(kConfigFile) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) // Store it - err = cfg.PersistConfig() - assert.NoErrorf(t, err, "Unable to persist configuration expected at %v", cfg.LoadedConfigPath()) + err := conf.PersistConfig() + assert.NoErrorf(t, err, "Unable to persist configuration expected at %v", conf.LoadedConfigPath()) // Verify that the file is there - _, err = os.Stat(cfg.LoadedConfigPath()) + _, err = os.Stat(conf.LoadedConfigPath()) assert.Falsef(t, os.IsNotExist(err), "Test config was not persisted at %v, cannot validate Purge", - cfg.LoadedConfigPath()) + conf.LoadedConfigPath()) // Delete it - err = cfg.Purge() - assert.NoErrorf(t, err, "Unable to Purge file at %v", cfg.LoadedConfigPath()) + err = conf.Purge() + assert.NoErrorf(t, err, "Unable to Purge file at %v", conf.LoadedConfigPath()) // Verify its gone - _, err = os.Stat(cfg.LoadedConfigPath()) - assert.Falsef(t, os.IsExist(err), "Purge failed to remove file at %v", cfg.LoadedConfigPath()) + _, err = os.Stat(conf.LoadedConfigPath()) + assert.Falsef(t, os.IsExist(err), "Purge failed to remove file at %v", conf.LoadedConfigPath()) } func TestKClusterString(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + kClusters := conf.KubeConfig().Clusters for kClust := range kClusters { assert.NotEmpty(t, config.KClusterString(kClusters[kClust])) @@ -410,7 +402,9 @@ func TestKClusterString(t *testing.T) { assert.EqualValues(t, config.KClusterString(nil), "null\n") } func TestKContextString(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + kContexts := conf.KubeConfig().Contexts for kCtx := range kContexts { assert.NotEmpty(t, config.KContextString(kContexts[kCtx])) @@ -418,7 +412,9 @@ func TestKContextString(t *testing.T) { assert.EqualValues(t, config.KClusterString(nil), "null\n") } func TestKAuthInfoString(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + kAuthInfos := conf.KubeConfig().AuthInfos for kAi := range kAuthInfos { assert.NotEmpty(t, config.KAuthInfoString(kAuthInfos[kAi])) @@ -449,7 +445,9 @@ func TestValidClusterTypeFail(t *testing.T) { assert.Error(t, err) } func TestGetCluster(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + cluster, err := conf.GetCluster("def", config.Ephemeral) require.NoError(t, err) @@ -467,8 +465,10 @@ func TestGetCluster(t *testing.T) { } func TestAddCluster(t *testing.T) { + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + co := config.DummyClusterOptions() - conf := config.InitConfig(t) cluster, err := conf.AddCluster(co) require.NoError(t, err) @@ -476,8 +476,10 @@ func TestAddCluster(t *testing.T) { } func TestModifyCluster(t *testing.T) { + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + co := config.DummyClusterOptions() - conf := config.InitConfig(t) cluster, err := conf.AddCluster(co) require.NoError(t, err) @@ -496,19 +498,25 @@ func TestModifyCluster(t *testing.T) { } func TestGetClusters(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + clusters := conf.GetClusters() assert.Len(t, clusters, 5) } func TestGetContexts(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + contexts := conf.GetContexts() assert.Len(t, contexts, 3) } func TestGetContext(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + context, err := conf.GetContext("def_ephemeral") require.NoError(t, err) @@ -522,15 +530,19 @@ func TestGetContext(t *testing.T) { } func TestAddContext(t *testing.T) { + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + co := config.DummyContextOptions() - conf := config.InitConfig(t) context := conf.AddContext(co) assert.EqualValues(t, conf.Contexts[co.Name], context) } func TestModifyContext(t *testing.T) { + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + co := config.DummyContextOptions() - conf := config.InitConfig(t) context := conf.AddContext(co) co.Namespace += stringDelta @@ -548,13 +560,17 @@ func TestModifyContext(t *testing.T) { // AuthInfo Related func TestGetAuthInfos(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + authinfos := conf.GetAuthInfos() assert.Len(t, authinfos, 3) } func TestGetAuthInfo(t *testing.T) { - conf := config.InitConfig(t) + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + authinfo, err := conf.GetAuthInfo("def-user") require.NoError(t, err) @@ -567,15 +583,19 @@ func TestGetAuthInfo(t *testing.T) { } func TestAddAuthInfo(t *testing.T) { + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + co := config.DummyAuthInfoOptions() - conf := config.InitConfig(t) authinfo := conf.AddAuthInfo(co) assert.EqualValues(t, conf.AuthInfos[co.Name], authinfo) } func TestModifyAuthInfo(t *testing.T) { + conf, cleanup := config.InitConfig(t) + defer cleanup(t) + co := config.DummyAuthInfoOptions() - conf := config.InitConfig(t) authinfo := conf.AddAuthInfo(co) co.Username += stringDelta diff --git a/pkg/config/test_utils.go b/pkg/config/test_utils.go index 60abd7c57..64095b3ac 100644 --- a/pkg/config/test_utils.go +++ b/pkg/config/test_utils.go @@ -24,6 +24,8 @@ import ( kubeconfig "k8s.io/client-go/tools/clientcmd/api" "github.com/stretchr/testify/require" + + "opendev.org/airship/airshipctl/testutil" ) // DummyConfig used by tests, to initialize min set of data @@ -145,25 +147,29 @@ func DummyClusterPurpose() *ClusterPurpose { return cp } -func InitConfig(t *testing.T) *Config { +// InitConfig creates a Config object meant for testing. +// +// The returned config object will be associated with real files stored in a +// directory in the user's temporary file storage +// This directory can be cleaned up by calling the returned "cleanup" function +func InitConfig(t *testing.T) (conf *Config, cleanup func(*testing.T)) { t.Helper() - testDir, err := ioutil.TempDir("", "airship-test") - require.NoError(t, err) + testDir, cleanup := testutil.TempDir(t, "airship-test") configPath := filepath.Join(testDir, "config") - err = ioutil.WriteFile(configPath, []byte(testConfigYAML), 0666) + err := ioutil.WriteFile(configPath, []byte(testConfigYAML), 0666) require.NoError(t, err) kubeConfigPath := filepath.Join(testDir, "kubeconfig") err = ioutil.WriteFile(kubeConfigPath, []byte(testKubeConfigYAML), 0666) require.NoError(t, err) - conf := NewConfig() + conf = NewConfig() err = conf.LoadConfig(configPath, kubeConfigPath) require.NoError(t, err) - return conf + return conf, cleanup } func DummyClusterOptions() *ClusterOptions { diff --git a/pkg/document/pull/pull_test.go b/pkg/document/pull/pull_test.go index fd8555ecd..bf3006678 100644 --- a/pkg/document/pull/pull_test.go +++ b/pkg/document/pull/pull_test.go @@ -16,6 +16,7 @@ import ( "opendev.org/airship/airshipctl/pkg/config" "opendev.org/airship/airshipctl/pkg/environment" + "opendev.org/airship/airshipctl/testutil" ) func getDummyPullSettings() *Settings { @@ -52,8 +53,9 @@ func TestPull(t *testing.T) { }, } - tmpDir, err := ioutil.TempDir("", "airshipctlPullTest-") - require.NoError(err) + tmpDir, cleanup := testutil.TempDir(t, "airshipctlPullTest-") + defer cleanup(t) + currentManifest.TargetPath = tmpDir _, err = repo2.NewRepository(".", currentManifest.Repository) @@ -92,8 +94,9 @@ func TestPull(t *testing.T) { } dummyPullSettings.SetConfig(conf) - tmpDir, err := ioutil.TempDir("", "airshipctlPullTest-") - require.NoError(err) + tmpDir, cleanup := testutil.TempDir(t, "airshipctlPullTest-") + defer cleanup(t) + mfst.TargetPath = tmpDir require.NoError(err) @@ -107,4 +110,6 @@ func TestPull(t *testing.T) { require.NoError(err) assert.Equal("ref: refs/heads/master", strings.TrimRight(string(contents), "\t \n")) }) + + testutil.CleanUpGitFixtures(t) } diff --git a/pkg/document/repo/repo_test.go b/pkg/document/repo/repo_test.go index 1a67c54ab..29f9627c4 100644 --- a/pkg/document/repo/repo_test.go +++ b/pkg/document/repo/repo_test.go @@ -11,6 +11,8 @@ import ( "gopkg.in/src-d/go-git.v4/plumbing" "gopkg.in/src-d/go-git.v4/plumbing/transport" "gopkg.in/src-d/go-git.v4/storage/memory" + + "opendev.org/airship/airshipctl/testutil" ) type mockBuilder struct { @@ -39,6 +41,8 @@ func (md mockBuilder) URL() string { return md.URLString } func TestNewRepositoryNegative(t *testing.T) { err := fixtures.Init() require.NoError(t, err) + defer testutil.CleanUpGitFixtures(t) + builder := &mockBuilder{ URLString: "", } @@ -50,6 +54,8 @@ func TestNewRepositoryNegative(t *testing.T) { func TestDownload(t *testing.T) { err := fixtures.Init() require.NoError(t, err) + defer testutil.CleanUpGitFixtures(t) + fx := fixtures.Basic().One() builder := &mockBuilder{ CheckoutOptions: &git.CheckoutOptions{ @@ -87,6 +93,8 @@ func TestDownload(t *testing.T) { func TestUpdate(t *testing.T) { err := fixtures.Init() require.NoError(t, err) + defer testutil.CleanUpGitFixtures(t) + fx := fixtures.Basic().One() checkout := &git.CheckoutOptions{ @@ -149,6 +157,8 @@ func TestUpdate(t *testing.T) { func TestOpen(t *testing.T) { err := fixtures.Init() require.NoError(t, err) + defer testutil.CleanUpGitFixtures(t) + fx := fixtures.Basic().One() url := fx.DotGit().Root() checkout := &git.CheckoutOptions{Branch: plumbing.Master} @@ -190,6 +200,8 @@ func TestOpen(t *testing.T) { func TestCheckout(t *testing.T) { err := fixtures.Init() require.NoError(t, err) + defer testutil.CleanUpGitFixtures(t) + fx := fixtures.Basic().One() url := fx.DotGit().Root() checkout := &git.CheckoutOptions{Branch: plumbing.Master} diff --git a/pkg/environment/settings_test.go b/pkg/environment/settings_test.go index 9ae2071d9..ad9a0d8be 100644 --- a/pkg/environment/settings_test.go +++ b/pkg/environment/settings_test.go @@ -17,16 +17,15 @@ limitations under the License. package environment import ( - "io/ioutil" "os" "path/filepath" "testing" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" - "github.com/stretchr/testify/require" "opendev.org/airship/airshipctl/pkg/config" + "opendev.org/airship/airshipctl/testutil" ) func TestInitFlags(t *testing.T) { @@ -40,8 +39,8 @@ func TestInitFlags(t *testing.T) { func TestInitConfig(t *testing.T) { t.Run("DefaultToHomeDirectory", func(subTest *testing.T) { // Set up a fake $HOME directory - testDir := makeTestDir(t) - defer deleteTestDir(t, testDir) + testDir, cleanup := testutil.TempDir(t, "test-home") + defer cleanup(t) defer setHome(testDir)() var testSettings AirshipCTLSettings @@ -55,8 +54,8 @@ func TestInitConfig(t *testing.T) { t.Run("PreferEnvToDefault", func(subTest *testing.T) { // Set up a fake $HOME directory - testDir := makeTestDir(t) - defer deleteTestDir(t, testDir) + testDir, cleanup := testutil.TempDir(t, "test-home") + defer cleanup(t) defer setHome(testDir)() var testSettings AirshipCTLSettings @@ -75,8 +74,8 @@ func TestInitConfig(t *testing.T) { t.Run("PreferCmdLineArgToDefault", func(subTest *testing.T) { // Set up a fake $HOME directory - testDir := makeTestDir(t) - defer deleteTestDir(t, testDir) + testDir, cleanup := testutil.TempDir(t, "test-home") + defer cleanup(t) defer setHome(testDir)() var testSettings AirshipCTLSettings @@ -94,8 +93,8 @@ func TestInitConfig(t *testing.T) { t.Run("PreferCmdLineArgToEnv", func(subTest *testing.T) { // Set up a fake $HOME directory - testDir := makeTestDir(t) - defer deleteTestDir(t, testDir) + testDir, cleanup := testutil.TempDir(t, "test-home") + defer cleanup(t) defer setHome(testDir)() var testSettings AirshipCTLSettings @@ -121,17 +120,6 @@ func TestInitConfig(t *testing.T) { }) } -func makeTestDir(t *testing.T) string { - testDir, err := ioutil.TempDir("", "airship-test") - require.NoError(t, err) - return testDir -} - -func deleteTestDir(t *testing.T, path string) { - err := os.RemoveAll(path) - require.NoError(t, err) -} - // setHome sets the HOME environment variable to `path`, and returns a function // that can be used to reset HOME to its original value func setHome(path string) (resetHome func()) { diff --git a/testutil/testdatafs.go b/testutil/testdatafs.go index 9ec6f4a7c..ae7606b2d 100644 --- a/testutil/testdatafs.go +++ b/testutil/testdatafs.go @@ -2,10 +2,12 @@ package testutil import ( "io/ioutil" + "os" "path/filepath" "testing" "github.com/stretchr/testify/require" + fixtures "gopkg.in/src-d/go-git-fixtures.v3" "sigs.k8s.io/kustomize/v3/pkg/fs" "opendev.org/airship/airshipctl/pkg/document" @@ -41,3 +43,26 @@ func NewTestBundle(t *testing.T, fixtureDir string) document.Bundle { require.NoError(t, err, "Failed to build a bundle, setting up TestBundle failed") return b } + +// CleanUpGitFixtures removes any temp directories created by the go-git test fixtures +func CleanUpGitFixtures(t *testing.T) { + if err := fixtures.Clean(); err != nil { + t.Logf("Could not clean up git fixtures: %v", err) + } +} + +// TempDir creates a new temporary directory in the system's temporary file +// storage with a name beginning with prefix. +// It returns the path of the new directory and a function that can be used to +// easily clean up that directory +func TempDir(t *testing.T, prefix string) (path string, cleanup func(*testing.T)) { + path, err := ioutil.TempDir("", prefix) + require.NoError(t, err, "Failed to create a temporary directory") + + return path, func(tt *testing.T) { + err := os.RemoveAll(path) + if err != nil { + t.Logf("Could not clean up temp directory %q: %v", path, err) + } + } +} diff --git a/tools/gomod_check b/tools/gomod_check index 034dd9a18..30ace5172 100755 --- a/tools/gomod_check +++ b/tools/gomod_check @@ -2,6 +2,7 @@ set -e backup_dir=$(mktemp -d) +trap 'rm -rf "$backup_dir"' EXIT revert() { cp "$backup_dir/go.mod" "go.mod"