Skip to content

Commit

Permalink
Removed string command parameter, it's not used anywhere except for a…
Browse files Browse the repository at this point in the history 
… test (#90)

Signed-off-by: Andreas Neumann <[email protected]>
  • Loading branch information
@ANeumann82
ANeumann82 authored May 14, 2020
1 parent 0771526 commit f487791
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 69 deletions.
2 changes: 1 addition & 1 deletion pkg/test/harness.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ func (h *Harness) Setup() {
h.fatal(fmt.Errorf("fatal error installing manifests: %v", err))
}
}
bgs, errs := testutils.RunCommands(h.GetLogger(), "default", "", h.TestSuite.Commands, "")
bgs, errs := testutils.RunCommands(h.GetLogger(), "default", h.TestSuite.Commands, "")
// assign any background processes first for cleanup in case of any errors
h.bgProcesses = append(h.bgProcesses, bgs...)
if len(errs) > 0 {
Expand Down
2 changes: 1 addition & 1 deletion pkg/test/step.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -379,7 +379,7 @@ func (s *Step) Run(namespace string) []error {
command.Background = false
}
}
if _, errors := testutils.RunCommands(s.Logger, namespace, "", s.Step.Commands, s.Dir); errors != nil {
if _, errors := testutils.RunCommands(s.Logger, namespace, s.Step.Commands, s.Dir); errors != nil {
testErrors = append(testErrors, errors...)
}
}
Expand Down
20 changes: 6 additions & 14 deletions pkg/test/utils/kubernetes.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -910,7 +910,7 @@ func StartTestEnvironment(KubeAPIServerFlags []string) (env TestEnvironment, err
// GetArgs parses a command line string into its arguments and appends a namespace if it is not already set.
// provides OS expansion of defined ENV VARs inside args to commands. The expansion is limited to what is defined on the OS
// and not variables defined for kuttl tests
func GetArgs(ctx context.Context, command string, cmd harness.Command, namespace string, env map[string]string) (*exec.Cmd, error) {
func GetArgs(ctx context.Context, cmd harness.Command, namespace string, env map[string]string) (*exec.Cmd, error) {
argSlice := []string{}

c := os.ExpandEnv(cmd.Command)
Expand All @@ -922,14 +922,6 @@ func GetArgs(ctx context.Context, command string, cmd harness.Command, namespace
return nil, err
}

if command != "" && argSplit[0] != command {
command = os.Expand(command, func(s string) string {
return env[s]
})

argSlice = append(argSlice, os.ExpandEnv(command))
}

argSlice = append(argSlice, argSplit...)

if cmd.Namespaced {
Expand All @@ -955,7 +947,7 @@ func GetArgs(ctx context.Context, command string, cmd harness.Command, namespace
// RunCommand runs a command with args.
// args gets split on spaces (respecting quoted strings).
// if the command is run in the background a reference to the process is returned for later cleanup
func RunCommand(ctx context.Context, namespace string, command string, cmd harness.Command, cwd string, stdout io.Writer, stderr io.Writer) (*exec.Cmd, error) {
func RunCommand(ctx context.Context, namespace string, cmd harness.Command, cwd string, stdout io.Writer, stderr io.Writer) (*exec.Cmd, error) {
actualDir, err := os.Getwd()
if err != nil {
return nil, err
Expand All @@ -966,7 +958,7 @@ func RunCommand(ctx context.Context, namespace string, command string, cmd harne
kudoENV["KUBECONFIG"] = fmt.Sprintf("%s/kubeconfig", actualDir)
kudoENV["PATH"] = fmt.Sprintf("%s/bin/:%s", actualDir, os.Getenv("PATH"))

builtCmd, err := GetArgs(ctx, command, cmd, namespace, kudoENV)
builtCmd, err := GetArgs(ctx, cmd, namespace, kudoENV)
if err != nil {
return nil, err
}
Expand Down Expand Up @@ -1003,7 +995,7 @@ func RunCommand(ctx context.Context, namespace string, command string, cmd harne
// RunCommands runs a set of commands, returning any errors.
// If `command` is set, then `command` will be the command that is invoked (if a command specifies it already, it will not be prepended again).
// commands running in the background are returned
func RunCommands(logger Logger, namespace string, command string, commands []harness.Command, workdir string) ([]*exec.Cmd, []error) {
func RunCommands(logger Logger, namespace string, commands []harness.Command, workdir string) ([]*exec.Cmd, []error) {
errs := []error{}
bgs := []*exec.Cmd{}

Expand All @@ -1012,9 +1004,9 @@ func RunCommands(logger Logger, namespace string, command string, commands []har
}

for _, cmd := range commands {
logger.Logf("running command: %s %q", command, cmd.Command)
logger.Logf("running command: %s", cmd)

bg, err := RunCommand(context.TODO(), namespace, command, cmd, workdir, logger, logger)
bg, err := RunCommand(context.TODO(), namespace, cmd, workdir, logger, logger)
if err != nil {
errs = append(errs, err)
}
Expand Down
10 changes: 5 additions & 5 deletions pkg/test/utils/kubernetes_integration_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ func TestRunCommand(t *testing.T) {
}

// assert foreground cmd returns nil
cmd, err := RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr)
cmd, err := RunCommand(context.TODO(), "", hcmd, "", stdout, stderr)
assert.NoError(t, err)
assert.Nil(t, cmd)
// foreground processes should have stdout
Expand All @@ -130,7 +130,7 @@ func TestRunCommand(t *testing.T) {
stdout = &bytes.Buffer{}

// assert background cmd returns process
cmd, err = RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr)
cmd, err = RunCommand(context.TODO(), "", hcmd, "", stdout, stderr)
assert.NoError(t, err)
assert.NotNil(t, cmd)
// no stdout for background processes
Expand All @@ -146,12 +146,12 @@ func TestRunCommandIgnoreErrors(t *testing.T) {
}

// assert foreground cmd returns nil
cmd, err := RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr)
cmd, err := RunCommand(context.TODO(), "", hcmd, "", stdout, stderr)
assert.NoError(t, err)
assert.Nil(t, cmd)

hcmd.IgnoreFailure = false
cmd, err = RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr)
cmd, err = RunCommand(context.TODO(), "", hcmd, "", stdout, stderr)
assert.Error(t, err)
assert.Nil(t, cmd)

Expand All @@ -160,7 +160,7 @@ func TestRunCommandIgnoreErrors(t *testing.T) {
Command: "bad-command",
IgnoreFailure: true,
}
cmd, err = RunCommand(context.TODO(), "", "", hcmd, "", stdout, stderr)
cmd, err = RunCommand(context.TODO(), "", hcmd, "", stdout, stderr)
assert.Error(t, err)
assert.Nil(t, cmd)
}
64 changes: 16 additions & 48 deletions pkg/test/utils/kubernetes_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -257,151 +257,119 @@ func TestGetKubectlArgs(t *testing.T) {
{
testName: "namespace long, combined already set at end is not modified",
namespace: "default",
args: "kuttl test --namespace=test-canary",
args: "kubectl kuttl test --namespace=test-canary",
expected: []string{
"kubectl", "kuttl", "test", "--namespace=test-canary",
},
},
{
testName: "namespace long already set at end is not modified",
namespace: "default",
args: "kuttl test --namespace test-canary",
args: "kubectl kuttl test --namespace test-canary",
expected: []string{
"kubectl", "kuttl", "test", "--namespace", "test-canary",
},
},
{
testName: "namespace short, combined already set at end is not modified",
namespace: "default",
args: "kuttl test -n=test-canary",
args: "kubectl kuttl test -n=test-canary",
expected: []string{
"kubectl", "kuttl", "test", "-n=test-canary",
},
},
{
testName: "namespace short already set at end is not modified",
namespace: "default",
args: "kuttl test -n test-canary",
args: "kubectl kuttl test -n test-canary",
expected: []string{
"kubectl", "kuttl", "test", "-n", "test-canary",
},
},
{
testName: "namespace long, combined already set at beginning is not modified",
namespace: "default",
args: "--namespace=test-canary kuttl test",
expected: []string{
"kubectl", "--namespace=test-canary", "kuttl", "test",
},
},
{
testName: "namespace long already set at beginning is not modified",
namespace: "default",
args: "--namespace test-canary kuttl test",
expected: []string{
"kubectl", "--namespace", "test-canary", "kuttl", "test",
},
},
{
testName: "namespace short, combined already set at beginning is not modified",
namespace: "default",
args: "-n=test-canary kuttl test",
expected: []string{
"kubectl", "-n=test-canary", "kuttl", "test",
},
},
{
testName: "namespace short already set at beginning is not modified",
namespace: "default",
args: "-n test-canary kuttl test",
expected: []string{
"kubectl", "-n", "test-canary", "kuttl", "test",
},
},
{
testName: "namespace long, combined already set in middle is not modified",
namespace: "default",
args: "kuttl --namespace=test-canary test",
args: "kubectl kuttl --namespace=test-canary test",
expected: []string{
"kubectl", "kuttl", "--namespace=test-canary", "test",
},
},
{
testName: "namespace long already set in middle is not modified",
namespace: "default",
args: "kuttl --namespace test-canary test",
args: "kubectl kuttl --namespace test-canary test",
expected: []string{
"kubectl", "kuttl", "--namespace", "test-canary", "test",
},
},
{
testName: "namespace short, combined already set in middle is not modified",
namespace: "default",
args: "kuttl -n=test-canary test",
args: "kubectl kuttl -n=test-canary test",
expected: []string{
"kubectl", "kuttl", "-n=test-canary", "test",
},
},
{
testName: "namespace short already set in middle is not modified",
namespace: "default",
args: "kuttl -n test-canary test",
args: "kubectl kuttl -n test-canary test",
expected: []string{
"kubectl", "kuttl", "-n", "test-canary", "test",
},
},
{
testName: "namespace not set is appended",
namespace: "default",
args: "kuttl test",
args: "kubectl kuttl test",
expected: []string{
"kubectl", "kuttl", "test", "--namespace", "default",
},
},
{
testName: "unknown arguments do not break parsing with namespace is not set",
namespace: "default",
args: "kuttl test --config kuttl-test.yaml",
args: "kubectl kuttl test --config kuttl-test.yaml",
expected: []string{
"kubectl", "kuttl", "test", "--config", "kuttl-test.yaml", "--namespace", "default",
},
},
{
testName: "unknown arguments do not break parsing if namespace is set at beginning",
namespace: "default",
args: "--namespace=test-canary kuttl test --config kuttl-test.yaml",
args: "kubectl --namespace=test-canary kuttl test --config kuttl-test.yaml",
expected: []string{
"kubectl", "--namespace=test-canary", "kuttl", "test", "--config", "kuttl-test.yaml",
},
},
{
testName: "unknown arguments do not break parsing if namespace is set at middle",
namespace: "default",
args: "kuttl --namespace=test-canary test --config kuttl-test.yaml",
args: "kubectl kuttl --namespace=test-canary test --config kuttl-test.yaml",
expected: []string{
"kubectl", "kuttl", "--namespace=test-canary", "test", "--config", "kuttl-test.yaml",
},
},
{
testName: "unknown arguments do not break parsing if namespace is set at end",
namespace: "default",
args: "kuttl test --config kuttl-test.yaml --namespace=test-canary",
args: "kubectl kuttl test --config kuttl-test.yaml --namespace=test-canary",
expected: []string{
"kubectl", "kuttl", "test", "--config", "kuttl-test.yaml", "--namespace=test-canary",
},
},
{
testName: "quotes are respected when parsing",
namespace: "default",
args: "kuttl \"test quoted\"",
args: "kubectl kuttl \"test quoted\"",
expected: []string{
"kubectl", "kuttl", "test quoted", "--namespace", "default",
},
},
{
testName: "os ENV are expanded",
namespace: "default",
args: "kuttl $TEST_FOO ${TEST_FOO}",
args: "kubectl kuttl $TEST_FOO ${TEST_FOO}",
env: map[string]string{"TEST_FOO": "test"},
expected: []string{
"kubectl", "kuttl", "test", "test", "--namespace", "default",
Expand Down Expand Up @@ -430,7 +398,7 @@ func TestGetKubectlArgs(t *testing.T) {
}
}()
}
cmd, err := GetArgs(context.TODO(), "kubectl", harness.Command{
cmd, err := GetArgs(context.TODO(), harness.Command{
Command: test.args,
Namespaced: true,
}, test.namespace, nil)
Expand Down

0 comments on commit f487791

Please sign in to comment.