Skip to content

Commit 06d6b01

Browse files
yangcao77yangcao77
committed
address review comments
Signed-off-by: Stephanie <yangcao@redhat.com>
1 parent 1adb1d5 commit 06d6b01

14 files changed

+140
-262
lines changed

‎pkg/devfile/generator/generators_test.go‎

Lines changed: 14 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -201,26 +201,20 @@ func TestGetContainers(t *testing.T) {
201201
// Unexpected error
202202
if (err != nil) != tt.wantErr {
203203
t.Errorf("TestGetContainers() error = %v, wantErr %v", err, tt.wantErr)
204-
return
205-
}
206-
207-
// Expected error and got an err
208-
if tt.wantErr && err != nil {
209-
return
210-
}
211-
212-
for _, container := range containers {
213-
if container.Name != tt.wantContainerName {
214-
t.Errorf("TestGetContainers error: Name mismatch - got: %s, wanted: %s", container.Name, tt.wantContainerName)
215-
}
216-
if container.Image != tt.wantContainerImage {
217-
t.Errorf("TestGetContainers error: Image mismatch - got: %s, wanted: %s", container.Image, tt.wantContainerImage)
218-
}
219-
if len(container.Env) > 0 && !reflect.DeepEqual(container.Env, tt.wantContainerEnv) {
220-
t.Errorf("TestGetContainers error: Env mismatch - got: %+v, wanted: %+v", container.Env, tt.wantContainerEnv)
221-
}
222-
if len(container.VolumeMounts) > 0 && !reflect.DeepEqual(container.VolumeMounts, tt.wantContainerVolMount) {
223-
t.Errorf("TestGetContainers error: Vol Mount mismatch - got: %+v, wanted: %+v", container.VolumeMounts, tt.wantContainerVolMount)
204+
} else if err == nil {
205+
for _, container := range containers {
206+
if container.Name != tt.wantContainerName {
207+
t.Errorf("TestGetContainers error: Name mismatch - got: %s, wanted: %s", container.Name, tt.wantContainerName)
208+
}
209+
if container.Image != tt.wantContainerImage {
210+
t.Errorf("TestGetContainers error: Image mismatch - got: %s, wanted: %s", container.Image, tt.wantContainerImage)
211+
}
212+
if len(container.Env) > 0 && !reflect.DeepEqual(container.Env, tt.wantContainerEnv) {
213+
t.Errorf("TestGetContainers error: Env mismatch - got: %+v, wanted: %+v", container.Env, tt.wantContainerEnv)
214+
}
215+
if len(container.VolumeMounts) > 0 && !reflect.DeepEqual(container.VolumeMounts, tt.wantContainerVolMount) {
216+
t.Errorf("TestGetContainers error: Vol Mount mismatch - got: %+v, wanted: %+v", container.VolumeMounts, tt.wantContainerVolMount)
217+
}
224218
}
225219
}
226220
})

‎pkg/devfile/generator/utils_test.go‎

