Merge pull request #8707 from zhijianli88/cleanup-tempfiles

Cleanup tempfiles

cmd/helm/dependency_build_test.go

@@ -28,7 +28,7 @@ import (
 )
 
 func TestDependencyBuildCmd(t *testing.T) {
-	srv, err := repotest.NewTempServer("testdata/testcharts/*.tgz")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz")
 	defer srv.Stop()
 	if err != nil {
 		t.Fatal(err)

cmd/helm/dependency_update_test.go

@@ -33,7 +33,7 @@ import (
 )
 
 func TestDependencyUpdateCmd(t *testing.T) {
-	srv, err := repotest.NewTempServer("testdata/testcharts/*.tgz")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -121,7 +121,7 @@ func TestDependencyUpdateCmd_DontDeleteOldChartsOnError(t *testing.T) {
 	defer resetEnv()()
 	defer ensure.HelmHome(t)()
 
-	srv, err := repotest.NewTempServer("testdata/testcharts/*.tgz")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/helm/pull_test.go

@@ -26,7 +26,7 @@ import (
 )
 
 func TestPullCmd(t *testing.T) {
-	srv, err := repotest.NewTempServer("testdata/testcharts/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/helm/repo_add_test.go

@@ -34,7 +34,7 @@ import (
 )
 
 func TestRepoAddCmd(t *testing.T) {
-	srv, err := repotest.NewTempServer("testdata/testserver/*.*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -53,7 +53,7 @@ func TestRepoAddCmd(t *testing.T) {
 }
 
 func TestRepoAdd(t *testing.T) {
-	ts, err := repotest.NewTempServer("testdata/testserver/*.*")
+	ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -118,7 +118,7 @@ func TestRepoAddConcurrentDirNotExist(t *testing.T) {
 }
 
 func repoAddConcurrent(t *testing.T, testName, repoFile string) {
-	ts, err := repotest.NewTempServer("testdata/testserver/*.*")
+	ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/helm/repo_remove_test.go

@@ -30,7 +30,7 @@ import (
 )
 
 func TestRepoRemove(t *testing.T) {
-	ts, err := repotest.NewTempServer("testdata/testserver/*.*")
+	ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/helm/repo_update_test.go

@@ -75,7 +75,7 @@ func TestUpdateCharts(t *testing.T) {
 	defer resetEnv()()
 	defer ensure.HelmHome(t)()
 
-	ts, err := repotest.NewTempServer("testdata/testserver/*.*")
+	ts, err := repotest.NewTempServerWithCleanup(t, "testdata/testserver/*.*")
 	if err != nil {
 		t.Fatal(err)
 	}

cmd/helm/show_test.go

@@ -26,7 +26,7 @@ import (
 )
 
 func TestShowPreReleaseChart(t *testing.T) {
-	srv, err := repotest.NewTempServer("testdata/testcharts/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/testcharts/*.tgz*")
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/action/action_test.go

@@ -20,6 +20,7 @@ import (
 	"flag"
 	"io/ioutil"
 	"net/http"
+	"os"
 	"path/filepath"
 	"testing"
 
@@ -56,6 +57,8 @@ func actionConfigFixture(t *testing.T) *Configuration {
 		t.Fatal(err)
 	}
 
+	t.Cleanup(func() { os.RemoveAll(tdir) })
+
 	cache, err := registry.NewCache(
 		registry.CacheOptDebug(true),
 		registry.CacheOptRoot(filepath.Join(tdir, registry.CacheRootDir)),

pkg/downloader/chart_downloader_test.go

@@ -172,7 +172,7 @@ func TestIsTar(t *testing.T) {
 
 func TestDownloadTo(t *testing.T) {
 	// Set up a fake repo with basic auth enabled
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*")
 	srv.Stop()
 	if err != nil {
 		t.Fatal(err)
@@ -229,7 +229,7 @@ func TestDownloadTo(t *testing.T) {
 
 func TestDownloadTo_TLS(t *testing.T) {
 	// Set up mock server w/ tls enabled
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*")
 	srv.Stop()
 	if err != nil {
 		t.Fatal(err)
@@ -285,7 +285,7 @@ func TestDownloadTo_VerifyLater(t *testing.T) {
 	dest := ensure.TempDir(t)
 
 	// Set up a fake repo
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*")
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/downloader/manager_test.go

@@ -183,7 +183,7 @@ func TestGetRepoNames(t *testing.T) {
 
 func TestUpdateBeforeBuild(t *testing.T) {
 	// Set up a fake repo
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*")
 	if err != nil {
 		t.Fatal(err)
 	}
@@ -257,7 +257,7 @@ func TestUpdateBeforeBuild(t *testing.T) {
 // If each of these main fields (name, version, repository) is not supplied by dep param, default value will be used.
 func checkBuildWithOptionalFields(t *testing.T, chartName string, dep chart.Dependency) {
 	// Set up a fake repo
-	srv, err := repotest.NewTempServer("testdata/*.tgz*")
+	srv, err := repotest.NewTempServerWithCleanup(t, "testdata/*.tgz*")
 	if err != nil {
 		t.Fatal(err)
 	}

pkg/kube/client_test.go

@@ -91,9 +91,12 @@ func newResponse(code int, obj runtime.Object) (*http.Response, error) {
 	return &http.Response{StatusCode: code, Header: header, Body: body}, nil
 }
 
-func newTestClient() *Client {
+func newTestClient(t *testing.T) *Client {
+	testFactory := cmdtesting.NewTestFactory()
+	t.Cleanup(testFactory.Cleanup)
+
 	return &Client{
-		Factory: cmdtesting.NewTestFactory().WithNamespace("default"),
+		Factory: testFactory.WithNamespace("default"),
 		Log:     nopLogger,
 	}
 }
@@ -107,7 +110,7 @@ func TestUpdate(t *testing.T) {
 
 	var actions []string
 
-	c := newTestClient()
+	c := newTestClient(t)
 	c.Factory.(*cmdtesting.TestFactory).UnstructuredClient = &fake.RESTClient{
 		NegotiatedSerializer: unstructuredSerializer,
 		Client: fake.CreateHTTPClient(func(req *http.Request) (*http.Response, error) {
@@ -232,7 +235,7 @@ func TestBuild(t *testing.T) {
 		},
 	}
 
-	c := newTestClient()
+	c := newTestClient(t)
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 			// Test for an invalid manifest
@@ -279,7 +282,7 @@ func TestPerform(t *testing.T) {
 				return nil
 			}
 
-			c := newTestClient()
+			c := newTestClient(t)
 			infos, err := c.Build(tt.reader, false)
 			if err != nil && err.Error() != tt.errMessage {
 				t.Errorf("Error while building manifests: %v", err)

pkg/repo/repotest/server.go

@@ -21,6 +21,7 @@ import (
 	"net/http/httptest"
 	"os"
 	"path/filepath"
+	"testing"
 
 	"helm.sh/helm/v3/internal/tlsutil"
 
@@ -29,6 +30,19 @@ import (
 	"helm.sh/helm/v3/pkg/repo"
 )
 
+// NewTempServerWithCleanup creates a server inside of a temp dir.
+//
+// If the passed in string is not "", it will be treated as a shell glob, and files
+// will be copied from that path to the server's docroot.
+//
+// The caller is responsible for stopping the server.
+// The temp dir will be removed by testing package automatically when test finished.
+func NewTempServerWithCleanup(t *testing.T, glob string) (*Server, error) {
+	srv, err := NewTempServer(glob)
+	t.Cleanup(func() { os.RemoveAll(srv.docroot) })
+	return srv, err
+}
+
 // NewTempServer creates a server inside of a temp dir.
 //
 // If the passed in string is not "", it will be treated as a shell glob, and files
@@ -36,6 +50,8 @@ import (
 //
 // The caller is responsible for destroying the temp directory as well as stopping
 // the server.
+//
+// Deprecated: use NewTempServerWithCleanup
 func NewTempServer(glob string) (*Server, error) {
 	tdir, err := ioutil.TempDir("", "helm-repotest-")
 	if err != nil {

pkg/repo/repotest/server_test.go

@@ -99,7 +99,7 @@ func TestServer(t *testing.T) {
 func TestNewTempServer(t *testing.T) {
 	defer ensure.HelmHome(t)()
 
-	srv, err := NewTempServer("testdata/examplechart-0.1.0.tgz")
+	srv, err := NewTempServerWithCleanup(t, "testdata/examplechart-0.1.0.tgz")
 	if err != nil {
 		t.Fatal(err)
 	}