Skip to content

Commit 61ec35a

Browse files
yangcao77yangcao77
authored
Merge pull request #116 from yangcao77/622-AddComponentBug
do not add to list if the element exist already
2 parents 202fa98 + b25ba5a commit 61ec35a

5 files changed

Lines changed: 132 additions & 19 deletions

File tree

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,13 +55,17 @@ func (d *DevfileV2) GetCommands(options common.DevfileOptions) ([]v1.Command, er
5555
func (d *DevfileV2) AddCommands(commands []v1.Command) error {
5656
var errorsList []string
5757
for _, command := range commands {
58+
var err error
5859
for _, devfileCommand := range d.Commands {
5960
if command.Id == devfileCommand.Id {
60-
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: command.Id, Field: "command"}).Error())
61-
continue
61+
err = &common.FieldAlreadyExistError{Name: command.Id, Field: "command"}
62+
errorsList = append(errorsList, err.Error())
63+
break
6264
}
6365
}
64-
d.Commands = append(d.Commands, command)
66+
if err == nil {
67+
d.Commands = append(d.Commands, command)
68+
}
6569
}
6670
if len(errorsList) > 0 {
6771
return fmt.Errorf("errors while adding commands:\n%s", strings.Join(errorsList, "\n"))

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

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,7 @@ func TestDevfile200_AddCommands(t *testing.T) {
235235
name string
236236
currentCommands []v1.Command
237237
newCommands []v1.Command
238+
wantCommands []v1.Command
238239
wantErr *string
239240
}{
240241
{
@@ -261,6 +262,26 @@ func TestDevfile200_AddCommands(t *testing.T) {
261262
},
262263
},
263264
},
265+
wantCommands: []v1.Command{
266+
{
267+
Id: "command1",
268+
CommandUnion: v1.CommandUnion{
269+
Exec: &v1.ExecCommand{},
270+
},
271+
},
272+
{
273+
Id: "command2",
274+
CommandUnion: v1.CommandUnion{
275+
Exec: &v1.ExecCommand{},
276+
},
277+
},
278+
{
279+
Id: "command3",
280+
CommandUnion: v1.CommandUnion{
281+
Exec: &v1.ExecCommand{},
282+
},
283+
},
284+
},
264285
wantErr: nil,
265286
},
266287
{
@@ -299,6 +320,26 @@ func TestDevfile200_AddCommands(t *testing.T) {
299320
},
300321
},
301322
},
323+
wantCommands: []v1.Command{
324+
{
325+
Id: "command1",
326+
CommandUnion: v1.CommandUnion{
327+
Exec: &v1.ExecCommand{},
328+
},
329+
},
330+
{
331+
Id: "command2",
332+
CommandUnion: v1.CommandUnion{
333+
Exec: &v1.ExecCommand{},
334+
},
335+
},
336+
{
337+
Id: "command3",
338+
CommandUnion: v1.CommandUnion{
339+
Exec: &v1.ExecCommand{},
340+
},
341+
},
342+
},
302343
wantErr: &multipleDupError,
303344
},
304345
}
@@ -321,9 +362,8 @@ func TestDevfile200_AddCommands(t *testing.T) {
321362
} else if tt.wantErr != nil {
322363
assert.Regexp(t, *tt.wantErr, err.Error(), "TestDevfile200_AddCommands(): Error message should match")
323364
} else {
324-
wantCommands := append(tt.currentCommands, tt.newCommands...)
325-
if !reflect.DeepEqual(d.Commands, wantCommands) {
326-
t.Errorf("TestDevfile200_AddCommands() wanted: %v, got: %v, difference at %v", wantCommands, d.Commands, pretty.Compare(wantCommands, d.Commands))
365+
if !reflect.DeepEqual(d.Commands, tt.wantCommands) {
366+
t.Errorf("TestDevfile200_AddCommands() wanted: %v, got: %v, difference at %v", tt.wantCommands, d.Commands, pretty.Compare(tt.wantCommands, d.Commands))
327367
}
328368
}
329369

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -79,13 +79,17 @@ func (d *DevfileV2) GetDevfileVolumeComponents(options common.DevfileOptions) ([
7979
func (d *DevfileV2) AddComponents(components []v1.Component) error {
8080
var errorsList []string
8181
for _, component := range components {
82+
var err error
8283
for _, devfileComponent := range d.Components {
8384
if component.Name == devfileComponent.Name {
84-
errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: component.Name, Field: "component"}).Error())
85-
continue
85+
err = &common.FieldAlreadyExistError{Name: component.Name, Field: "component"}
86+
errorsList = append(errorsList, err.Error())
87+
break
8688
}
8789
}
88-
d.Components = append(d.Components, component)
90+
if err == nil {
91+
d.Components = append(d.Components, component)
92+
}
8993
}
9094
if len(errorsList) > 0 {
9195
return fmt.Errorf("errors while adding components:\n%s", strings.Join(errorsList, "\n"))

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

Lines changed: 43 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ func TestDevfile200_AddComponent(t *testing.T) {
2020
name string
2121
currentComponents []v1.Component
2222
newComponents []v1.Component
23+
wantComponents []v1.Component
2324
wantErr *string
2425
}{
2526
{
@@ -46,6 +47,26 @@ func TestDevfile200_AddComponent(t *testing.T) {
4647
},
4748
},
4849
},
50+
wantComponents: []v1.Component{
51+
{
52+
Name: "component1",
53+
ComponentUnion: v1.ComponentUnion{
54+
Container: &v1.ContainerComponent{},
55+
},
56+
},
57+
{
58+
Name: "component2",
59+
ComponentUnion: v1.ComponentUnion{
60+
Volume: &v1.VolumeComponent{},
61+
},
62+
},
63+
{
64+
Name: "component3",
65+
ComponentUnion: v1.ComponentUnion{
66+
Container: &v1.ContainerComponent{},
67+
},
68+
},
69+
},
4970
wantErr: nil,
5071
},
5172
{
@@ -84,6 +105,26 @@ func TestDevfile200_AddComponent(t *testing.T) {
84105
},
85106
},
86107
},
108+
wantComponents: []v1.Component{
109+
{
110+
Name: "component1",
111+
ComponentUnion: v1.ComponentUnion{
112+
Container: &v1.ContainerComponent{},
113+
},
114+
},
115+
{
116+
Name: "component2",
117+
ComponentUnion: v1.ComponentUnion{
118+
Volume: &v1.VolumeComponent{},
119+
},
120+
},
121+
{
122+
Name: "component3",
123+
ComponentUnion: v1.ComponentUnion{
124+
Container: &v1.ContainerComponent{},
125+
},
126+
},
127+
},
87128
wantErr: &multipleDupError,
88129
},
89130
}
@@ -106,9 +147,8 @@ func TestDevfile200_AddComponent(t *testing.T) {
106147
} else if tt.wantErr != nil {
107148
assert.Regexp(t, *tt.wantErr, err.Error(), "TestDevfile200_AddComponents(): Error message should match")
108149
} 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))
150+
if !reflect.DeepEqual(d.Components, tt.wantComponents) {
151+
t.Errorf("TestDevfile200_AddComponents() wanted: %v, got: %v, difference at %v", tt.wantComponents, d.Components, pretty.Compare(tt.wantComponents, d.Components))
112152
}
113153
}
114154
})

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

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -195,9 +195,10 @@ func TestDevfile200_AddProjects(t *testing.T) {
195195
multipleDupError := fmt.Sprintf("%s\n%s", "project java-starter already exists in devfile", "project quarkus-starter already exists in devfile")
196196

197197
tests := []struct {
198-
name string
199-
args []v1.Project
200-
wantErr *string
198+
name string
199+
args []v1.Project
200+
wantProjects []v1.Project
201+
wantErr *string
201202
}{
202203
{
203204
name: "It should add project",
@@ -209,6 +210,22 @@ func TestDevfile200_AddProjects(t *testing.T) {
209210
Name: "spring-pet-clinic",
210211
},
211212
},
213+
wantProjects: []v1.Project{
214+
{
215+
Name: "java-starter",
216+
ClonePath: "/project",
217+
},
218+
{
219+
Name: "quarkus-starter",
220+
ClonePath: "/test",
221+
},
222+
{
223+
Name: "nodejs",
224+
},
225+
{
226+
Name: "spring-pet-clinic",
227+
},
228+
},
212229
wantErr: nil,
213230
},
214231

@@ -222,6 +239,16 @@ func TestDevfile200_AddProjects(t *testing.T) {
222239
Name: "quarkus-starter",
223240
},
224241
},
242+
wantProjects: []v1.Project{
243+
{
244+
Name: "java-starter",
245+
ClonePath: "/project",
246+
},
247+
{
248+
Name: "quarkus-starter",
249+
ClonePath: "/test",
250+
},
251+
},
225252
wantErr: &multipleDupError,
226253
},
227254
}
@@ -234,10 +261,8 @@ func TestDevfile200_AddProjects(t *testing.T) {
234261
} else if tt.wantErr != nil {
235262
assert.Regexp(t, *tt.wantErr, err.Error(), "TestDevfile200_AddProjects(): Error message should match")
236263
} else if err == nil {
237-
wantProjects := append(currentProject, tt.args...)
238-
239-
if !reflect.DeepEqual(d.Projects, wantProjects) {
240-
t.Errorf("TestDevfile200_AddProjects() error: wanted: %v, got: %v, difference at %v", wantProjects, d.Projects, pretty.Compare(wantProjects, d.Projects))
264+
if !reflect.DeepEqual(d.Projects, tt.wantProjects) {
265+
t.Errorf("TestDevfile200_AddProjects() error: wanted: %v, got: %v, difference at %v", tt.wantProjects, d.Projects, pretty.Compare(tt.wantProjects, d.Projects))
241266
}
242267
}
243268
})

0 commit comments

Comments
 (0)