From cc71517ed39d95e2980081c6c80e402dba30e024 Mon Sep 17 00:00:00 2001
From: Hidde Beydals <hello@hidde.co>
Date: Tue, 16 Nov 2021 10:39:58 +0100
Subject: [PATCH] tests: ensure proper garbage collection

- Ensure the proper path is garbage collected for libgit2 repositories,
  as the `Path` method on the repository object returns the `.git`
  directory, and not the root path.
- Ensure the Helm test server does not get swapped during tests,
  with as side-effect that no obsolete temporary directories remain.

Signed-off-by: Hidde Beydals <hello@hidde.co>
---
 controllers/helmchart_controller_test.go      |  4 ++--
 controllers/helmrepository_controller_test.go |  7 +------
 pkg/git/libgit2/checkout_test.go              | 20 ++++++++++++++-----
 3 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/controllers/helmchart_controller_test.go b/controllers/helmchart_controller_test.go
index de3f7ad321..35462d467f 100644
--- a/controllers/helmchart_controller_test.go
+++ b/controllers/helmchart_controller_test.go
@@ -79,8 +79,8 @@ var _ = Describe("HelmChartReconciler", func() {
 		})
 
 		AfterEach(func() {
-			os.RemoveAll(helmServer.Root())
 			helmServer.Stop()
+			os.RemoveAll(helmServer.Root())
 
 			err = k8sClient.Delete(context.Background(), namespace)
 			Expect(err).NotTo(HaveOccurred(), "failed to delete test namespace")
@@ -995,8 +995,8 @@ var _ = Describe("HelmChartReconciler", func() {
 			gitServer.StopHTTP()
 			os.RemoveAll(gitServer.Root())
 
-			os.RemoveAll(helmServer.Root())
 			helmServer.Stop()
+			os.RemoveAll(helmServer.Root())
 
 			err = k8sClient.Delete(context.Background(), namespace)
 			Expect(err).NotTo(HaveOccurred(), "failed to delete test namespace")
diff --git a/controllers/helmrepository_controller_test.go b/controllers/helmrepository_controller_test.go
index 126ed11c50..e7d945a60b 100644
--- a/controllers/helmrepository_controller_test.go
+++ b/controllers/helmrepository_controller_test.go
@@ -64,8 +64,8 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 		})
 
 		AfterEach(func() {
-			os.RemoveAll(helmServer.Root())
 			helmServer.Stop()
+			os.RemoveAll(helmServer.Root())
 
 			Eventually(func() error {
 				return k8sClient.Delete(context.Background(), namespace)
@@ -207,9 +207,6 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 		})
 
 		It("Authenticates when basic auth credentials are provided", func() {
-			helmServer, err = helmtestserver.NewTempHelmServer()
-			Expect(err).NotTo(HaveOccurred())
-
 			var username, password = "john", "doe"
 			helmServer.WithMiddleware(func(handler http.Handler) http.Handler {
 				return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
@@ -221,8 +218,6 @@ var _ = Describe("HelmRepositoryReconciler", func() {
 					handler.ServeHTTP(w, r)
 				})
 			})
-			defer os.RemoveAll(helmServer.Root())
-			defer helmServer.Stop()
 			helmServer.Start()
 
 			Expect(helmServer.PackageChart(path.Join("testdata/charts/helmchart"))).Should(Succeed())
diff --git a/pkg/git/libgit2/checkout_test.go b/pkg/git/libgit2/checkout_test.go
index 24ca72b30f..c244760942 100644
--- a/pkg/git/libgit2/checkout_test.go
+++ b/pkg/git/libgit2/checkout_test.go
@@ -34,6 +34,8 @@ func TestCheckoutBranch_Checkout(t *testing.T) {
 	if err != nil {
 		t.Fatal(err)
 	}
+	defer repo.Free()
+	defer os.RemoveAll(filepath.Join(repo.Path(), ".."))
 
 	firstCommit, err := commitFile(repo, "branch", "init", time.Now())
 	if err != nil {
@@ -131,6 +133,8 @@ func TestCheckoutTag_Checkout(t *testing.T) {
 			if err != nil {
 				t.Fatal(err)
 			}
+			defer repo.Free()
+			defer os.RemoveAll(filepath.Join(repo.Path(), ".."))
 
 			var commit *git2go.Commit
 			if tt.tag != "" {
@@ -177,7 +181,7 @@ func TestCheckoutCommit_Checkout(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer repo.Free()
-	defer os.RemoveAll(repo.Path())
+	defer os.RemoveAll(filepath.Join(repo.Path(), ".."))
 
 	c, err := commitFile(repo, "commit", "init", time.Now())
 	if err != nil {
@@ -190,7 +194,10 @@ func TestCheckoutCommit_Checkout(t *testing.T) {
 	commit := CheckoutCommit{
 		Commit: c.String(),
 	}
-	tmpDir, _ := os.MkdirTemp("", "git2go")
+	tmpDir, err := os.MkdirTemp("", "git2go")
+	if err != nil {
+		t.Fatal(err)
+	}
 	defer os.RemoveAll(tmpDir)
 
 	cc, err := commit.Checkout(context.TODO(), tmpDir, repo.Path(), nil)
@@ -203,8 +210,11 @@ func TestCheckoutCommit_Checkout(t *testing.T) {
 	commit = CheckoutCommit{
 		Commit: "4dc3185c5fc94eb75048376edeb44571cece25f4",
 	}
-	tmpDir2, _ := os.MkdirTemp("", "git2go")
-	defer os.RemoveAll(tmpDir)
+	tmpDir2, err := os.MkdirTemp("", "git2go")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.RemoveAll(tmpDir2)
 
 	cc, err = commit.Checkout(context.TODO(), tmpDir2, repo.Path(), nil)
 	g.Expect(err).To(HaveOccurred())
@@ -279,7 +289,7 @@ func TestCheckoutTagSemVer_Checkout(t *testing.T) {
 		t.Fatal(err)
 	}
 	defer repo.Free()
-	defer os.RemoveAll(repo.Path())
+	defer os.RemoveAll(filepath.Join(repo.Path(), ".."))
 
 	refs := make(map[string]string, len(tags))
 	for _, tt := range tags {