Skip to content

Commit

Permalink
added check for conflicting flags (#2515)
Browse files Browse the repository at this point in the history 
* added check for conflicting flags

* removed conflicting flags from tests

* resolve failing test #2

* added comment for checkConflictingFlags

* resolved already incorrectly written tests

* resolve some incorrectly written servicecatalog tests too

* use of frontendContext correctly in test
  • Loading branch information
@girishramnani @openshift-merge-robot
girishramnani authored and openshift-merge-robot committed Jan 20, 2020
1 parent ffd01ac commit 44699f5
Show file tree
Hide file tree
Showing 5 changed files with 67 additions and 18 deletions.
34 changes: 34 additions & 0 deletions pkg/odo/genericclioptions/runnable.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package genericclioptions

import (
"flag"
"fmt"
"os"

"github.com/openshift/odo/pkg/log"
Expand All @@ -22,12 +23,45 @@ func GenericRun(o Runnable, cmd *cobra.Command, args []string) {
// fixes / checks all related machine readable output functions
CheckMachineReadableOutputCommand(cmd)

// LogErrorAndExit is used so that we get -o (jsonoutput) for cmds which have json output implemented
util.LogErrorAndExit(checkConflictingFlags(cmd), "")
// Run completion, validation and run.
util.LogErrorAndExit(o.Complete(cmd.Name(), cmd, args), "")
util.LogErrorAndExit(o.Validate(), "")
util.LogErrorAndExit(o.Run(), "")
}

// checkConflictingFlags checks for conflicting flags. Currently --context cannot be provided
// with either --app, --project and --component as that information can be fetched from the local
// config.
func checkConflictingFlags(cmd *cobra.Command) error {

// we allow providing --context with --app and --project in case of `odo create` or `odo component create`
if cmd.Name() == "create" {
if cmd.HasParent() {
if cmd.Parent().Name() == "odo" || cmd.Parent().Name() == "component" {
return nil
}
}
}
app := stringFlagLookup(cmd, "app")
project := stringFlagLookup(cmd, "project")
context := stringFlagLookup(cmd, "context")
component := stringFlagLookup(cmd, "component")
if (context != "") && (app != "" || project != "" || component != "") {
return fmt.Errorf("cannot provide --app, --project or --component flag when --context is provided")
}
return nil
}
func stringFlagLookup(cmd *cobra.Command, flagName string) string {
flag := cmd.Flags().Lookup(flagName)
// a check to make sure if the flag is not defined we return blank
if flag == nil {
return ""
}
return flag.Value.String()
}

// CheckMachineReadableOutputCommand performs machine-readable output functions required to
// have it work correctly
func CheckMachineReadableOutputCommand(cmd *cobra.Command) {
Expand Down
11 changes: 6 additions & 5 deletions tests/integration/cmd_link_unlink_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ var _ = Describe("odo link and unlink command tests", func() {
helper.CopyExample(filepath.Join("source", "python"), backendContext)
helper.CmdShouldPass("odo", "create", "python", "backend", "--context", backendContext, "--project", project)
helper.CmdShouldPass("odo", "push", "--context", backendContext)
stdErr := helper.CmdShouldFail("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", backendContext, "--port", "1234")
stdErr := helper.CmdShouldFail("odo", "link", "backend", "--context", frontendContext, "--port", "1234")
Expect(stdErr).To(ContainSubstring("Unable to properly link to component backend using port 1234"))
})
})
Expand All @@ -94,7 +94,8 @@ var _ = Describe("odo link and unlink command tests", func() {
helper.CmdShouldPass("odo", "url", "create", "--port", "8080", "--context", backendContext)
helper.CmdShouldPass("odo", "push", "--context", backendContext)

helper.CmdShouldPass("odo", "link", "backend", "--component", "frontend", "--project", project, "--port", "8778", "--context", backendContext)
// we link
helper.CmdShouldPass("odo", "link", "backend", "--context", frontendContext, "--port", "8778")
// ensure that the proper envFrom entry was created
envFromOutput := oc.GetEnvFromEntry("frontend", "app", project)
Expect(envFromOutput).To(ContainSubstring("backend"))
Expand All @@ -104,9 +105,9 @@ var _ = Describe("odo link and unlink command tests", func() {
oc.WaitForDCRollout(dcName, project, 20*time.Second)
helper.HttpWaitFor(frontendURL, "Hello world from node.js!", 20, 1)

outputErr := helper.CmdShouldFail("odo", "link", "backend", "--component", "frontend", "--project", project, "--port", "8778", "--context", backendContext)
outputErr := helper.CmdShouldFail("odo", "link", "backend", "--port", "8778", "--context", frontendContext)
Expect(outputErr).To(ContainSubstring("been linked"))
helper.CmdShouldPass("odo", "unlink", "backend", "--component", "frontend", "--project", project, "--port", "8778", "--context", backendContext)
helper.CmdShouldPass("odo", "unlink", "backend", "--port", "8778", "--context", frontendContext)
})

It("Wait till frontend dc rollout properly after linking the frontend application to the backend", func() {
Expand All @@ -131,7 +132,7 @@ var _ = Describe("odo link and unlink command tests", func() {
envFromOutput := oc.GetEnvFromEntry("frontend", appName, project)
Expect(envFromOutput).To(ContainSubstring("backend"))

helper.CmdShouldPass("odo", "unlink", "backend", "--app", appName, "--port", "8080", "--context", frontendContext)
helper.CmdShouldPass("odo", "unlink", "backend", "--port", "8080", "--context", frontendContext)
})

It("should successfully delete component after linked component is deleted", func() {
Expand Down
19 changes: 16 additions & 3 deletions tests/integration/component.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,19 @@ func componentTests(args ...string) {
Expect(projectList).To(ContainSubstring(project))
})

It("shouldn't error when creating a component with --project and --context at the same time", func() {
helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--git", "https://github.com/openshift/nodejs-ex", "--project", project, "--context", context, "--app", "testing")...)
helper.CmdShouldPass("odo", append(args, "push", "--context", context, "-v4")...)
oc.SwitchProject(project)
projectList := helper.CmdShouldPass("odo", "project", "list")
Expect(projectList).To(ContainSubstring(project))
})

It("should error when listing components (basically anything other then creating) with --project and --context ", func() {
helper.CmdShouldPass("odo", append(args, "create", "nodejs", "cmp-git", "--git", "https://github.com/openshift/nodejs-ex", "--project", project, "--context", context, "--app", "testing")...)
helper.CmdShouldFail("odo", "list", "--project", project, "--context", context)
})

It("Without an application should create one", func() {
componentName := helper.RandString(6)
helper.CmdShouldPass("odo", append(args, "create", "nodejs", "--project", project, componentName, "--ref", "master", "--git", "https://github.com/openshift/nodejs-ex")...)
Expand Down Expand Up @@ -485,8 +498,8 @@ func componentTests(args ...string) {
helper.CmdShouldPass("odo", append(args, "create", "nodejs", componentName, "--app", appName, "--project", project, "--env", "key=value,key1=value1")...)
helper.ValidateLocalCmpExist(context, "Type,nodejs", "Name,"+componentName, "Application,"+appName)
helper.CmdShouldPass("odo", append(args, "push", "--context", context)...)
helper.CmdShouldPass("odo", append(args, "delete", "--context", context, "-f", "--all", "--app", appName)...)
componentList := helper.CmdShouldPass("odo", append(args, "list", "--context", context, "--app", appName, "--project", project)...)
helper.CmdShouldPass("odo", append(args, "delete", "--context", context, "-f", "--all")...)
componentList := helper.CmdShouldPass("odo", append(args, "list", "--app", appName, "--project", project)...)
Expect(componentList).NotTo(ContainSubstring(componentName))
files := helper.ListFilesInDir(context)
Expect(files).NotTo(ContainElement(".odo"))
Expand All @@ -499,7 +512,7 @@ func componentTests(args ...string) {
helper.CmdShouldPass("odo", append(args, "push", "--context", context)...)
helper.CmdShouldPass("odo", append(args, "delete", "--context", context, "-f")...)
helper.CmdShouldPass("odo", append(args, "delete", "--all", "-f", "--context", context)...)
componentList := helper.CmdShouldPass("odo", append(args, "list", "--context", context, "--app", appName, "--project", project)...)
componentList := helper.CmdShouldPass("odo", append(args, "list", "--app", appName, "--project", project)...)
Expect(componentList).NotTo(ContainSubstring(componentName))
files := helper.ListFilesInDir(context)
Expect(files).NotTo(ContainElement(".odo"))
Expand Down
15 changes: 8 additions & 7 deletions tests/integration/servicecatalog/cmd_link_unlink_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ var _ = Describe("odo link and unlink command tests", func() {
helper.CopyExample(filepath.Join("source", "python"), context2)
helper.CmdShouldPass("odo", "create", "python", "backend", "--context", context2, "--project", project)
helper.CmdShouldPass("odo", "push", "--context", context2)
stdErr := helper.CmdShouldFail("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2, "--port", "1234")
stdErr := helper.CmdShouldFail("odo", "link", "backend", "--context", context1, "--port", "1234")
Expect(stdErr).To(ContainSubstring("Unable to properly link to component backend using port 1234"))
})
})
Expand All @@ -94,7 +94,8 @@ var _ = Describe("odo link and unlink command tests", func() {
helper.CmdShouldPass("odo", "url", "create", "--context", context2)
helper.CmdShouldPass("odo", "push", "--context", context2)

helper.CmdShouldPass("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--context", context1)

// ensure that the proper envFrom entry was created
envFromOutput := oc.GetEnvFromEntry("frontend", "app", project)
Expect(envFromOutput).To(ContainSubstring("backend"))
Expand All @@ -104,9 +105,9 @@ var _ = Describe("odo link and unlink command tests", func() {
oc.WaitForDCRollout(dcName, project, 20*time.Second)
helper.HttpWaitFor(frontendURL, "Hello world from node.js!", 20, 1)

outputErr := helper.CmdShouldFail("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2)
outputErr := helper.CmdShouldFail("odo", "link", "backend", "--context", context1)
Expect(outputErr).To(ContainSubstring("been linked"))
helper.CmdShouldPass("odo", "unlink", "backend", "--component", "frontend", "--project", project, "--context", context2)
helper.CmdShouldPass("odo", "unlink", "backend", "--context", context1)
})
})

Expand All @@ -128,7 +129,7 @@ var _ = Describe("odo link and unlink command tests", func() {
helper.CopyExample(filepath.Join("source", "python"), context2)
helper.CmdShouldPass("odo", "create", "python", "backend", "--context", context2, "--project", project)
helper.CmdShouldPass("odo", "push", "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--context", context1) // context1 is the frontend
// Switching to context2 dir because --context flag is not supported with service command
helper.Chdir(context2)
helper.CmdShouldPass("odo", "service", "create", "mysql-persistent")
Expand All @@ -141,7 +142,7 @@ var _ = Describe("odo link and unlink command tests", func() {
// ensure that the proper envFrom entry was created
envFromOutput := oc.GetEnvFromEntry("backend", "app", project)
Expect(envFromOutput).To(ContainSubstring("mysql-persistent"))
outputErr := helper.CmdShouldFail("odo", "link", "mysql-persistent", "--component", "backend", "--project", project, "--context", context2)
outputErr := helper.CmdShouldFail("odo", "link", "mysql-persistent", "--context", context2)
Expect(outputErr).To(ContainSubstring("been linked"))
})
})
Expand All @@ -164,7 +165,7 @@ var _ = Describe("odo link and unlink command tests", func() {
helper.CopyExample(filepath.Join("source", "python"), context2)
helper.CmdShouldPass("odo", "create", "python", "backend", "--context", context2, "--project", project)
helper.CmdShouldPass("odo", "push", "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--context", context1)
helper.Chdir(context2)
helper.CmdShouldPass("odo", "service", "create", "mysql-persistent")

Expand Down
6 changes: 3 additions & 3 deletions tests/integration/servicecatalog/cmd_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -312,7 +312,7 @@ var _ = Describe("odo service command tests", func() {
helper.CopyExample(filepath.Join("source", "python"), context2)
helper.CmdShouldPass("odo", "create", "python", "backend", "--context", context2, "--project", project)
helper.CmdShouldPass("odo", "push", "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--context", context1)
// Switching to context2 dir because --context flag is not supported with service command
helper.Chdir(context2)
helper.CmdShouldPass("odo", "service", "create", "mysql-persistent")
Expand All @@ -325,7 +325,7 @@ var _ = Describe("odo service command tests", func() {
// ensure that the proper envFrom entry was created
envFromOutput := oc.GetEnvFromEntry("backend", "app", project)
Expect(envFromOutput).To(ContainSubstring("mysql-persistent"))
outputErr := helper.CmdShouldFail("odo", "link", "mysql-persistent", "--component", "backend", "--project", project, "--context", context2)
outputErr := helper.CmdShouldFail("odo", "link", "mysql-persistent", "--context", context2)
Expect(outputErr).To(ContainSubstring("been linked"))
})
})
Expand All @@ -344,7 +344,7 @@ var _ = Describe("odo service command tests", func() {
helper.CopyExample(filepath.Join("source", "python"), context2)
helper.CmdShouldPass("odo", "create", "python", "backend", "--context", context2, "--project", project)
helper.CmdShouldPass("odo", "push", "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--component", "frontend", "--project", project, "--context", context2)
helper.CmdShouldPass("odo", "link", "backend", "--context", context1)
helper.Chdir(context2)
helper.CmdShouldPass("odo", "service", "create", "mysql-persistent")

Expand Down

0 comments on commit 44699f5

Please sign in to comment.