Lines changed: 32 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -402,11 +402,11 @@ func TestAddSyncFolder(t *testing.T) {
402402

403403
if !tt.wantErr == (err != nil) {
404404
t.Errorf("expected %v, actual %v", tt.wantErr, err)
405-
}
406-
407-
for _, env := range container.Env {
408-
if env.Name == EnvProjectsSrc && env.Value != tt.want {
409-
t.Errorf("expected %s, actual %s", tt.want, env.Value)
405+
} else if err == nil {
406+
for _, env := range container.Env {
407+
if env.Name == EnvProjectsSrc && env.Value != tt.want {
408+
t.Errorf("expected %s, actual %s", tt.want, env.Value)
409+
}
410410
}
411411
}
412412
})
@@ -764,26 +764,20 @@ func TestGetServiceSpec(t *testing.T) {
764764
// Unexpected error
765765
if (err != nil) != tt.wantErr {
766766
t.Errorf("TestGetServiceSpec() error = %v, wantErr %v", err, tt.wantErr)
767-
return
768-
}
769-
770-
// Expected error and got an err
771-
if tt.wantErr && err != nil {
772-
return
773-
}
774-
775-
if !reflect.DeepEqual(serviceSpec.Selector, tt.labels) {
776-
t.Errorf("expected service selector is %v, actual %v", tt.labels, serviceSpec.Selector)
777-
}
778-
if len(serviceSpec.Ports) != len(tt.wantPorts) {
779-
t.Errorf("expected service ports length is %v, actual %v", len(tt.wantPorts), len(serviceSpec.Ports))
780-
} else {
781-
for i := range serviceSpec.Ports {
782-
if serviceSpec.Ports[i].Name != tt.wantPorts[i].Name {
783-
t.Errorf("expected name %s, actual name %s", tt.wantPorts[i].Name, serviceSpec.Ports[i].Name)
784-
}
785-
if serviceSpec.Ports[i].Port != tt.wantPorts[i].Port {
786-
t.Errorf("expected port number is %v, actual %v", tt.wantPorts[i].Port, serviceSpec.Ports[i].Port)
767+
} else if err == nil {
768+
if !reflect.DeepEqual(serviceSpec.Selector, tt.labels) {
769+
t.Errorf("expected service selector is %v, actual %v", tt.labels, serviceSpec.Selector)
770+
}
771+
if len(serviceSpec.Ports) != len(tt.wantPorts) {
772+
t.Errorf("expected service ports length is %v, actual %v", len(tt.wantPorts), len(serviceSpec.Ports))
773+
} else {
774+
for i := range serviceSpec.Ports {
775+
if serviceSpec.Ports[i].Name != tt.wantPorts[i].Name {
776+
t.Errorf("expected name %s, actual name %s", tt.wantPorts[i].Name, serviceSpec.Ports[i].Name)
777+
}
778+
if serviceSpec.Ports[i].Port != tt.wantPorts[i].Port {
779+
t.Errorf("expected port number is %v, actual %v", tt.wantPorts[i].Port, serviceSpec.Ports[i].Port)
780+
}
787781
}
788782
}
789783
}
@@ -1095,11 +1089,10 @@ func TestGetPortExposure(t *testing.T) {
10951089
}
10961090

10971091
mapCreated, err := getPortExposure(devObj, tt.filterOptions)
1098-
if !tt.wantErr && err != nil {
1099-
t.Errorf("TestGetPortExposure unexpected error: %v", err)
1100-
} else if tt.wantErr && err == nil {
1101-
t.Errorf("TestGetPortExposure expected error but got nil")
1102-
} else if !reflect.DeepEqual(mapCreated, tt.wantMap) {
1092+
// Checks for unexpected error cases
1093+
if !tt.wantErr == (err != nil) {
1094+
t.Errorf("TestGetPortExposure unexpected error %v, wantErr %v", err, tt.wantErr)
1095+
} else if err == nil && !reflect.DeepEqual(mapCreated, tt.wantMap) {
11031096
t.Errorf("TestGetPortExposure Expected: %v, got %v", tt.wantMap, mapCreated)
11041097
}
11051098

@@ -1233,16 +1226,16 @@ func TestGetPVCSpec(t *testing.T) {
12331226
// Checks for unexpected error cases
12341227
if !tt.wantErr == (err != nil) {
12351228
t.Errorf("resource.ParseQuantity unexpected error %v, wantErr %v", err, tt.wantErr)
1236-
}
1237-
1238-
pvcSpec := getPVCSpec(quantity)
1239-
if pvcSpec.AccessModes[0] != corev1.ReadWriteOnce {
1240-
t.Errorf("AccessMode Error: expected %s, actual %s", corev1.ReadWriteMany, pvcSpec.AccessModes[0])
1241-
}
1229+
} else if err == nil {
1230+
pvcSpec := getPVCSpec(quantity)
1231+
if pvcSpec.AccessModes[0] != corev1.ReadWriteOnce {
1232+
t.Errorf("AccessMode Error: expected %s, actual %s", corev1.ReadWriteMany, pvcSpec.AccessModes[0])
1233+
}
12421234

1243-
pvcSpecQuantity := pvcSpec.Resources.Requests["storage"]
1244-
if pvcSpecQuantity.String() != quantity.String() {
1245-
t.Errorf("pvcSpec.Resources.Requests Error: expected %v, actual %v", pvcSpecQuantity.String(), quantity.String())
1235+
pvcSpecQuantity := pvcSpec.Resources.Requests["storage"]
1236+
if pvcSpecQuantity.String() != quantity.String() {
1237+
t.Errorf("pvcSpec.Resources.Requests Error: expected %v, actual %v", pvcSpecQuantity.String(), quantity.String())
1238+
}
12461239
}
12471240
})
12481241
}

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

Lines changed: 2 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -66,19 +66,13 @@ func (d *DevfileV2) AddCommands(commands []v1.Command) error {
6666
// UpdateCommand updates the command with the given id
6767
// return an error if the command is not found
6868
func (d *DevfileV2) UpdateCommand(command v1.Command) error {
69-
found := false
7069
for i := range d.Commands {
7170
if d.Commands[i].Id == command.Id {
72-
found = true
7371
d.Commands[i] = command
74-
d.Commands[i].Id = d.Commands[i].Id
75-
break
72+
return nil
7673
}
7774
}
78-
if !found {
79-
return fmt.Errorf("update command failed: command %s not found", command.Id)
80-
}
81-
return nil
75+
return fmt.Errorf("update command failed: command %s not found", command.Id)
8276
}
8377

8478
// DeleteCommand removes the specified command

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

Lines changed: 20 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -295,11 +295,10 @@ func TestDevfile200_AddCommands(t *testing.T) {
295295
},
296296
}
297297

298-
got := d.AddCommands(tt.newCommands)
299-
if !tt.wantErr && got != nil {
300-
t.Errorf("TestDevfile200_AddCommands() unexpected error - %v", got)
301-
} else if tt.wantErr && got == nil {
302-
t.Errorf("TestDevfile200_AddCommands() wanted an error but got nil")
298+
err := d.AddCommands(tt.newCommands)
299+
// Unexpected error
300+
if (err != nil) != tt.wantErr {
301+
t.Errorf("TestDevfile200_AddCommands() error = %v, wantErr %v", err, tt.wantErr)
303302
}
304303
})
305304
}
@@ -383,30 +382,26 @@ func TestDevfile200_UpdateCommands(t *testing.T) {
383382
// Unexpected error
384383
if (err != nil) != tt.wantErr {
385384
t.Errorf("TestDevfile200_UpdateCommands() error = %v, wantErr %v", err, tt.wantErr)
386-
return
387-
}
388-
if err != nil {
389-
return
390-
}
391-
392-
commands, err := d.GetCommands(common.DevfileOptions{})
393-
if err != nil {
394-
t.Errorf("TestDevfile200_UpdateCommands() unxpected error %v", err)
395-
return
396-
}
385+
} else if err == nil {
386+
commands, err := d.GetCommands(common.DevfileOptions{})
387+
if err != nil {
388+
t.Errorf("TestDevfile200_UpdateCommands() unxpected error %v", err)
389+
return
390+
}
397391

398-
matched := false
399-
for _, devfileCommand := range commands {
400-
if tt.newCommand.Id == devfileCommand.Id {
401-
matched = true
402-
if !reflect.DeepEqual(devfileCommand, tt.newCommand) {
403-
t.Errorf("TestDevfile200_UpdateCommands() command mismatch - wanted %+v, got %+v", tt.newCommand, devfileCommand)
392+
matched := false
393+
for _, devfileCommand := range commands {
394+
if tt.newCommand.Id == devfileCommand.Id {
395+
matched = true
396+
if !reflect.DeepEqual(devfileCommand, tt.newCommand) {
397+
t.Errorf("TestDevfile200_UpdateCommands() command mismatch - wanted %+v, got %+v", tt.newCommand, devfileCommand)
398+
}
404399
}
405400
}
406-
}
407401

408-
if !matched {
409-
t.Errorf("TestDevfile200_UpdateCommands() command mismatch - did not find command with id %s", tt.newCommand.Id)
402+
if !matched {
403+
t.Errorf("TestDevfile200_UpdateCommands() command mismatch - did not find command with id %s", tt.newCommand.Id)
404+
}
410405
}
411406
})
412407
}
@@ -483,7 +478,6 @@ func TestDeleteCommands(t *testing.T) {
483478
err := d.DeleteCommand(tt.commandToDelete)
484479
if (err != nil) != tt.wantErr {
485480
t.Errorf("DeleteCommand() error = %v, wantErr %v", err, tt.wantErr)
486-
return
487481
} else if err == nil {
488482
assert.Equal(t, tt.wantCommands, d.Commands, "The two values should be the same.")
489483
}

‎pkg/devfile/parser/data/v2/common/command_helper_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -324,9 +324,7 @@ func TestGetCommandType(t *testing.T) {
324324
// Unexpected error
325325
if (err != nil) != tt.wantErr {
326326
t.Errorf("TestGetCommandType() error = %v, wantErr %v", err, tt.wantErr)
327-
return
328-
}
329-
if got != tt.commandType {
327+
} else if err == nil && got != tt.commandType {
330328
t.Errorf("TestGetCommandType error: command type mismatch, expected: %v got: %v", tt.commandType, got)
331329
}
332330
})

