Skip to content

Commit 10b92da

Browse files
JuneezeeJuneezee
committed
test: use T.TempDir to create temporary test directory
This commit replaces `os.MkdirTemp` with `t.TempDir` in tests. The directory created by `t.TempDir` is automatically removed when the test and all its subtests complete. Prior to this commit, temporary directory created using `os.MkdirTemp` needs to be removed manually by calling `os.RemoveAll`, which is omitted in some tests. The error handling boilerplate e.g. defer func() { if err := os.RemoveAll(dir); err != nil { t.Fatal(err) } } is also tedious, but `t.TempDir` handles this for us nicely. Reference: https://pkg.go.dev/testing#T.TempDir Signed-off-by: Eng Zer Jun <[email protected]>
1 parent bc5a47e commit 10b92da

19 files changed

+125
-378
lines changed

‎controllers/bucket_controller_fetch_test.go‎

Lines changed: 18 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -107,19 +107,15 @@ func Test_fetchEtagIndex(t *testing.T) {
107107
}
108108

109109
t.Run("fetches etag index", func(t *testing.T) {
110-
tmp, err := os.MkdirTemp("", "test-bucket")
111-
if err != nil {
112-
t.Fatal(err)
113-
}
114-
defer os.RemoveAll(tmp)
110+
tmp := t.TempDir()
115111

116112
client := mockBucketClient{bucketName: bucketName}
117113
client.addObject("foo.yaml", mockBucketObject{data: "foo.yaml", etag: "etag1"})
118114
client.addObject("bar.yaml", mockBucketObject{data: "bar.yaml", etag: "etag2"})
119115
client.addObject("baz.yaml", mockBucketObject{data: "baz.yaml", etag: "etag3"})
120116

121117
index := newEtagIndex()
122-
err = fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
118+
err := fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
123119
if err != nil {
124120
t.Fatal(err)
125121
}
@@ -128,33 +124,25 @@ func Test_fetchEtagIndex(t *testing.T) {
128124
})
129125

130126
t.Run("an error while bucket does not exist", func(t *testing.T) {
131-
tmp, err := os.MkdirTemp("", "test-bucket")
132-
if err != nil {
133-
t.Fatal(err)
134-
}
135-
defer os.RemoveAll(tmp)
127+
tmp := t.TempDir()
136128

137129
client := mockBucketClient{bucketName: "other-bucket-name"}
138130

139131
index := newEtagIndex()
140-
err = fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
132+
err := fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
141133
assert.ErrorContains(t, err, "not found")
142134
})
143135

144136
t.Run("filters with .sourceignore rules", func(t *testing.T) {
145-
tmp, err := os.MkdirTemp("", "test-bucket")
146-
if err != nil {
147-
t.Fatal(err)
148-
}
149-
defer os.RemoveAll(tmp)
137+
tmp := t.TempDir()
150138

151139
client := mockBucketClient{bucketName: bucketName}
152140
client.addObject(".sourceignore", mockBucketObject{etag: "sourceignore1", data: `*.txt`})
153141
client.addObject("foo.yaml", mockBucketObject{etag: "etag1", data: "foo.yaml"})
154142
client.addObject("foo.txt", mockBucketObject{etag: "etag2", data: "foo.txt"})
155143

156144
index := newEtagIndex()
157-
err = fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
145+
err := fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
158146
if err != nil {
159147
t.Fatal(err)
160148
}
@@ -170,11 +158,7 @@ func Test_fetchEtagIndex(t *testing.T) {
170158
})
171159

172160
t.Run("filters with ignore rules from object", func(t *testing.T) {
173-
tmp, err := os.MkdirTemp("", "test-bucket")
174-
if err != nil {
175-
t.Fatal(err)
176-
}
177-
defer os.RemoveAll(tmp)
161+
tmp := t.TempDir()
178162

179163
client := mockBucketClient{bucketName: bucketName}
180164
client.addObject(".sourceignore", mockBucketObject{etag: "sourceignore1", data: `*.txt`})
@@ -185,7 +169,7 @@ func Test_fetchEtagIndex(t *testing.T) {
185169
bucket.Spec.Ignore = &ignore
186170

187171
index := newEtagIndex()
188-
err = fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
172+
err := fetchEtagIndex(context.TODO(), client, bucket.DeepCopy(), index, tmp)
189173
if err != nil {
190174
t.Fatal(err)
191175
}
@@ -212,11 +196,7 @@ func Test_fetchFiles(t *testing.T) {
212196
}
213197

214198
t.Run("fetches files", func(t *testing.T) {
215-
tmp, err := os.MkdirTemp("", "test-bucket")
216-
if err != nil {
217-
t.Fatal(err)
218-
}
219-
defer os.RemoveAll(tmp)
199+
tmp := t.TempDir()
220200

221201
client := mockBucketClient{bucketName: bucketName}
222202
client.addObject("foo.yaml", mockBucketObject{data: "foo.yaml", etag: "etag1"})
@@ -225,7 +205,7 @@ func Test_fetchFiles(t *testing.T) {
225205

226206
index := client.objectsToEtagIndex()
227207

228-
err = fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
208+
err := fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
229209
if err != nil {
230210
t.Fatal(err)
231211
}
@@ -240,34 +220,26 @@ func Test_fetchFiles(t *testing.T) {
240220
})
241221

242222
t.Run("an error while fetching returns an error for the whole procedure", func(t *testing.T) {
243-
tmp, err := os.MkdirTemp("", "test-bucket")
244-
if err != nil {
245-
t.Fatal(err)
246-
}
247-
defer os.RemoveAll(tmp)
223+
tmp := t.TempDir()
248224

249225
client := mockBucketClient{bucketName: bucketName, objects: map[string]mockBucketObject{}}
250226
client.objects["error"] = mockBucketObject{}
251227

252-
err = fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), client.objectsToEtagIndex(), tmp)
228+
err := fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), client.objectsToEtagIndex(), tmp)
253229
if err == nil {
254230
t.Fatal("expected error but got nil")
255231
}
256232
})
257233

258234
t.Run("a changed etag updates the index", func(t *testing.T) {
259-
tmp, err := os.MkdirTemp("", "test-bucket")
260-
if err != nil {
261-
t.Fatal(err)
262-
}
263-
defer os.RemoveAll(tmp)
235+
tmp := t.TempDir()
264236

265237
client := mockBucketClient{bucketName: bucketName}
266238
client.addObject("foo.yaml", mockBucketObject{data: "foo.yaml", etag: "etag2"})
267239

268240
index := newEtagIndex()
269241
index.Add("foo.yaml", "etag1")
270-
err = fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
242+
err := fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
271243
if err != nil {
272244
t.Fatal(err)
273245
}
@@ -276,11 +248,7 @@ func Test_fetchFiles(t *testing.T) {
276248
})
277249

278250
t.Run("a disappeared index entry is removed from the index", func(t *testing.T) {
279-
tmp, err := os.MkdirTemp("", "test-bucket")
280-
if err != nil {
281-
t.Fatal(err)
282-
}
283-
defer os.RemoveAll(tmp)
251+
tmp := t.TempDir()
284252

285253
client := mockBucketClient{bucketName: bucketName}
286254
client.addObject("foo.yaml", mockBucketObject{data: "foo.yaml", etag: "etag1"})
@@ -290,7 +258,7 @@ func Test_fetchFiles(t *testing.T) {
290258
// Does not exist on server
291259
index.Add("bar.yaml", "etag2")
292260

293-
err = fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
261+
err := fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
294262
if err != nil {
295263
t.Fatal(err)
296264
}
@@ -301,11 +269,7 @@ func Test_fetchFiles(t *testing.T) {
301269

302270
t.Run("can fetch more than maxConcurrentFetches", func(t *testing.T) {
303271
// this will fail if, for example, the semaphore is not used correctly and blocks
304-
tmp, err := os.MkdirTemp("", "test-bucket")
305-
if err != nil {
306-
t.Fatal(err)
307-
}
308-
defer os.RemoveAll(tmp)
272+
tmp := t.TempDir()
309273

310274
client := mockBucketClient{bucketName: bucketName}
311275
for i := 0; i < 2*maxConcurrentBucketFetches; i++ {
@@ -314,7 +278,7 @@ func Test_fetchFiles(t *testing.T) {
314278
}
315279
index := client.objectsToEtagIndex()
316280

317-
err = fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
281+
err := fetchIndexFiles(context.TODO(), client, bucket.DeepCopy(), index, tmp)
318282
if err != nil {
319283
t.Fatal(err)
320284
}

‎controllers/bucket_controller_test.go‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -539,9 +539,7 @@ func TestBucketReconciler_reconcileSource_generic(t *testing.T) {
539539
Client: builder.Build(),
540540
Storage: testStorage,
541541
}
542-
tmpDir, err := os.MkdirTemp("", "reconcile-bucket-source-")
543-
g.Expect(err).ToNot(HaveOccurred())
544-
defer os.RemoveAll(tmpDir)
542+
tmpDir := t.TempDir()
545543

546544
obj := &sourcev1.Bucket{
547545
TypeMeta: metav1.TypeMeta{
@@ -834,9 +832,7 @@ func TestBucketReconciler_reconcileSource_gcs(t *testing.T) {
834832
Client: builder.Build(),
835833
Storage: testStorage,
836834
}
837-
tmpDir, err := os.MkdirTemp("", "reconcile-bucket-source-")
838-
g.Expect(err).ToNot(HaveOccurred())
839-
defer os.RemoveAll(tmpDir)
835+
tmpDir := t.TempDir()
840836

841837
// Test bucket object.
842838
obj := &sourcev1.Bucket{
@@ -992,9 +988,7 @@ func TestBucketReconciler_reconcileArtifact(t *testing.T) {
992988
Storage: testStorage,
993989
}
994990

995-
tmpDir, err := os.MkdirTemp("", "reconcile-bucket-artifact-")
996-
g.Expect(err).ToNot(HaveOccurred())
997-
defer os.RemoveAll(tmpDir)
991+
tmpDir := t.TempDir()
998992

999993
obj := &sourcev1.Bucket{
1000994
TypeMeta: metav1.TypeMeta{

‎controllers/gitrepository_controller_test.go‎

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -509,9 +509,7 @@ func TestGitRepositoryReconciler_reconcileSource_authStrategy(t *testing.T) {
509509
t.Skipf("Skipped for Git implementation %q", i)
510510
}
511511

512-
tmpDir, err := os.MkdirTemp("", "auth-strategy-")
513-
g.Expect(err).To(BeNil())
514-
defer os.RemoveAll(tmpDir)
512+
tmpDir := t.TempDir()
515513

516514
obj := obj.DeepCopy()
517515
obj.Spec.GitImplementation = i
@@ -671,9 +669,7 @@ func TestGitRepositoryReconciler_reconcileSource_checkoutStrategy(t *testing.T)
671669
t.Skipf("Skipped for Git implementation %q", i)
672670
}
673671

674-
tmpDir, err := os.MkdirTemp("", "checkout-strategy-")
675-
g.Expect(err).NotTo(HaveOccurred())
676-
defer os.RemoveAll(tmpDir)
672+
tmpDir := t.TempDir()
677673

678674
obj := obj.DeepCopy()
679675
obj.Spec.GitImplementation = i
@@ -1072,9 +1068,7 @@ func TestGitRepositoryReconciler_reconcileInclude(t *testing.T) {
10721068
tt.beforeFunc(obj)
10731069
}
10741070

1075-
tmpDir, err := os.MkdirTemp("", "include-")
1076-
g.Expect(err).NotTo(HaveOccurred())
1077-
defer os.RemoveAll(tmpDir)
1071+
tmpDir := t.TempDir()
10781072

10791073
var commit git.Commit
10801074
var includes artifactSet

‎controllers/helmchart_controller_test.go‎

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -307,9 +307,7 @@ func TestHelmChartReconciler_reconcileStorage(t *testing.T) {
307307
func TestHelmChartReconciler_reconcileSource(t *testing.T) {
308308
g := NewWithT(t)
309309

310-
tmpDir, err := os.MkdirTemp("", "reconcile-tarball-")
311-
g.Expect(err).ToNot(HaveOccurred())
312-
defer os.RemoveAll(tmpDir)
310+
tmpDir := t.TempDir()
313311

314312
storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
315313
g.Expect(err).ToNot(HaveOccurred())
@@ -781,9 +779,7 @@ func TestHelmChartReconciler_buildFromHelmRepository(t *testing.T) {
781779
func TestHelmChartReconciler_buildFromTarballArtifact(t *testing.T) {
782780
g := NewWithT(t)
783781

784-
tmpDir, err := os.MkdirTemp("", "reconcile-tarball-")
785-
g.Expect(err).ToNot(HaveOccurred())
786-
defer os.RemoveAll(tmpDir)
782+
tmpDir := t.TempDir()
787783

788784
storage, err := NewStorage(tmpDir, "example.com", retentionTTL, retentionRecords)
789785
g.Expect(err).ToNot(HaveOccurred())

‎controllers/helmrepository_controller_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -728,9 +728,7 @@ func TestHelmRepositoryReconciler_reconcileArtifact(t *testing.T) {
728728
},
729729
}
730730

731-
tmpDir, err := os.MkdirTemp("", "test-reconcile-artifact-")
732-
g.Expect(err).ToNot(HaveOccurred())
733-
defer os.RemoveAll(tmpDir)
731+
tmpDir := t.TempDir()
734732

735733
// Create an empty cache file.
736734
cachePath := filepath.Join(tmpDir, "index.yaml")

0 commit comments

Comments
 (0)