Skip to content

Commit

Permalink
PR Feedback for command group
Browse files Browse the repository at this point in the history
Signed-off-by: Maysun J Faisal <maysun.j.faisal@ibm.com>
  • Loading branch information
maysunfaisal committed Jun 16, 2020
1 parent f2b6223 commit 2653ae1
Show file tree
Hide file tree
Showing 5 changed files with 15 additions and 11 deletions.
14 changes: 7 additions & 7 deletions pkg/devfile/adapters/common/command.go
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import (
func getCommand(data data.DevfileData, commandName string, groupType common.DevfileCommandGroupType) (supportedCommand common.DevfileCommand, err error) {

commands := data.GetCommands()
var firstMatchedCommand common.DevfileCommand
var onlyCommand common.DevfileCommand

if commandName == "" {
// validate the command groups before searching for a command match
Expand Down Expand Up @@ -58,17 +58,17 @@ func getCommand(data data.DevfileData, commandName string, groupType common.Devf
if command.Exec.Group.IsDefault {
// We need to scan all the commands to find default command
return command, validateCommand(data, command)
} else if reflect.DeepEqual(firstMatchedCommand, common.DevfileCommand{}) {
} else if reflect.DeepEqual(onlyCommand, common.DevfileCommand{}) {
// store the first matched command in case there is no default command
firstMatchedCommand = command
onlyCommand = command
}
}
}

if commandName == "" {
// if default command is not found return the first command found for the matching type.
if !reflect.DeepEqual(firstMatchedCommand, common.DevfileCommand{}) {
return firstMatchedCommand, validateCommand(data, firstMatchedCommand)
if !reflect.DeepEqual(onlyCommand, common.DevfileCommand{}) {
return onlyCommand, validateCommand(data, onlyCommand)
}
}

Expand All @@ -93,7 +93,7 @@ func getCommand(data data.DevfileData, commandName string, groupType common.Devf
// 2. multiple commands belonging to a single group cannot have more than one default
func validateCommandsForGroup(data data.DevfileData, groupType common.DevfileCommandGroupType) error {

commands := getCommandsForGroup(data, groupType)
commands := getCommandsByGroup(data, groupType)

defaultCommandCount := 0

Expand All @@ -109,7 +109,7 @@ func validateCommandsForGroup(data data.DevfileData, groupType common.DevfileCom
}

if defaultCommandCount != 1 {
return fmt.Errorf("there should be one default command for command group %v", groupType)
return fmt.Errorf("there should be at most one default command for command group %v", groupType)
}

return nil
Expand Down
3 changes: 2 additions & 1 deletion pkg/devfile/adapters/common/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,8 @@ func GetSupportedComponents(data data.DevfileData) []common.DevfileComponent {
return components
}

func getCommandsForGroup(data data.DevfileData, groupType common.DevfileCommandGroupType) []common.DevfileCommand {
// getCommandsByGroup gets commands by the group kind
func getCommandsByGroup(data data.DevfileData, groupType common.DevfileCommandGroupType) []common.DevfileCommand {
var commands []common.DevfileCommand

for _, command := range data.GetCommands() {
Expand Down
2 changes: 1 addition & 1 deletion pkg/devfile/adapters/common/utils_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -297,7 +297,7 @@ func TestGetCommandsForGroup(t *testing.T) {
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
commands := getCommandsForGroup(devObj.Data, tt.groupType)
commands := getCommandsByGroup(devObj.Data, tt.groupType)

if len(commands) != tt.numberOfCommands {
t.Errorf("TestGetCommandsForGroup error: number of commands mismatch for group %v, expected: %v got: %v", string(tt.groupType), tt.numberOfCommands, len(commands))
Expand Down
3 changes: 3 additions & 0 deletions pkg/devfile/parser/data/2.0.0/components.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ func (d *Devfile200) GetCommands() []common.DevfileCommand {
var commands []common.DevfileCommand

for _, command := range d.Commands {
// we convert devfile command id to lowercase so that we can handle
// cases efficiently without being error prone
// we also convert the odo push commands from build-command and run-command flags
command.Exec.Id = strings.ToLower(command.Exec.Id)
commands = append(commands, command)
}
Expand Down
4 changes: 2 additions & 2 deletions tests/integration/devfile/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -123,8 +123,8 @@ func ExecWithMultipleDefaults(projectDirPath, cmpName, namespace string) {
args = useProjectIfAvailable(args, namespace)
output := helper.CmdShouldFail("odo", args...)
helper.MatchAllInOutput(output, []string{
"there should be one default command for command group build",
"there should be one default command for command group run",
"there should be at most one default command for command group build",
"there should be at most one default command for command group run",
})
}

Expand Down

0 comments on commit 2653ae1

Please sign in to comment.