Skip to content

Commit 5d88bd3

Browse files
yangcao77yangcao77
authored
Merge pull request #86 from yangcao77/449-add-apis-bug
update add apis - process everything before return an error
2 parents c59cfa4 + 25af2f1 commit 5d88bd3

8 files changed

+173
-51
lines changed

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import (
55
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
66
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
77
"reflect"
8+
"strings"
89
)
910

1011
// GetCommands returns the slice of Command objects parsed from the Devfile
@@ -49,17 +50,22 @@ func (d *DevfileV2) GetCommands(options common.DevfileOptions) ([]v1.Command, er
4950
}
5051

5152
// AddCommands adds the slice of Command objects to the Devfile's commands
52-
// 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
5355
func (d *DevfileV2) AddCommands(commands []v1.Command) error {
54-
56+
var errorsList []string
5557
for _, command := range commands {
5658
for _, devfileCommand := range d.Commands {
5759
if command.Id == devfileCommand.Id {
58-
return &common.FieldAlreadyExistError{Name: command.Id, Field: "command"}
60+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: command.Id, Field: "command"}).Error())
61+
continue
5962
}
6063
}
6164
d.Commands = append(d.Commands, command)
6265
}
66+
if len(errorsList) > 0 {
67+
return fmt.Errorf("errors while adding commands:\n%s", strings.Join(errorsList, "\n"))
68+
}
6369
return nil
6470
}
6571

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

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

33
import (
4+
"fmt"
5+
"github.com/kylelemons/godebug/pretty"
46
"reflect"
57
"testing"
68

@@ -226,15 +228,13 @@ func TestDevfile200_GetCommands(t *testing.T) {
226228
}
227229

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

230-
type args struct {
231-
name string
232-
}
233233
tests := []struct {
234234
name string
235235
currentCommands []v1.Command
236236
newCommands []v1.Command
237-
wantErr bool
237+
wantErr *string
238238
}{
239239
{
240240
name: "Command does not exist",
@@ -260,17 +260,23 @@ func TestDevfile200_AddCommands(t *testing.T) {
260260
},
261261
},
262262
},
263-
wantErr: false,
263+
wantErr: nil,
264264
},
265265
{
266-
name: "Command does exist",
266+
name: "Multiple duplicate commands",
267267
currentCommands: []v1.Command{
268268
{
269269
Id: "command1",
270270
CommandUnion: v1.CommandUnion{
271271
Exec: &v1.ExecCommand{},
272272
},
273273
},
274+
{
275+
Id: "command2",
276+
CommandUnion: v1.CommandUnion{
277+
Exec: &v1.ExecCommand{},
278+
},
279+
},
274280
},
275281
newCommands: []v1.Command{
276282
{
@@ -279,8 +285,20 @@ func TestDevfile200_AddCommands(t *testing.T) {
279285
Exec: &v1.ExecCommand{},
280286
},
281287
},
288+
{
289+
Id: "command2",
290+
CommandUnion: v1.CommandUnion{
291+
Exec: &v1.ExecCommand{},
292+
},
293+
},
294+
{
295+
Id: "command3",
296+
CommandUnion: v1.CommandUnion{
297+
Exec: &v1.ExecCommand{},
298+
},
299+
},
282300
},
283-
wantErr: true,
301+
wantErr: &multipleDupError,
284302
},
285303
}
286304
for _, tt := range tests {
@@ -297,9 +315,17 @@ func TestDevfile200_AddCommands(t *testing.T) {
297315

298316
err := d.AddCommands(tt.newCommands)
299317
// Unexpected error
300-
if (err != nil) != tt.wantErr {
318+
if (err != nil) != (tt.wantErr != nil) {
301319
t.Errorf("TestDevfile200_AddCommands() error = %v, wantErr %v", err, tt.wantErr)
320+
} else if tt.wantErr != nil {
321+
assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
322+
} else {
323+
wantCommands := append(tt.currentCommands, tt.newCommands...)
324+
if !reflect.DeepEqual(d.Commands, wantCommands) {
325+
t.Errorf("TestDevfile200_AddCommands() wanted: %v, got: %v, difference at %v", wantCommands, d.Commands, pretty.Compare(wantCommands, d.Commands))
326+
}
302327
}
328+
303329
})
304330
}
305331
}

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

