diff --git a/pkg/devfile/parser/data/v2/commands.go b/pkg/devfile/parser/data/v2/commands.go index 618be2aa..0bb29a8a 100644 --- a/pkg/devfile/parser/data/v2/commands.go +++ b/pkg/devfile/parser/data/v2/commands.go @@ -5,6 +5,7 @@ import ( v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser/data/v2/common" "reflect" + "strings" ) // 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 } // AddCommands adds the slice of Command objects to the Devfile's commands -// if a command is already defined, error out +// a command is considered as invalid if it is already defined +// command list passed in will be all processed, and returns a total error of all invalid commands func (d *DevfileV2) AddCommands(commands []v1.Command) error { - + var errorsList []string for _, command := range commands { for _, devfileCommand := range d.Commands { if command.Id == devfileCommand.Id { - return &common.FieldAlreadyExistError{Name: command.Id, Field: "command"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: command.Id, Field: "command"}).Error()) + continue } } d.Commands = append(d.Commands, command) } + if len(errorsList) > 0 { + return fmt.Errorf("errors while adding commands:\n%s", strings.Join(errorsList, "\n")) + } return nil } diff --git a/pkg/devfile/parser/data/v2/commands_test.go b/pkg/devfile/parser/data/v2/commands_test.go index 24ec24bc..d0bf9002 100644 --- a/pkg/devfile/parser/data/v2/commands_test.go +++ b/pkg/devfile/parser/data/v2/commands_test.go @@ -1,6 +1,8 @@ package v2 import ( + "fmt" + "github.com/kylelemons/godebug/pretty" "reflect" "testing" @@ -226,15 +228,13 @@ func TestDevfile200_GetCommands(t *testing.T) { } func TestDevfile200_AddCommands(t *testing.T) { + multipleDupError := fmt.Sprintf("%s\n%s", "command command1 already exists in devfile", "command command2 already exists in devfile") - type args struct { - name string - } tests := []struct { name string currentCommands []v1.Command newCommands []v1.Command - wantErr bool + wantErr *string }{ { name: "Command does not exist", @@ -260,10 +260,10 @@ func TestDevfile200_AddCommands(t *testing.T) { }, }, }, - wantErr: false, + wantErr: nil, }, { - name: "Command does exist", + name: "Multiple duplicate commands", currentCommands: []v1.Command{ { Id: "command1", @@ -271,6 +271,12 @@ func TestDevfile200_AddCommands(t *testing.T) { Exec: &v1.ExecCommand{}, }, }, + { + Id: "command2", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{}, + }, + }, }, newCommands: []v1.Command{ { @@ -279,8 +285,20 @@ func TestDevfile200_AddCommands(t *testing.T) { Exec: &v1.ExecCommand{}, }, }, + { + Id: "command2", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{}, + }, + }, + { + Id: "command3", + CommandUnion: v1.CommandUnion{ + Exec: &v1.ExecCommand{}, + }, + }, }, - wantErr: true, + wantErr: &multipleDupError, }, } for _, tt := range tests { @@ -297,9 +315,17 @@ func TestDevfile200_AddCommands(t *testing.T) { err := d.AddCommands(tt.newCommands) // Unexpected error - if (err != nil) != tt.wantErr { + if (err != nil) != (tt.wantErr != nil) { t.Errorf("TestDevfile200_AddCommands() error = %v, wantErr %v", err, tt.wantErr) + } else if tt.wantErr != nil { + assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match") + } else { + wantCommands := append(tt.currentCommands, tt.newCommands...) + if !reflect.DeepEqual(d.Commands, wantCommands) { + t.Errorf("TestDevfile200_AddCommands() wanted: %v, got: %v, difference at %v", wantCommands, d.Commands, pretty.Compare(wantCommands, d.Commands)) + } } + }) } } diff --git a/pkg/devfile/parser/data/v2/components.go b/pkg/devfile/parser/data/v2/components.go index 98c4badc..acd62ec1 100644 --- a/pkg/devfile/parser/data/v2/components.go +++ b/pkg/devfile/parser/data/v2/components.go @@ -3,6 +3,7 @@ package v2 import ( "fmt" "reflect" + "strings" v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser/data/v2/common" @@ -73,17 +74,22 @@ func (d *DevfileV2) GetDevfileVolumeComponents(options common.DevfileOptions) ([ } // AddComponents adds the slice of Component objects to the devfile's components -// if a component is already defined, error out +// a component is considered as invalid if it is already defined +// component list passed in will be all processed, and returns a total error of all invalid components func (d *DevfileV2) AddComponents(components []v1.Component) error { - + var errorsList []string for _, component := range components { for _, devfileComponent := range d.Components { if component.Name == devfileComponent.Name { - return &common.FieldAlreadyExistError{Name: component.Name, Field: "component"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: component.Name, Field: "component"}).Error()) + continue } } d.Components = append(d.Components, component) } + if len(errorsList) > 0 { + return fmt.Errorf("errors while adding components:\n%s", strings.Join(errorsList, "\n")) + } return nil } diff --git a/pkg/devfile/parser/data/v2/components_test.go b/pkg/devfile/parser/data/v2/components_test.go index 0d82a902..001f680e 100644 --- a/pkg/devfile/parser/data/v2/components_test.go +++ b/pkg/devfile/parser/data/v2/components_test.go @@ -1,6 +1,8 @@ package v2 import ( + "fmt" + "github.com/kylelemons/godebug/pretty" "reflect" "testing" @@ -12,12 +14,13 @@ import ( ) func TestDevfile200_AddComponent(t *testing.T) { + multipleDupError := fmt.Sprintf("%s\n%s", "component component1 already exists in devfile", "component component2 already exists in devfile") tests := []struct { name string currentComponents []v1.Component newComponents []v1.Component - wantErr bool + wantErr *string }{ { name: "successfully add the component", @@ -43,7 +46,7 @@ func TestDevfile200_AddComponent(t *testing.T) { }, }, }, - wantErr: false, + wantErr: nil, }, { name: "error out on duplicate component", @@ -68,8 +71,20 @@ func TestDevfile200_AddComponent(t *testing.T) { Container: &v1.ContainerComponent{}, }, }, + { + Name: "component2", + ComponentUnion: v1.ComponentUnion{ + Volume: &v1.VolumeComponent{}, + }, + }, + { + Name: "component3", + ComponentUnion: v1.ComponentUnion{ + Volume: &v1.VolumeComponent{}, + }, + }, }, - wantErr: true, + wantErr: &multipleDupError, }, } for _, tt := range tests { @@ -86,8 +101,15 @@ func TestDevfile200_AddComponent(t *testing.T) { err := d.AddComponents(tt.newComponents) // Unexpected error - if (err != nil) != tt.wantErr { + if (err != nil) != (tt.wantErr != nil) { t.Errorf("TestDevfile200_AddComponents() error = %v, wantErr %v", err, tt.wantErr) + } else if tt.wantErr != nil { + assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match") + } else { + wantComponents := append(tt.currentComponents, tt.newComponents...) + if !reflect.DeepEqual(d.Components, wantComponents) { + t.Errorf("TestDevfile200_AddComponents() wanted: %v, got: %v, difference at %v", wantComponents, d.Components, pretty.Compare(wantComponents, d.Components)) + } } }) } diff --git a/pkg/devfile/parser/data/v2/events.go b/pkg/devfile/parser/data/v2/events.go index eb3e3864..15c88b00 100644 --- a/pkg/devfile/parser/data/v2/events.go +++ b/pkg/devfile/parser/data/v2/events.go @@ -1,8 +1,10 @@ package v2 import ( + "fmt" v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser/data/v2/common" + "strings" ) // GetEvents returns the Events Object parsed from devfile @@ -14,41 +16,48 @@ func (d *DevfileV2) GetEvents() v1.Events { } // AddEvents adds the Events Object to the devfile's events -// if the event is already defined in the devfile, error out +// an event field is considered as invalid if it is already defined +// all event fields will be checked and processed, and returns a total error of all event fields func (d *DevfileV2) AddEvents(events v1.Events) error { if d.Events == nil { d.Events = &v1.Events{} } - + var errorsList []string if len(events.PreStop) > 0 { if len(d.Events.PreStop) > 0 { - return &common.FieldAlreadyExistError{Field: "pre stop"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre stop"}).Error()) + } else { + d.Events.PreStop = events.PreStop } - d.Events.PreStop = events.PreStop } if len(events.PreStart) > 0 { if len(d.Events.PreStart) > 0 { - return &common.FieldAlreadyExistError{Field: "pre start"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "pre start"}).Error()) + } else { + d.Events.PreStart = events.PreStart } - d.Events.PreStart = events.PreStart } if len(events.PostStop) > 0 { if len(d.Events.PostStop) > 0 { - return &common.FieldAlreadyExistError{Field: "post stop"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post stop"}).Error()) + } else { + d.Events.PostStop = events.PostStop } - d.Events.PostStop = events.PostStop } if len(events.PostStart) > 0 { if len(d.Events.PostStart) > 0 { - return &common.FieldAlreadyExistError{Field: "post start"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Field: "event field", Name: "post start"}).Error()) + } else { + d.Events.PostStart = events.PostStart } - d.Events.PostStart = events.PostStart } - + if len(errorsList) > 0 { + return fmt.Errorf("errors while adding events:\n%s", strings.Join(errorsList, "\n")) + } return nil } diff --git a/pkg/devfile/parser/data/v2/events_test.go b/pkg/devfile/parser/data/v2/events_test.go index ef0bf16d..58b0ed3f 100644 --- a/pkg/devfile/parser/data/v2/events_test.go +++ b/pkg/devfile/parser/data/v2/events_test.go @@ -1,6 +1,9 @@ package v2 import ( + "fmt" + "github.com/kylelemons/godebug/pretty" + "github.com/stretchr/testify/assert" "reflect" "testing" @@ -8,12 +11,14 @@ import ( ) func TestDevfile200_AddEvents(t *testing.T) { + multipleDupError := fmt.Sprintf("%s\n%s", "event field pre start already exists in devfile", "event field post stop already exists in devfile") tests := []struct { name string currentEvents *v1.Events newEvents v1.Events - wantErr bool + wantEvents v1.Events + wantErr *string }{ { name: "successfully add the events", @@ -27,7 +32,13 @@ func TestDevfile200_AddEvents(t *testing.T) { PostStart: []string{"postStart1"}, }, }, - wantErr: false, + wantEvents: v1.Events{ + DevWorkspaceEvents: v1.DevWorkspaceEvents{ + PreStart: []string{"preStart1"}, + PostStart: []string{"postStart1"}, + }, + }, + wantErr: nil, }, { name: "successfully add the events to empty devfile event", @@ -37,21 +48,29 @@ func TestDevfile200_AddEvents(t *testing.T) { PostStart: []string{"postStart1"}, }, }, - wantErr: false, + wantEvents: v1.Events{ + DevWorkspaceEvents: v1.DevWorkspaceEvents{ + PostStart: []string{"postStart1"}, + }, + }, + wantErr: nil, }, { name: "event already present", currentEvents: &v1.Events{ DevWorkspaceEvents: v1.DevWorkspaceEvents{ PreStart: []string{"preStart1"}, + PostStop: []string{"postStop1"}, }, }, newEvents: v1.Events{ DevWorkspaceEvents: v1.DevWorkspaceEvents{ PreStart: []string{"preStart2"}, + PostStop: []string{"postStop2"}, + PreStop: []string{"preStop"}, }, }, - wantErr: true, + wantErr: &multipleDupError, }, } for _, tt := range tests { @@ -68,8 +87,14 @@ func TestDevfile200_AddEvents(t *testing.T) { err := d.AddEvents(tt.newEvents) - if (err != nil) != tt.wantErr { - t.Errorf("DeleteCommand() error = %v, wantErr %v", err, tt.wantErr) + if (err != nil) != (tt.wantErr != nil) { + t.Errorf("TestDevfile200_AddEvents() error = %v, wantErr %v", err, tt.wantErr) + } else if tt.wantErr != nil { + assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match") + } else { + if !reflect.DeepEqual(*d.Events, tt.wantEvents) { + t.Errorf("TestDevfile200_AddEvents() wanted: %v, got: %v, difference at %v", tt.wantEvents, *d.Events, pretty.Compare(tt.wantEvents, *d.Events)) + } } }) diff --git a/pkg/devfile/parser/data/v2/projects.go b/pkg/devfile/parser/data/v2/projects.go index 1499a551..c90fb2bf 100644 --- a/pkg/devfile/parser/data/v2/projects.go +++ b/pkg/devfile/parser/data/v2/projects.go @@ -5,6 +5,7 @@ import ( v1 "github.com/devfile/api/v2/pkg/apis/workspaces/v1alpha2" "github.com/devfile/library/pkg/devfile/parser/data/v2/common" "reflect" + "strings" ) // GetProjects returns the Project Object parsed from devfile @@ -40,9 +41,11 @@ func (d *DevfileV2) GetProjects(options common.DevfileOptions) ([]v1.Project, er } // AddProjects adss the slice of Devfile projects to the Devfile's project list -// if a project is already defined, error out +// a project is considered as invalid if it is already defined +// project list passed in will be all processed, and returns a total error of all invalid projects func (d *DevfileV2) AddProjects(projects []v1.Project) error { projectsMap := make(map[string]bool) + var errorsList []string for _, project := range d.Projects { projectsMap[project.Name] = true } @@ -51,9 +54,13 @@ func (d *DevfileV2) AddProjects(projects []v1.Project) error { if _, ok := projectsMap[project.Name]; !ok { d.Projects = append(d.Projects, project) } else { - return &common.FieldAlreadyExistError{Name: project.Name, Field: "project"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: project.Name, Field: "project"}).Error()) + continue } } + if len(errorsList) > 0 { + return fmt.Errorf("errors while adding projects:\n%s", strings.Join(errorsList, "\n")) + } return nil } @@ -118,9 +125,11 @@ func (d *DevfileV2) GetStarterProjects(options common.DevfileOptions) ([]v1.Star } // AddStarterProjects adds the slice of Devfile starter projects to the Devfile's starter project list -// if a starter project is already defined, error out +// a starterProject is considered as invalid if it is already defined +// starterProject list passed in will be all processed, and returns a total error of all invalid starterProjects func (d *DevfileV2) AddStarterProjects(projects []v1.StarterProject) error { projectsMap := make(map[string]bool) + var errorsList []string for _, project := range d.StarterProjects { projectsMap[project.Name] = true } @@ -129,9 +138,13 @@ func (d *DevfileV2) AddStarterProjects(projects []v1.StarterProject) error { if _, ok := projectsMap[project.Name]; !ok { d.StarterProjects = append(d.StarterProjects, project) } else { - return &common.FieldAlreadyExistError{Name: project.Name, Field: "starterProject"} + errorsList = append(errorsList, (&common.FieldAlreadyExistError{Name: project.Name, Field: "starterProject"}).Error()) + continue } } + if len(errorsList) > 0 { + return fmt.Errorf("errors while adding starterProjects:\n%s", strings.Join(errorsList, "\n")) + } return nil } diff --git a/pkg/devfile/parser/data/v2/projects_test.go b/pkg/devfile/parser/data/v2/projects_test.go index cc8394be..2f85c95a 100644 --- a/pkg/devfile/parser/data/v2/projects_test.go +++ b/pkg/devfile/parser/data/v2/projects_test.go @@ -1,6 +1,7 @@ package v2 import ( + "fmt" "reflect" "testing" @@ -191,14 +192,15 @@ func TestDevfile200_AddProjects(t *testing.T) { }, } + multipleDupError := fmt.Sprintf("%s\n%s", "project java-starter already exists in devfile", "project quarkus-starter already exists in devfile") + tests := []struct { name string - wantErr bool args []v1.Project + wantErr *string }{ { - name: "It should add project", - wantErr: false, + name: "It should add project", args: []v1.Project{ { Name: "nodejs", @@ -207,24 +209,30 @@ func TestDevfile200_AddProjects(t *testing.T) { Name: "spring-pet-clinic", }, }, + wantErr: nil, }, { - name: "It should give error if tried to add already present starter project", - wantErr: true, + name: "It should give error if tried to add already present starter project", args: []v1.Project{ + { + Name: "java-starter", + }, { Name: "quarkus-starter", }, }, + wantErr: &multipleDupError, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := d.AddProjects(tt.args) - if (err != nil) != tt.wantErr { + if (err != nil) != (tt.wantErr != nil) { t.Errorf("TestDevfile200_AddProjects() error = %v, wantErr %v", err, tt.wantErr) + } else if tt.wantErr != nil { + assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match") } else if err == nil { wantProjects := append(currentProject, tt.args...) @@ -566,15 +574,15 @@ func TestDevfile200_AddStarterProjects(t *testing.T) { }, }, } + multipleDupError := fmt.Sprintf("%s\n%s", "starterProject java-starter already exists in devfile", "starterProject quarkus-starter already exists in devfile") tests := []struct { name string - wantErr bool args []v1.StarterProject + wantErr *string }{ { - name: "It should add starter project", - wantErr: false, + name: "It should add starter project", args: []v1.StarterProject{ { Name: "nodejs", @@ -585,25 +593,32 @@ func TestDevfile200_AddStarterProjects(t *testing.T) { Description: "starter project for springboot", }, }, + wantErr: nil, }, { - name: "It should give error if tried to add already present starter project", - wantErr: true, + name: "It should give error if tried to add already present starter project", args: []v1.StarterProject{ + { + Name: "java-starter", + Description: "starter project for java", + }, { Name: "quarkus-starter", Description: "starter project for quarkus", }, }, + wantErr: &multipleDupError, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { err := d.AddStarterProjects(tt.args) - if (err != nil) != tt.wantErr { + if (err != nil) != (tt.wantErr != nil) { t.Errorf("TestDevfile200_AddStarterProjects() error = %v, wantErr %v", err, tt.wantErr) + } else if tt.wantErr != nil { + assert.Regexp(t, *tt.wantErr, err.Error(), "Error message should match") } else if err == nil { wantProjects := append(currentProject, tt.args...)