From 3350760d85714bd94291e017ab49c9e5554e05c3 Mon Sep 17 00:00:00 2001 From: Li Zhijian Date: Wed, 9 Sep 2020 14:12:52 +0800 Subject: [PATCH] testing: Use T.cleanup() to cleanup tempdir Previously, after 'make test', some many temp dir have not been removed. Since this commit, temp dir created by ensure.HelmHome() ensure.TempDir() and ensure.TempFile() will be removed automatically in the end of testing - Use T.cleanup to remove the tempdir - Remove unnecessary defer Signed-off-by: Li Zhijian --- cmd/helm/create_test.go | 6 +++--- cmd/helm/dependency_update_test.go | 2 +- cmd/helm/repo_update_test.go | 2 +- cmd/helm/root_test.go | 2 +- internal/test/ensure/ensure.go | 13 ++++--------- pkg/action/package_test.go | 4 ---- pkg/chartutil/save_test.go | 1 - pkg/downloader/chart_downloader_test.go | 2 +- pkg/lint/rules/dependencies_test.go | 2 -- pkg/lint/rules/template_test.go | 2 -- pkg/lint/rules/values_test.go | 4 ---- pkg/plugin/installer/http_installer_test.go | 6 +++--- pkg/plugin/installer/vcs_installer_test.go | 6 +++--- pkg/repo/repotest/server_test.go | 4 ++-- 14 files changed, 19 insertions(+), 37 deletions(-) diff --git a/cmd/helm/create_test.go b/cmd/helm/create_test.go index 1db6bed52..a65036711 100644 --- a/cmd/helm/create_test.go +++ b/cmd/helm/create_test.go @@ -31,7 +31,7 @@ import ( ) func TestCreateCmd(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) cname := "testchart" dir := ensure.TempDir(t) defer testChdir(t, dir)() @@ -62,7 +62,7 @@ func TestCreateCmd(t *testing.T) { } func TestCreateStarterCmd(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) cname := "testchart" defer resetEnv()() os.MkdirAll(helmpath.CachePath(), 0755) @@ -128,7 +128,7 @@ func TestCreateStarterCmd(t *testing.T) { func TestCreateStarterAbsoluteCmd(t *testing.T) { defer resetEnv()() - defer ensure.HelmHome(t)() + ensure.HelmHome(t) cname := "testchart" // Create a starter. diff --git a/cmd/helm/dependency_update_test.go b/cmd/helm/dependency_update_test.go index 896018735..e11d48774 100644 --- a/cmd/helm/dependency_update_test.go +++ b/cmd/helm/dependency_update_test.go @@ -152,7 +152,7 @@ func TestDependencyUpdateCmd(t *testing.T) { func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) { defer resetEnv()() - defer ensure.HelmHome(t)() + ensure.HelmHome(t) srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz") if err != nil { diff --git a/cmd/helm/repo_update_test.go b/cmd/helm/repo_update_test.go index f4936b367..611557ccd 100644 --- a/cmd/helm/repo_update_test.go +++ b/cmd/helm/repo_update_test.go @@ -73,7 +73,7 @@ func TestUpdateCustomCacheCmd(t *testing.T) { func TestUpdateCharts(t *testing.T) { defer resetEnv()() - defer ensure.HelmHome(t)() + ensure.HelmHome(t) ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*") if err != nil { diff --git a/cmd/helm/root_test.go b/cmd/helm/root_test.go index 075544971..65e6d66c7 100644 --- a/cmd/helm/root_test.go +++ b/cmd/helm/root_test.go @@ -77,7 +77,7 @@ func TestRootCmd(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) for k, v := range tt.envvars { os.Setenv(k, v) diff --git a/internal/test/ensure/ensure.go b/internal/test/ensure/ensure.go index 3c0e4575c..f15098797 100644 --- a/internal/test/ensure/ensure.go +++ b/internal/test/ensure/ensure.go @@ -27,7 +27,7 @@ import ( ) // HelmHome sets up a Helm Home in a temp dir. -func HelmHome(t *testing.T) func() { +func HelmHome(t *testing.T) { t.Helper() base := TempDir(t) os.Setenv(xdg.CacheHomeEnvVar, base) @@ -36,9 +36,6 @@ func HelmHome(t *testing.T) func() { os.Setenv(helmpath.CacheHomeEnvVar, "") os.Setenv(helmpath.ConfigHomeEnvVar, "") os.Setenv(helmpath.DataHomeEnvVar, "") - return func() { - os.RemoveAll(base) - } } // TempDir ensures a scratch test directory for unit testing purposes. @@ -48,6 +45,8 @@ func TempDir(t *testing.T) string { if err != nil { t.Fatal(err) } + t.Cleanup(func() { os.RemoveAll(d) }) + return d } @@ -55,11 +54,7 @@ func TempDir(t *testing.T) string { // // It returns the path to the directory (to which you will still need to join the filename) // -// You must clean up the directory that is returned. -// -// tempdir := TempFile(t, "foo", []byte("bar")) -// defer os.RemoveAll(tempdir) -// filename := filepath.Join(tempdir, "foo") +// The whole TempDir will be Remove automatically func TempFile(t *testing.T, name string, data []byte) string { path := TempDir(t) filename := filepath.Join(path, name) diff --git a/pkg/action/package_test.go b/pkg/action/package_test.go index 5c5fed571..8e6533059 100644 --- a/pkg/action/package_test.go +++ b/pkg/action/package_test.go @@ -18,7 +18,6 @@ package action import ( "io/ioutil" - "os" "path" "testing" @@ -30,7 +29,6 @@ import ( func TestPassphraseFileFetcher(t *testing.T) { secret := "secret" directory := ensure.TempFile(t, "passphrase-file", []byte(secret)) - defer os.RemoveAll(directory) fetcher, err := passphraseFileFetcher(path.Join(directory, "passphrase-file"), nil) if err != nil { @@ -50,7 +48,6 @@ func TestPassphraseFileFetcher(t *testing.T) { func TestPassphraseFileFetcher_WithLineBreak(t *testing.T) { secret := "secret" directory := ensure.TempFile(t, "passphrase-file", []byte(secret+"\n\n.")) - defer os.RemoveAll(directory) fetcher, err := passphraseFileFetcher(path.Join(directory, "passphrase-file"), nil) if err != nil { @@ -69,7 +66,6 @@ func TestPassphraseFileFetcher_WithLineBreak(t *testing.T) { func TestPassphraseFileFetcher_WithInvalidStdin(t *testing.T) { directory := ensure.TempDir(t) - defer os.RemoveAll(directory) stdin, err := ioutil.TempFile(directory, "non-existing") if err != nil { diff --git a/pkg/chartutil/save_test.go b/pkg/chartutil/save_test.go index 3a45b2992..c92ff8dcf 100644 --- a/pkg/chartutil/save_test.go +++ b/pkg/chartutil/save_test.go @@ -37,7 +37,6 @@ import ( func TestSave(t *testing.T) { tmp := ensure.TempDir(t) - defer os.RemoveAll(tmp) for _, dest := range []string{tmp, path.Join(tmp, "newdir")} { t.Run("outDir="+dest, func(t *testing.T) { diff --git a/pkg/downloader/chart_downloader_test.go b/pkg/downloader/chart_downloader_test.go index 334d7aaa1..0ae016592 100644 --- a/pkg/downloader/chart_downloader_test.go +++ b/pkg/downloader/chart_downloader_test.go @@ -280,7 +280,7 @@ func TestDownloadTo_TLS(t *testing.T) { } func TestDownloadTo_VerifyLater(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) dest := ensure.TempDir(t) defer os.RemoveAll(dest) diff --git a/pkg/lint/rules/dependencies_test.go b/pkg/lint/rules/dependencies_test.go index 075190eac..7d06d96bc 100644 --- a/pkg/lint/rules/dependencies_test.go +++ b/pkg/lint/rules/dependencies_test.go @@ -16,7 +16,6 @@ limitations under the License. package rules import ( - "os" "path/filepath" "testing" @@ -80,7 +79,6 @@ func TestValidateDependencyInMetadata(t *testing.T) { func TestDependencies(t *testing.T) { tmp := ensure.TempDir(t) - defer os.RemoveAll(tmp) c := chartWithBadDependencies() err := chartutil.SaveDir(&c, tmp) diff --git a/pkg/lint/rules/template_test.go b/pkg/lint/rules/template_test.go index eb076a1bf..aefc136b8 100644 --- a/pkg/lint/rules/template_test.go +++ b/pkg/lint/rules/template_test.go @@ -178,7 +178,6 @@ func TestDeprecatedAPIFails(t *testing.T) { }, } tmpdir := ensure.TempDir(t) - defer os.RemoveAll(tmpdir) if err := chartutil.SaveDir(&mychart, tmpdir); err != nil { t.Fatal(err) @@ -235,7 +234,6 @@ func TestStrictTemplateParsingMapError(t *testing.T) { }, } dir := ensure.TempDir(t) - defer os.RemoveAll(dir) if err := chartutil.SaveDir(&ch, dir); err != nil { t.Fatal(err) } diff --git a/pkg/lint/rules/values_test.go b/pkg/lint/rules/values_test.go index 6d93d630e..86ee6a4ee 100644 --- a/pkg/lint/rules/values_test.go +++ b/pkg/lint/rules/values_test.go @@ -67,7 +67,6 @@ func TestValidateValuesFileWellFormed(t *testing.T) { not:well[]{}formed ` tmpdir := ensure.TempFile(t, "values.yaml", []byte(badYaml)) - defer os.RemoveAll(tmpdir) valfile := filepath.Join(tmpdir, "values.yaml") if err := validateValuesFile(valfile, map[string]interface{}{}); err == nil { t.Fatal("expected values file to fail parsing") @@ -77,7 +76,6 @@ func TestValidateValuesFileWellFormed(t *testing.T) { func TestValidateValuesFileSchema(t *testing.T) { yaml := "username: admin\npassword: swordfish" tmpdir := ensure.TempFile(t, "values.yaml", []byte(yaml)) - defer os.RemoveAll(tmpdir) createTestingSchema(t, tmpdir) valfile := filepath.Join(tmpdir, "values.yaml") @@ -90,7 +88,6 @@ func TestValidateValuesFileSchemaFailure(t *testing.T) { // 1234 is an int, not a string. This should fail. yaml := "username: 1234\npassword: swordfish" tmpdir := ensure.TempFile(t, "values.yaml", []byte(yaml)) - defer os.RemoveAll(tmpdir) createTestingSchema(t, tmpdir) valfile := filepath.Join(tmpdir, "values.yaml") @@ -109,7 +106,6 @@ func TestValidateValuesFileSchemaOverrides(t *testing.T) { "password": "swordfish", } tmpdir := ensure.TempFile(t, "values.yaml", []byte(yaml)) - defer os.RemoveAll(tmpdir) createTestingSchema(t, tmpdir) valfile := filepath.Join(tmpdir, "values.yaml") diff --git a/pkg/plugin/installer/http_installer_test.go b/pkg/plugin/installer/http_installer_test.go index e89fea29d..812e4e8e6 100644 --- a/pkg/plugin/installer/http_installer_test.go +++ b/pkg/plugin/installer/http_installer_test.go @@ -80,7 +80,7 @@ func mockArchiveServer() *httptest.Server { } func TestHTTPInstaller(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) srv := mockArchiveServer() defer srv.Close() @@ -129,7 +129,7 @@ func TestHTTPInstaller(t *testing.T) { } func TestHTTPInstallerNonExistentVersion(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) srv := mockArchiveServer() defer srv.Close() source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" @@ -165,7 +165,7 @@ func TestHTTPInstallerUpdate(t *testing.T) { srv := mockArchiveServer() defer srv.Close() source := srv.URL + "/plugins/fake-plugin-0.0.1.tar.gz" - defer ensure.HelmHome(t)() + ensure.HelmHome(t) if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) diff --git a/pkg/plugin/installer/vcs_installer_test.go b/pkg/plugin/installer/vcs_installer_test.go index 6785264b3..0bb0b6780 100644 --- a/pkg/plugin/installer/vcs_installer_test.go +++ b/pkg/plugin/installer/vcs_installer_test.go @@ -49,7 +49,7 @@ func (r *testRepo) UpdateVersion(version string) error { } func TestVCSInstaller(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) if err := os.MkdirAll(helmpath.DataPath("plugins"), 0755); err != nil { t.Fatalf("Could not create %s: %s", helmpath.DataPath("plugins"), err) @@ -102,7 +102,7 @@ func TestVCSInstaller(t *testing.T) { } func TestVCSInstallerNonExistentVersion(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) source := "https://github.com/adamreese/helm-env" version := "0.2.0" @@ -124,7 +124,7 @@ func TestVCSInstallerNonExistentVersion(t *testing.T) { } } func TestVCSInstallerUpdate(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) source := "https://github.com/adamreese/helm-env" diff --git a/pkg/repo/repotest/server_test.go b/pkg/repo/repotest/server_test.go index 1ad979fdc..2849d7da6 100644 --- a/pkg/repo/repotest/server_test.go +++ b/pkg/repo/repotest/server_test.go @@ -31,7 +31,7 @@ import ( // Young'n, in these here parts, we test our tests. func TestServer(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) rootDir := ensure.TempDir(t) defer os.RemoveAll(rootDir) @@ -99,7 +99,7 @@ func TestServer(t *testing.T) { } func TestNewTempServer(t *testing.T) { - defer ensure.HelmHome(t)() + ensure.HelmHome(t) srv, err := NewTempServerWithCleanup(t, "testdata/examplechart-0.1.0.tgz") if err != nil {