+9-3
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ package v2
33
import (
44
"fmt"
55
"reflect"
6+
"strings"
67

78
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
89
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
@@ -73,17 +74,22 @@ func (d *DevfileV2) GetDevfileVolumeComponents(options common.DevfileOptions) ([
7374
}
7475

7576
// AddComponents adds the slice of Component objects to the devfile's components
76-
// 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
7779
func (d *DevfileV2) AddComponents(components []v1.Component) error {
78-
80+
var errorsList []string
7981
for _, component := range components {
8082
for _, devfileComponent := range d.Components {
8183
if component.Name == devfileComponent.Name {
82-
return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
84+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: component.Name, Field: "component"}).Error())
85+
continue
8386
}
8487
}
8588
d.Components = append(d.Components, component)
8689
}
90+
if len(errorsList) > 0 {
91+
return fmt.Errorf("errors while adding components:\n%s", strings.Join(errorsList, "\n"))
92+
}
8793
return nil
8894
}
8995

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

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

33
import (
4+
"fmt"
5+
"github.com/kylelemons/godebug/pretty"
46
"reflect"
57
"testing"
68

@@ -12,12 +14,13 @@ import (
1214
)
1315

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

1619
tests := []struct {
1720
name string
1821
currentComponents []v1.Component
1922
newComponents []v1.Component
20-
wantErr bool
23+
wantErr *string
2124
}{
2225
{
2326
name: "successfully add the component",
@@ -43,7 +46,7 @@ func TestDevfile200_AddComponent(t *testing.T) {
4346
},
4447
},
4548
},
46-
wantErr: false,
49+
wantErr: nil,
4750
},
4851
{
4952
name: "error out on duplicate component",
@@ -68,8 +71,20 @@ func TestDevfile200_AddComponent(t *testing.T) {
6871
Container: &v1.ContainerComponent{},
6972
},
7073
},
74+
{
75+
Name: "component2",
76+
ComponentUnion: v1.ComponentUnion{
77+
Volume: &v1.VolumeComponent{},
78+
},
79+
},
80+
{
81+
Name: "component3",
82+
ComponentUnion: v1.ComponentUnion{
83+
Volume: &v1.VolumeComponent{},
84+
},
85+
},
7186
},
72-
wantErr: true,
87+
wantErr: &multipleDupError,
7388
},
7489
}
7590
for _, tt := range tests {
@@ -86,8 +101,15 @@ func TestDevfile200_AddComponent(t *testing.T) {
86101

87102
err := d.AddComponents(tt.newComponents)
88103
// Unexpected error
89-
if (err != nil) != tt.wantErr {
104+
if (err != nil) != (tt.wantErr != nil) {
90105
t.Errorf("TestDevfile200_AddComponents() error = %v, wantErr %v", err, tt.wantErr)
106+
} else if tt.wantErr != nil {
107+
assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
108+
} else {
109+
wantComponents := append(tt.currentComponents, tt.newComponents...)
110+
if !reflect.DeepEqual(d.Components, wantComponents) {
111+
t.Errorf("TestDevfile200_AddComponents() wanted: %v, got: %v, difference at %v", wantComponents, d.Components, pretty.Compare(wantComponents, d.Components))
112+
}
91113
}
92114
})
93115
}

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

+20-11
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
package v2
22

33
import (
4+
"fmt"
45
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
56
"github.com/devfile/library/pkg/devfile/parser/data/v2/common"
7+
"strings"
68
)
79

810
// GetEvents returns the Events Object parsed from devfile
@@ -14,41 +16,48 @@ func (d *DevfileV2) GetEvents() v1.Events {
1416
}
1517

