Skip to content

Commit a35b85d

Browse files
yangcao77yangcao77
committed
update add apis description, and check error in unit tests
Signed-off-by: Stephanie <[email protected]>
1 parent 7724ad6 commit a35b85d

8 files changed

+102
-39
lines changed

‎pkg/devfile/parser/data/v2/commands.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,8 @@ func (d *DevfileV2) GetCommands(options common.DevfileOptions) ([]v1.Command, er
5050
}
5151

5252
// AddCommands adds the slice of Command objects to the Devfile's commands
53-
// if a command is already defined, error out
53+
// a command is considered as invalid if it is already defined
54+
// command list passed in will be all processed, and returns a total error of all invalid commands
5455
func (d *DevfileV2) AddCommands(commands []v1.Command) error {
5556
var errorsList []string
5657
for _, command := range commands {

‎pkg/devfile/parser/data/v2/commands_test.go

+27-8
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package v2
22

33
import (
4+
"fmt"
45
"reflect"
56
"testing"
67

@@ -226,15 +227,13 @@ func TestDevfile200_GetCommands(t *testing.T) {
226227
}
227228

228229
func TestDevfile200_AddCommands(t *testing.T) {
230+
multipleDupError := fmt.Sprintf("%s\n%s", "command command1 already exists in devfile", "command command2 already exists in devfile")
229231

230-
type args struct {
231-
name string
232-
}
233232
tests := []struct {
234233
name string
235234
currentCommands []v1.Command
236235
newCommands []v1.Command
237-
wantErr bool
236+
wantErr *string
238237
}{
239238
{
240239
name: "Command does not exist",
@@ -260,17 +259,23 @@ func TestDevfile200_AddCommands(t *testing.T) {
260259
},
261260
},
262261
},
263-
wantErr: false,
262+
wantErr: nil,
264263
},
265264
{
266-
name: "Command does exist",
265+
name: "Multiple duplicate commands",
267266
currentCommands: []v1.Command{
268267
{
269268
Id: "command1",
270269
CommandUnion: v1.CommandUnion{
271270
Exec: &v1.ExecCommand{},
272271
},
273272
},
273+
{
274+
Id: "command2",
275+
CommandUnion: v1.CommandUnion{
276+
Exec: &v1.ExecCommand{},
277+
},
278+
},
274279
},
275280
newCommands: []v1.Command{
276281
{
@@ -279,8 +284,20 @@ func TestDevfile200_AddCommands(t *testing.T) {
279284
Exec: &v1.ExecCommand{},
280285
},
281286
},
287+
{
288+
Id: "command2",
289+
CommandUnion: v1.CommandUnion{
290+
Exec: &v1.ExecCommand{},
291+
},
292+
},
293+
{
294+
Id: "command3",
295+
CommandUnion: v1.CommandUnion{
296+
Exec: &v1.ExecCommand{},
297+
},
298+
},
282299
},
283-
wantErr: true,
300+
wantErr: &multipleDupError,
284301
},
285302
}
286303
for _, tt := range tests {
@@ -297,8 +314,10 @@ func TestDevfile200_AddCommands(t *testing.T) {
297314

298315
err := d.AddCommands(tt.newCommands)
299316
// Unexpected error
300-
if (err != nil) != tt.wantErr {
317+
if (err != nil) != (tt.wantErr != nil) {
301318
t.Errorf("TestDevfile200_AddCommands() error = %v, wantErr %v", err, tt.wantErr)
319+
} else if tt.wantErr != nil {
320+
assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
302321
}
303322
})
304323
}

‎pkg/devfile/parser/data/v2/components.go

+2-1
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,8 @@ func (d *DevfileV2) GetDevfileVolumeComponents(options common.DevfileOptions) ([
7474
}
7575

7676
// AddComponents adds the slice of Component objects to the devfile's components
77-
// if a component is already defined, error out
77+
// a component is considered as invalid if it is already defined
78+
// component list passed in will be all processed, and returns a total error of all invalid components
7879
func (d *DevfileV2) AddComponents(components []v1.Component) error {
7980
var errorsList []string
8081
for _, component := range components {

‎pkg/devfile/parser/data/v2/components_test.go

+20-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package v2
22

33
import (
4+
"fmt"
45
"reflect"
56
"testing"
67

@@ -12,12 +13,13 @@ import (
1213
)
1314

1415
func TestDevfile200_AddComponent(t *testing.T) {
16+
multipleDupError := fmt.Sprintf("%s\n%s", "component component1 already exists in devfile", "component component2 already exists in devfile")
1517

1618
tests := []struct {
1719
name string
1820
currentComponents []v1.Component
1921
newComponents []v1.Component
20-
wantErr bool
22+
wantErr *string
2123
}{
2224
{
2325
name: "successfully add the component",
@@ -43,7 +45,7 @@ func TestDevfile200_AddComponent(t *testing.T) {
4345
},
4446
},
4547
},
46-
wantErr: false,
48+
wantErr: nil,
4749
},
4850
{
4951
name: "error out on duplicate component",
@@ -68,8 +70,20 @@ func TestDevfile200_AddComponent(t *testing.T) {
6870
Container: &v1.ContainerComponent{},
6971
},
7072
},
73+
{
74+
Name: "component2",
75+
ComponentUnion: v1.ComponentUnion{
76+
Volume: &v1.VolumeComponent{},
77+
},
78+
},
79+
{
80+
Name: "component3",
81+
ComponentUnion: v1.ComponentUnion{
82+
Volume: &v1.VolumeComponent{},
83+
},
84+
},
7185
},
72-
wantErr: true,
86+
wantErr: &multipleDupError,
7387
},
7488
}
7589
for _, tt := range tests {
@@ -86,8 +100,10 @@ func TestDevfile200_AddComponent(t *testing.T) {
86100

87101
err := d.AddComponents(tt.newComponents)
88102
// Unexpected error
89-
if (err != nil) != tt.wantErr {
103+
if (err != nil) != (tt.wantErr != nil) {
90104
t.Errorf("TestDevfile200_AddComponents() error = %v, wantErr %v", err, tt.wantErr)
105+
} else if tt.wantErr != nil {
106+
assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
91107
}
92108
})
93109
}

‎pkg/devfile/parser/data/v2/events.go

+6-5
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,8 @@ func (d *DevfileV2) GetEvents() v1.Events {
1616
}
1717

1818
// AddEvents adds the Events Object to the devfile's events
19-
// if the event is already defined in the devfile, error out
19+
// an event field is considered as invalid if it is already defined
20+
// all event fields will be checked and processed, and returns a total error of all event fields
2021
func (d *DevfileV2) AddEvents(events v1.Events) error {
2122

2223
if d.Events == nil {
@@ -25,31 +26,31 @@ func (d *DevfileV2) AddEvents(events v1.Events) error {
2526
var errorsList []string
2627
if len(events.PreStop) > 0 {
2728
if len(d.Events.PreStop) > 0 {
28-
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "pre stop"}).Error())
29+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre stop"}).Error())
2930
} else {
3031
d.Events.PreStop = events.PreStop
3132
}
3233
}
3334

3435
if len(events.PreStart) > 0 {
3536
if len(d.Events.PreStart) > 0 {
36-
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "pre start"}).Error())
37+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre start"}).Error())
3738
} else {
3839
d.Events.PreStart = events.PreStart
3940
}
4041
}
4142