‎pkg/devfile/parser/data/v2/common/component_helper_test.go‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -179,9 +179,7 @@ func TestGetComponentType(t *testing.T) {
179179
// Unexpected error
180180
if (err != nil) != tt.wantErr {
181181
t.Errorf("TestGetComponentType() error = %v, wantErr %v", err, tt.wantErr)
182-
return
183-
}
184-
if got != tt.componentType {
182+
} else if err == nil && got != tt.componentType {
185183
t.Errorf("TestGetComponentType error: component type mismatch, expected: %v got: %v", tt.componentType, got)
186184
}
187185
})

‎pkg/devfile/parser/data/v2/common/options_test.go‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,11 +63,10 @@ func TestFilterDevfileObject(t *testing.T) {
6363
for _, tt := range tests {
6464
t.Run(tt.name, func(t *testing.T) {
6565
filterIn, err := FilterDevfileObject(tt.attributes, tt.options)
66-
if !tt.wantErr && err != nil {
67-
t.Errorf("TestFilterDevfileObject unexpected error - %v", err)
68-
} else if tt.wantErr && err == nil {
69-
t.Errorf("TestFilterDevfileObject wanted error got nil")
70-
} else if filterIn != tt.wantFilter {
66+
// Unexpected error
67+
if (err != nil) != tt.wantErr {
68+
t.Errorf("TestFilterDevfileObject() error = %v, wantErr %v", err, tt.wantErr)
69+
} else if err == nil && filterIn != tt.wantFilter {
7170
t.Errorf("TestFilterDevfileObject error - expected %v got %v", tt.wantFilter, filterIn)
7271
}
7372
})

‎pkg/devfile/parser/data/v2/common/project_helper_test.go‎

Lines changed: 11 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -103,16 +103,16 @@ func TestGitLikeProjectSource_GetDefaultSource(t *testing.T) {
103103
got1, got2, got3, err := GetDefaultSource(tt.gitLikeProjectSource)
104104
if (err != nil) != tt.wantErr {
105105
t.Errorf("GitLikeProjectSource.GetDefaultSource() error = %v, wantErr %v", err, tt.wantErr)
106-
return
107-
}
108-
if got1 != tt.want1 {
109-
t.Errorf("GitLikeProjectSource.GetDefaultSource() got1 = %v, want %v", got1, tt.want1)
110-
}
111-
if got2 != tt.want2 {
112-
t.Errorf("GitLikeProjectSource.GetDefaultSource() got2 = %v, want %v", got2, tt.want2)
113-
}
114-
if got3 != tt.want3 {
115-
t.Errorf("GitLikeProjectSource.GetDefaultSource() got2 = %v, want %v", got3, tt.want3)
106+
} else if err == nil {
107+
if got1 != tt.want1 {
108+
t.Errorf("GitLikeProjectSource.GetDefaultSource() got1 = %v, want %v", got1, tt.want1)
109+
}
110+
if got2 != tt.want2 {
111+
t.Errorf("GitLikeProjectSource.GetDefaultSource() got2 = %v, want %v", got2, tt.want2)
112+
}
113+
if got3 != tt.want3 {
114+
t.Errorf("GitLikeProjectSource.GetDefaultSource() got2 = %v, want %v", got3, tt.want3)
115+
}
116116
}
117117
})
118118
}
@@ -170,9 +170,7 @@ func TestGetProjectSrcType(t *testing.T) {
170170
// Unexpected error
171171
if (err != nil) != tt.wantErr {
172172
t.Errorf("TestGetProjectSrcType() error = %v, wantErr %v", err, tt.wantErr)
173-
return
174-
}
175-
if got != tt.projectSrcType {
173+
} else if err == nil && got != tt.projectSrcType {
176174
t.Errorf("TestGetProjectSrcType error: project src type mismatch, expected: %v got: %v", tt.projectSrcType, got)
177175
}
178176
})

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

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -90,18 +90,13 @@ func (d *DevfileV2) AddComponents(components []v1.Component) error {
9090
// UpdateComponent updates the component with the given name
9191
// return an error if the component is not found
9292
func (d *DevfileV2) UpdateComponent(component v1.Component) error {
93-
found := false
9493
for i := range d.Components {
9594
if d.Components[i].Name == component.Name {
9695
d.Components[i] = component
97-
found = true
98-
break
96+
return nil
9997
}
10098
}
101-
if !found {
102-
return fmt.Errorf("update component failed: component %s not found", component.Name)
103-
}
104-
return nil
99+
return fmt.Errorf("update component failed: component %s not found", component.Name)
105100
}
106101

107102
// DeleteComponent removes the specified component

0 commit comments

Comments
 (0)