1618
// AddEvents adds the Events Object to the devfile's events
17-
// 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
1821
func (d *DevfileV2) AddEvents(events v1.Events) error {
1922

2023
if d.Events == nil {
2124
d.Events = &v1.Events{}
2225
}
23-
26+
var errorsList []string
2427
if len(events.PreStop) > 0 {
2528
if len(d.Events.PreStop) > 0 {
26-
return &common.FieldAlreadyExistError{Field: "pre stop"}
29+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre stop"}).Error())
30+
} else {
31+
d.Events.PreStop = events.PreStop
2732
}
28-
d.Events.PreStop = events.PreStop
2933
}
3034

3135
if len(events.PreStart) > 0 {
3236
if len(d.Events.PreStart) > 0 {
33-
return &common.FieldAlreadyExistError{Field: "pre start"}
37+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre start"}).Error())
38+
} else {
39+
d.Events.PreStart = events.PreStart
3440
}
35-
d.Events.PreStart = events.PreStart
3641
}
3742

3843
if len(events.PostStop) > 0 {
3944
if len(d.Events.PostStop) > 0 {
40-
return &common.FieldAlreadyExistError{Field: "post stop"}
45+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post stop"}).Error())
46+
} else {
47+
d.Events.PostStop = events.PostStop
4148
}
42-
d.Events.PostStop = events.PostStop
4349
}
4450

4551
if len(events.PostStart) > 0 {
4652
if len(d.Events.PostStart) > 0 {
47-
return &common.FieldAlreadyExistError{Field: "post start"}
53+
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post start"}).Error())
54+
} else {
55+
d.Events.PostStart = events.PostStart
4856
}
49-
d.Events.PostStart = events.PostStart
5057
}
51-
58+
if len(errorsList) > 0 {
59+
return fmt.Errorf("errors while adding events:\n%s", strings.Join(errorsList, "\n"))
60+
}
5261
return nil
5362
}
5463

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

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

33
import (
4+
"fmt"
5+
"github.com/kylelemons/godebug/pretty"
6+
"github.com/stretchr/testify/assert"
47
"reflect"
58
"testing"
69

710
v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2"
811
)
912

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

1216
tests := []struct {
1317
name string
1418
currentEvents *v1.Events
1519
newEvents v1.Events
16-
wantErr bool
20+
wantEvents v1.Events
21+
wantErr *string
1722
}{
1823
{
1924
name: "successfully add the events",
@@ -27,7 +32,13 @@ func TestDevfile200_AddEvents(t *testing.T) {
2732
PostStart: []string{"postStart1"},
2833
},
2934
},
30-
wantErr: false,
35+
wantEvents: v1.Events{
36+
DevWorkspaceEvents: v1.DevWorkspaceEvents{
37+
PreStart: []string{"preStart1"},
38+
PostStart: []string{"postStart1"},
39+
},
40+
},
41+
wantErr: nil,
3142
},
3243
{
3344
name: "successfully add the events to empty devfile event",
@@ -37,21 +48,29 @@ func TestDevfile200_AddEvents(t *testing.T) {
3748
PostStart: []string{"postStart1"},
3849
},
3950
},
40-
wantErr: false,
51+
wantEvents: v1.Events{
52+
DevWorkspaceEvents: v1.DevWorkspaceEvents{
53+
PostStart: []string{"postStart1"},
54+
},
55+
},
56+
wantErr: nil,
4157
},
4258
{
4359
name: "event already present",
4460
currentEvents: &v1.Events{
4561
DevWorkspaceEvents: v1.DevWorkspaceEvents{
4662
PreStart: []string{"preStart1"},
63+
PostStop: []string{"postStop1"},
4764
},
4865
},
4966
newEvents: v1.Events{
5067
DevWorkspaceEvents: v1.DevWorkspaceEvents{
5168
PreStart: []string{"preStart2"},
69+
PostStop: []string{"postStop2"},
70+
PreStop: []string{"preStop"},
5271
},
5372
},
54-
wantErr: true,
73+
wantErr: &multipleDupError,
5574
},
5675
}
5776
for _, tt := range tests {
@@ -68,8 +87,14 @@ func TestDevfile200_AddEvents(t *testing.T) {
6887

6988
err := d.AddEvents(tt.newEvents)
7089

71-
if (err != nil) != tt.wantErr {
72-
t.Errorf("DeleteCommand() error = %v, wantErr %v", err, tt.wantErr)
90+
if (err != nil) != (tt.wantErr != nil) {
91+
t.Errorf("TestDevfile200_AddEvents() error = %v, wantErr %v", err, tt.wantErr)
92+
} else if tt.wantErr != nil {
93+
assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match")
94+
} else {
95+
if !reflect.DeepEqual(*d.Events, tt.wantEvents) {
96+
t.Errorf("TestDevfile200_AddEvents() wanted: %v, got: %v, difference at %v", tt.wantEvents, *d.Events, pretty.Compare(tt.wantEvents, *d.Events))
97+
}
7398
}
7499

75100
})

0 commit comments

Comments
 (0)