Skip to content

Commit

Permalink
proper file close operation based on feedback
Browse files Browse the repository at this point in the history
Signed-off-by: Tom Huang <tom.huang@weave.works>
  • Loading branch information
tomhuang12 committed Jan 11, 2022
1 parent 8868d39 commit 5bb4283
Show file tree
Hide file tree
Showing 2 changed files with 17 additions and 14 deletions.
11 changes: 7 additions & 4 deletions controllers/storage.go
Original file line number Diff line number Diff line change
Expand Up @@ -355,10 +355,13 @@ func (s *Storage) CopyFromPath(artifact *sourcev1.Artifact, path string) (err er
if err != nil {
return err
}
if err = s.Copy(artifact, f); err != nil {
return err
}
return f.Close()
defer func() {
if cerr := f.Close(); cerr != nil && err == nil {
err = cerr
}
}()
err = s.Copy(artifact, f)
return err
}

// CopyToPath copies the contents in the (sub)path of the given artifact to the given path.
Expand Down
20 changes: 10 additions & 10 deletions controllers/storage_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -303,21 +303,21 @@ func TestStorageCopyFromPath(t *testing.T) {
return
}

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

tests := []struct {
name string
file *File
want *File
wantErr bool
name string
file *File
want *File
expectMismatch bool
}{
{
name: "content match",
Expand All @@ -338,9 +338,9 @@ func TestStorageCopyFromPath(t *testing.T) {
},
want: &File{
Name: "manifest.yaml",
Content: []byte(`bad contents`),
Content: []byte(`mismatch contents`),
},
wantErr: true,
expectMismatch: true,
},
}
for _, tt := range tests {
Expand All @@ -360,7 +360,7 @@ func TestStorageCopyFromPath(t *testing.T) {
if err := storage.CopyFromPath(&artifact, absPath); err != nil {
t.Errorf("CopyFromPath() error = %v", err)
}
matchFile(t, storage, artifact, tt.want, tt.wantErr)
matchFile(t, storage, artifact, tt.want, tt.expectMismatch)
})
}
}

0 comments on commit 5bb4283

Please sign in to comment.