4243
if len(events.PostStop) > 0 {
4344
if len(d.Events.PostStop) > 0 {
44-
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "post stop"}).Error())
45+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post stop"}).Error())
4546
} else {
4647
d.Events.PostStop = events.PostStop
4748
}
4849
}
4950

5051
if len(events.PostStart) > 0 {
5152
if len(d.Events.PostStart) > 0 {
52-
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "post start"}).Error())
53+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post start"}).Error())
5354
} else {
5455
d.Events.PostStart = events.PostStart
5556
}

‎pkg/devfile/parser/data/v2/events_test.go

+14-6
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,22 @@
11
package v2
22

33
import (
4+
"fmt"
5+
"github.com/stretchr/testify/assert"
46
"reflect"
57
"testing"
68

79
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
810
)
911

1012
func TestDevfile200_AddEvents(t *testing.T) {
13+
multipleDupError := fmt.Sprintf("%s\n%s", "event field pre start already exists in devfile", "event field post stop already exists in devfile")
1114

1215
tests := []struct {
1316
name string
1417
currentEvents *v1.Events
1518
newEvents v1.Events
16-
wantErr bool
19+
wantErr *string
1720
}{
1821
{
1922
name: "successfully add the events",
@@ -27,7 +30,7 @@ func TestDevfile200_AddEvents(t *testing.T) {
2730
PostStart: []string{"postStart1"},
2831
},
2932
},
30-
wantErr: false,
33+
wantErr: nil,
3134
},
3235
{
3336
name: "successfully add the events to empty devfile event",
@@ -37,21 +40,24 @@ func TestDevfile200_AddEvents(t *testing.T) {
3740
PostStart: []string{"postStart1"},
3841
},
3942
},
40-
wantErr: false,
43+
wantErr: nil,
4144
},
4245
{
4346
name: "event already present",
4447
currentEvents: &v1.Events{
4548
DevWorkspaceEvents: v1.DevWorkspaceEvents{
4649
PreStart: []string{"preStart1"},
50+
PostStop: []string{"postStop1"},
4751
},
4852
},
4953
newEvents: v1.Events{
5054
DevWorkspaceEvents: v1.DevWorkspaceEvents{
5155
PreStart: []string{"preStart2"},
56+
PostStop: []string{"postStop2"},
57+
PreStop: []string{"preStop"},
5258
},
5359
},
54-
wantErr: true,
60+
wantErr: &multipleDupError,
5561
},
5662
}
5763
for _, tt := range tests {
@@ -68,8 +74,10 @@ func TestDevfile200_AddEvents(t *testing.T) {
6874

6975
err := d.AddEvents(tt.newEvents)
7076

71-
if (err != nil) != tt.wantErr {
72-
t.Errorf("DeleteCommand() error = %v, wantErr %v", err, tt.wantErr)
77+
if (err != nil) != (tt.wantErr != nil) {
78+
t.Errorf("TestDevfile200_AddEvents() error = %v, wantErr %v", err, tt.wantErr)
79+
} else if tt.wantErr != nil {
80+
assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
7381
}
7482

7583
})

‎pkg/devfile/parser/data/v2/projects.go

+4-2
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ func (d *DevfileV2) GetProjects(options common.DevfileOptions) ([]v1.Project, er
4141
}
4242

4343
// AddProjects adss the slice of Devfile projects to the Devfile's project list
44-
// if a project is already defined, error out
44+
// a project is considered as invalid if it is already defined
45+
// project list passed in will be all processed, and returns a total error of all invalid projects
4546
func (d *DevfileV2) AddProjects(projects []v1.Project) error {
4647
projectsMap := make(map[string]bool)
4748
var errorsList []string
@@ -124,7 +125,8 @@ func (d *DevfileV2) GetStarterProjects(options common.DevfileOptions) ([]v1.Star
124125
}
125126

126127
// AddStarterProjects adds the slice of Devfile starter projects to the Devfile's starter project list
127-
// if a starter project is already defined, error out
128+
// a starterProject is considered as invalid if it is already defined
129+
// starterProject list passed in will be all processed, and returns a total error of all invalid starterProjects
128130
func (d *DevfileV2) AddStarterProjects(projects []v1.StarterProject) error {
129131
projectsMap := make(map[string]bool)
130132
var errorsList []string

0 commit comments

Comments
 (0)