Skip to content

Commit 5bb4283

Browse files
committed
proper file close operation based on feedback
Signed-off-by: Tom Huang <tom.huang@weave.works>
1 parent 8868d39 commit 5bb4283

File tree

2 files changed

+17
-14
lines changed

2 files changed

+17
-14
lines changed

controllers/storage.go

+7-4
Original file line numberDiff line numberDiff line change
@@ -355,10 +355,13 @@ func (s *Storage) CopyFromPath(artifact *sourcev1.Artifact, path string) (err er
355355
if err != nil {
356356
return err
357357
}
358-
if err = s.Copy(artifact, f); err != nil {
359-
return err
360-
}
361-
return f.Close()
358+
defer func() {
359+
if cerr := f.Close(); cerr != nil && err == nil {
360+
err = cerr
361+
}
362+
}()
363+
err = s.Copy(artifact, f)
364+
return err
362365
}
363366

364367
// CopyToPath copies the contents in the (sub)path of the given artifact to the given path.

controllers/storage_test.go

+10-10
Original file line numberDiff line numberDiff line change
@@ -303,21 +303,21 @@ func TestStorageCopyFromPath(t *testing.T) {
303303
return
304304
}
305305

306-
matchFile := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, file *File, wantErr bool) {
306+
matchFile := func(t *testing.T, storage *Storage, artifact sourcev1.Artifact, file *File, expectMismatch bool) {
307307
c, err := os.ReadFile(storage.LocalPath(artifact))
308308
if err != nil {
309309
t.Fatalf("failed reading file: %v", err)
310310
}
311-
if (string(c) != string(file.Content)) != wantErr {
312-
t.Errorf("artifact content does not match, got: %q, want: %q", string(c), string(file.Content))
311+
if (string(c) != string(file.Content)) != expectMismatch {
312+
t.Errorf("artifact content does not match and not expecting mismatch, got: %q, want: %q", string(c), string(file.Content))
313313
}
314314
}
315315

316316
tests := []struct {
317-
name string
318-
file *File
319-
want *File
320-
wantErr bool
317+
name string
318+
file *File
319+
want *File
320+
expectMismatch bool
321321
}{
322322
{
323323
name: "content match",
@@ -338,9 +338,9 @@ func TestStorageCopyFromPath(t *testing.T) {
338338
},
339339
want: &File{
340340
Name: "manifest.yaml",
341-
Content: []byte(`bad contents`),
341+
Content: []byte(`mismatch contents`),
342342
},
343-
wantErr: true,
343+
expectMismatch: true,
344344
},
345345
}
346346
for _, tt := range tests {
@@ -360,7 +360,7 @@ func TestStorageCopyFromPath(t *testing.T) {
360360
if err := storage.CopyFromPath(&artifact, absPath); err != nil {
361361
t.Errorf("CopyFromPath() error = %v", err)
362362
}
363-
matchFile(t, storage, artifact, tt.want, tt.wantErr)
363+
matchFile(t, storage, artifact, tt.want, tt.expectMismatch)
364364
})
365365
}
366366
}

0 commit comments

Comments
 (0)