From ae03dd7f46064d2a1a72dbc65f11135216ffc033 Mon Sep 17 00:00:00 2001 From: Vincent Demeester Date: Mon, 23 Apr 2018 14:13:52 +0200 Subject: [PATCH] Handle some TODOs in tests Use more gotestyourself for `env.Patch`, and `icmd.RunCommand` Signed-off-by: Vincent Demeester --- cli/command/cli_test.go | 18 +++--------------- e2e/container/attach_test.go | 5 +++-- e2e/container/kill_test.go | 7 ++----- e2e/container/run_test.go | 12 ++---------- e2e/stack/remove_test.go | 16 +++------------- 5 files changed, 13 insertions(+), 45 deletions(-) diff --git a/cli/command/cli_test.go b/cli/command/cli_test.go index 432e653fdc..d8ac647dfa 100644 --- a/cli/command/cli_test.go +++ b/cli/command/cli_test.go @@ -14,6 +14,7 @@ import ( "github.com/docker/docker/client" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" + "github.com/gotestyourself/gotestyourself/env" "github.com/gotestyourself/gotestyourself/fs" "github.com/pkg/errors" "golang.org/x/net/context" @@ -44,7 +45,7 @@ func TestNewAPIClientFromFlags(t *testing.T) { func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) { customVersion := "v3.3.3" - defer patchEnvVariable(t, "DOCKER_API_VERSION", customVersion)() + defer env.Patch(t, "DOCKER_API_VERSION", customVersion)() opts := &flags.CommonOptions{} configFile := &configfile.ConfigFile{} @@ -53,19 +54,6 @@ func TestNewAPIClientFromFlagsWithAPIVersionFromEnv(t *testing.T) { assert.Check(t, is.Equal(customVersion, apiclient.ClientVersion())) } -// TODO: use gotestyourself/env.Patch -func patchEnvVariable(t *testing.T, key, value string) func() { - oldValue, ok := os.LookupEnv(key) - assert.NilError(t, os.Setenv(key, value)) - return func() { - if !ok { - assert.NilError(t, os.Unsetenv(key)) - return - } - assert.NilError(t, os.Setenv(key, oldValue)) - } -} - type fakeClient struct { client.Client pingFunc func() (types.Ping, error) @@ -260,7 +248,7 @@ func TestOrchestratorSwitch(t *testing.T) { version: defaultVersion, } if testcase.envOrchestrator != "" { - defer patchEnvVariable(t, "DOCKER_ORCHESTRATOR", testcase.envOrchestrator)() + defer env.Patch(t, "DOCKER_ORCHESTRATOR", testcase.envOrchestrator)() } cli := &DockerCli{client: apiclient, err: os.Stderr} diff --git a/e2e/container/attach_test.go b/e2e/container/attach_test.go index 3e7d0ed244..393d8cc81e 100644 --- a/e2e/container/attach_test.go +++ b/e2e/container/attach_test.go @@ -1,6 +1,7 @@ package container import ( + "fmt" "strings" "testing" @@ -19,8 +20,8 @@ func TestAttachExitCode(t *testing.T) { } func runBackgroundContainsWithExitCode(t *testing.T, exitcode int) string { - result := icmd.RunCmd(shell(t, - "docker run -d -i --rm %s sh -c 'read; exit %d'", fixtures.AlpineImage, exitcode)) + result := icmd.RunCommand("docker", "run", "-d", "-i", "--rm", fixtures.AlpineImage, + "sh", "-c", fmt.Sprintf("read; exit %d", exitcode)) result.Assert(t, icmd.Success) return strings.TrimSpace(result.Stdout()) } diff --git a/e2e/container/kill_test.go b/e2e/container/kill_test.go index 25a158c113..cfa0193867 100644 --- a/e2e/container/kill_test.go +++ b/e2e/container/kill_test.go @@ -32,17 +32,14 @@ func TestKillContainer(t *testing.T) { } func runBackgroundTop(t *testing.T) string { - result := icmd.RunCmd(shell(t, - "docker run -d %s top", fixtures.AlpineImage)) + result := icmd.RunCommand("docker", "run", "-d", fixtures.AlpineImage, "top") result.Assert(t, icmd.Success) return strings.TrimSpace(result.Stdout()) } func containerStatus(t *testing.T, containerID, status string) func(poll.LogT) poll.Result { return func(poll.LogT) poll.Result { - result := icmd.RunCmd( - shell(t, "docker inspect -f '{{ .State.Status }}' %s", containerID), - ) + result := icmd.RunCommand("docker", "inspect", "-f", "{{ .State.Status }}", containerID) result.Assert(t, icmd.Success) actual := strings.TrimSpace(result.Stdout()) if actual == status { diff --git a/e2e/container/run_test.go b/e2e/container/run_test.go index 3c954ae488..488dbcec80 100644 --- a/e2e/container/run_test.go +++ b/e2e/container/run_test.go @@ -5,7 +5,6 @@ import ( "testing" "github.com/docker/cli/e2e/internal/fixtures" - shlex "github.com/flynn-archive/go-shlex" "github.com/gotestyourself/gotestyourself/assert" is "github.com/gotestyourself/gotestyourself/assert/cmp" "github.com/gotestyourself/gotestyourself/golden" @@ -17,8 +16,8 @@ const registryPrefix = "registry:5000" func TestRunAttachedFromRemoteImageAndRemove(t *testing.T) { image := createRemoteImage(t) - result := icmd.RunCmd(shell(t, - "docker run --rm %s echo this is output", image)) + result := icmd.RunCommand("docker", "run", "--rm", image, + "echo", "this", "is", "output") result.Assert(t, icmd.Success) assert.Check(t, is.Equal("this is output\n", result.Stdout())) @@ -54,10 +53,3 @@ func createRemoteImage(t *testing.T) string { icmd.RunCommand("docker", "rmi", image).Assert(t, icmd.Success) return image } - -// TODO: move to gotestyourself -func shell(t *testing.T, format string, args ...interface{}) icmd.Cmd { - cmd, err := shlex.Split(fmt.Sprintf(format, args...)) - assert.NilError(t, err) - return icmd.Cmd{Command: cmd} -} diff --git a/e2e/stack/remove_test.go b/e2e/stack/remove_test.go index 0ec8639073..a75939084e 100644 --- a/e2e/stack/remove_test.go +++ b/e2e/stack/remove_test.go @@ -1,13 +1,10 @@ package stack import ( - "fmt" "strings" "testing" "github.com/docker/cli/internal/test/environment" - shlex "github.com/flynn-archive/go-shlex" - "github.com/gotestyourself/gotestyourself/assert" "github.com/gotestyourself/gotestyourself/golden" "github.com/gotestyourself/gotestyourself/icmd" "github.com/gotestyourself/gotestyourself/poll" @@ -20,7 +17,7 @@ func TestRemove(t *testing.T) { deployFullStack(t, stackname) defer cleanupFullStack(t, stackname) - result := icmd.RunCmd(shell(t, "docker stack rm %s", stackname)) + result := icmd.RunCommand("docker", "stack", "rm", stackname) result.Assert(t, icmd.Expected{Err: icmd.None}) golden.Assert(t, result.Stdout(), "stack-remove-success.golden") @@ -28,8 +25,8 @@ func TestRemove(t *testing.T) { func deployFullStack(t *testing.T, stackname string) { // TODO: this stack should have full options not minimal options - result := icmd.RunCmd(shell(t, - "docker stack deploy --compose-file=./testdata/full-stack.yml %s", stackname)) + result := icmd.RunCommand("docker", "stack", "deploy", + "--compose-file=./testdata/full-stack.yml", stackname) result.Assert(t, icmd.Success) poll.WaitOn(t, taskCount(stackname, 2), pollSettings) @@ -66,10 +63,3 @@ func taskCount(stackname string, expected int) func(t poll.LogT) poll.Result { func lines(out string) int { return len(strings.Split(strings.TrimSpace(out), "\n")) } - -// TODO: move to gotestyourself -func shell(t *testing.T, format string, args ...interface{}) icmd.Cmd { - cmd, err := shlex.Split(fmt.Sprintf(format, args...)) - assert.NilError(t, err) - return icmd.Cmd{Command: cmd} -}