From 683b6226ede1eec766e166b19d77d8613c39e6e3 Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 Sep 2017 14:54:03 -0400 Subject: [PATCH 1/3] Add gotestyourself/poll Signed-off-by: Daniel Nephin --- vendor.conf | 2 +- .../gotestyourself/gotestyourself/README.md | 3 +- .../gotestyourself/golden/golden.go | 8 +- .../gotestyourself/icmd/command.go | 19 ++- .../gotestyourself/icmd/exitcode.go | 6 +- .../gotestyourself/gotestyourself/icmd/ops.go | 4 + .../gotestyourself/poll/poll.go | 133 ++++++++++++++++++ 7 files changed, 155 insertions(+), 20 deletions(-) create mode 100644 vendor/github.com/gotestyourself/gotestyourself/icmd/ops.go create mode 100644 vendor/github.com/gotestyourself/gotestyourself/poll/poll.go diff --git a/vendor.conf b/vendor.conf index 004318a681..4e4d165238 100755 --- a/vendor.conf +++ b/vendor.conf @@ -21,7 +21,7 @@ github.com/gogo/protobuf v0.4 github.com/golang/protobuf 7a211bcf3bce0e3f1d74f9894916e6f116ae83b4 github.com/gorilla/context v1.1 github.com/gorilla/mux v1.1 -github.com/gotestyourself/gotestyourself v1.0.0 +github.com/gotestyourself/gotestyourself v1.1.0 github.com/inconshreveable/mousetrap 76626ae9c91c4f2a10f34cad8ce83ea42c93bb75 github.com/mattn/go-shellwords v1.0.3 github.com/Microsoft/go-winio v0.4.4 diff --git a/vendor/github.com/gotestyourself/gotestyourself/README.md b/vendor/github.com/gotestyourself/gotestyourself/README.md index 2648074900..037fba6c77 100644 --- a/vendor/github.com/gotestyourself/gotestyourself/README.md +++ b/vendor/github.com/gotestyourself/gotestyourself/README.md @@ -18,10 +18,11 @@ patterns. a program to summarize `go test` output and test failures * [icmd](http://godoc.org/github.com/gotestyourself/gotestyourself/icmd) - execute binaries and test the output +* [poll](http://godoc.org/github.com/gotestyourself/gotestyourself/poll) - + test asynchronous code by polling until a desired state is reached * [skip](http://godoc.org/github.com/gotestyourself/gotestyourself/skip) - skip tests based on conditions - ## Related * [testify/assert](https://godoc.org/github.com/stretchr/testify/assert) and diff --git a/vendor/github.com/gotestyourself/gotestyourself/golden/golden.go b/vendor/github.com/gotestyourself/gotestyourself/golden/golden.go index 230ff685cd..9b1c8ba4d0 100644 --- a/vendor/github.com/gotestyourself/gotestyourself/golden/golden.go +++ b/vendor/github.com/gotestyourself/gotestyourself/golden/golden.go @@ -37,8 +37,8 @@ func update(t require.TestingT, filename string, actual []byte) { } // Assert compares the actual content to the expected content in the golden file. -// If `--update-golden` is set then the actual content is written to the golden -// file. +// If the `-test.update-golden` flag is set then the actual content is written +// to the golden file. // Returns whether the assertion was successful (true) or not (false) func Assert(t require.TestingT, actual string, filename string, msgAndArgs ...interface{}) bool { expected := Get(t, filename) @@ -60,8 +60,8 @@ func Assert(t require.TestingT, actual string, filename string, msgAndArgs ...in } // AssertBytes compares the actual result to the expected result in the golden -// file. If `--update-golden` is set then the actual content is written to the -// golden file. +// file. If the `-test.update-golden` flag is set then the actual content is +// written to the golden file. // Returns whether the assertion was successful (true) or not (false) // nolint: lll func AssertBytes(t require.TestingT, actual []byte, filename string, msgAndArgs ...interface{}) bool { diff --git a/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go b/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go index 2470fdc662..8729457b4f 100644 --- a/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go +++ b/vendor/github.com/gotestyourself/gotestyourself/icmd/command.go @@ -18,10 +18,8 @@ type testingT interface { Fatalf(string, ...interface{}) } -const ( - // None is a token to inform Result.Assert that the output should be empty - None string = "" -) +// None is a token to inform Result.Assert that the output should be empty +const None string = "[NOTHING]" type lockedBuffer struct { m sync.RWMutex @@ -170,8 +168,7 @@ func (r *Result) Combined() string { return r.outBuffer.String() + r.errBuffer.String() } -// SetExitError sets Error and ExitCode based on Error -func (r *Result) SetExitError(err error) { +func (r *Result) setExitError(err error) { if err == nil { return } @@ -196,7 +193,7 @@ func Command(command string, args ...string) Cmd { } // RunCmd runs a command and returns a Result -func RunCmd(cmd Cmd, cmdOperators ...func(*Cmd)) *Result { +func RunCmd(cmd Cmd, cmdOperators ...CmdOp) *Result { for _, op := range cmdOperators { op(&cmd) } @@ -207,7 +204,7 @@ func RunCmd(cmd Cmd, cmdOperators ...func(*Cmd)) *Result { return WaitOnCmd(cmd.Timeout, result) } -// RunCommand parses a command line and runs it, returning a result +// RunCommand runs a command with default options, and returns a result func RunCommand(command string, args ...string) *Result { return RunCmd(Command(command, args...)) } @@ -218,7 +215,7 @@ func StartCmd(cmd Cmd) *Result { if result.Error != nil { return result } - result.SetExitError(result.Cmd.Start()) + result.setExitError(result.Cmd.Start()) return result } @@ -253,7 +250,7 @@ func buildCmd(cmd Cmd) *Result { // only wait until the timeout. func WaitOnCmd(timeout time.Duration, result *Result) *Result { if timeout == time.Duration(0) { - result.SetExitError(result.Cmd.Wait()) + result.setExitError(result.Cmd.Wait()) return result } @@ -271,7 +268,7 @@ func WaitOnCmd(timeout time.Duration, result *Result) *Result { } result.Timeout = true case err := <-done: - result.SetExitError(err) + result.setExitError(err) } return result } diff --git a/vendor/github.com/gotestyourself/gotestyourself/icmd/exitcode.go b/vendor/github.com/gotestyourself/gotestyourself/icmd/exitcode.go index 32272b4bbb..9356dbcdef 100644 --- a/vendor/github.com/gotestyourself/gotestyourself/icmd/exitcode.go +++ b/vendor/github.com/gotestyourself/gotestyourself/icmd/exitcode.go @@ -7,9 +7,9 @@ import ( "github.com/pkg/errors" ) -// GetExitCode returns the ExitStatus of a process from the error returned by +// getExitCode returns the ExitStatus of a process from the error returned by // exec.Run(). If the exit status could not be parsed an error is returned. -func GetExitCode(err error) (int, error) { +func getExitCode(err error) (int, error) { if exiterr, ok := err.(*exec.ExitError); ok { if procExit, ok := exiterr.Sys().(syscall.WaitStatus); ok { return procExit.ExitStatus(), nil @@ -22,7 +22,7 @@ func processExitCode(err error) (exitCode int) { if err == nil { return 0 } - exitCode, exiterr := GetExitCode(err) + exitCode, exiterr := getExitCode(err) if exiterr != nil { // TODO: Fix this so we check the error's text. // we've failed to retrieve exit code, so we set it to 127 diff --git a/vendor/github.com/gotestyourself/gotestyourself/icmd/ops.go b/vendor/github.com/gotestyourself/gotestyourself/icmd/ops.go new file mode 100644 index 0000000000..02b1d84023 --- /dev/null +++ b/vendor/github.com/gotestyourself/gotestyourself/icmd/ops.go @@ -0,0 +1,4 @@ +package icmd + +// CmdOp is an operation which modified a Cmd structure used to execute commands +type CmdOp func(*Cmd) diff --git a/vendor/github.com/gotestyourself/gotestyourself/poll/poll.go b/vendor/github.com/gotestyourself/gotestyourself/poll/poll.go new file mode 100644 index 0000000000..3838d38e13 --- /dev/null +++ b/vendor/github.com/gotestyourself/gotestyourself/poll/poll.go @@ -0,0 +1,133 @@ +/*Package poll provides tools for testing asynchronous code. + */ +package poll + +import ( + "fmt" + "time" +) + +// TestingT is the subset of testing.T used by WaitOn +type TestingT interface { + LogT + Fatalf(format string, args ...interface{}) +} + +// LogT is a logging interface that is passed to the WaitOn check function +type LogT interface { + Log(args ...interface{}) + Logf(format string, args ...interface{}) +} + +// Settings are used to configure the behaviour of WaitOn +type Settings struct { + // Timeout is the maximum time to wait for the condition. Defaults to 10s + Timeout time.Duration + // Delay is the time to sleep between checking the condition. Detaults to + // 1ms + Delay time.Duration +} + +func defaultConfig() *Settings { + return &Settings{Timeout: 10 * time.Second, Delay: time.Millisecond} +} + +// SettingOp is a function which accepts and modifies Settings +type SettingOp func(config *Settings) + +// WithDelay sets the delay to wait between polls +func WithDelay(delay time.Duration) SettingOp { + return func(config *Settings) { + config.Delay = delay + } +} + +// WithTimeout sets the timeout +func WithTimeout(timeout time.Duration) SettingOp { + return func(config *Settings) { + config.Timeout = timeout + } +} + +// Result of a check performed by WaitOn +type Result interface { + // Error indicates that the check failed and polling should stop, and the + // the has failed + Error() error + // Done indicates that polling should stop, and the test should proceed + Done() bool + // Message provides the most recent state when polling has not completed + Message() string +} + +type result struct { + done bool + message string + err error +} + +func (r result) Done() bool { + return r.done +} + +func (r result) Message() string { + return r.message +} + +func (r result) Error() error { + return r.err +} + +// Continue returns a Result that indicates to WaitOn that it should continue +// polling. The message text will be used as the failure message if the timeout +// is reached. +func Continue(message string, args ...interface{}) Result { + return result{message: fmt.Sprintf(message, args...)} +} + +// Success returns a Result where Done() returns true, which indicates to WaitOn +// that it should stop polling and exit without an error. +func Success() Result { + return result{done: true} +} + +// Error returns a Result that indicates to WaitOn that it should fail the test +// and stop polling. +func Error(err error) Result { + return result{err: err} +} + +// WaitOn a condition or until a timeout. Poll by calling check and exit when +// check returns a done Result. To fail a test and exit polling with an error +// return a error result. +func WaitOn(t TestingT, check func(t LogT) Result, pollOps ...SettingOp) { + config := defaultConfig() + for _, pollOp := range pollOps { + pollOp(config) + } + + var lastMessage string + after := time.After(config.Timeout) + chResult := make(chan Result) + for { + go func() { + chResult <- check(t) + }() + select { + case <-after: + if lastMessage == "" { + lastMessage = "first check never completed" + } + t.Fatalf("timeout hit after %s: %s", config.Timeout, lastMessage) + case result := <-chResult: + switch { + case result.Error() != nil: + t.Fatalf("polling check failed: %s", result.Error()) + case result.Done(): + return + } + time.Sleep(config.Delay) + lastMessage = result.Message() + } + } +} From 677d17150a06dde5f50a7eb1bdb8612f230e526c Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 Sep 2017 15:30:33 -0400 Subject: [PATCH 2/3] Move common e2e things into an internal package. Signed-off-by: Daniel Nephin --- e2e/stack/main_test.go | 13 ++------ e2e/stack/remove_test.go | 48 +++++++--------------------- internal/test/environment/testenv.go | 21 ++++++++++++ 3 files changed, 35 insertions(+), 47 deletions(-) create mode 100644 internal/test/environment/testenv.go diff --git a/e2e/stack/main_test.go b/e2e/stack/main_test.go index 74081f457b..0d36e45bd4 100644 --- a/e2e/stack/main_test.go +++ b/e2e/stack/main_test.go @@ -5,22 +5,13 @@ import ( "os" "testing" - "github.com/pkg/errors" + "github.com/docker/cli/internal/test/environment" ) func TestMain(m *testing.M) { - if err := setupTestEnv(); err != nil { + if err := environment.Setup(); err != nil { fmt.Println(err.Error()) os.Exit(3) } os.Exit(m.Run()) } - -// TODO: move to shared internal package -func setupTestEnv() error { - dockerHost := os.Getenv("TEST_DOCKER_HOST") - if dockerHost == "" { - return errors.New("$TEST_DOCKER_HOST must be set") - } - return os.Setenv("DOCKER_HOST", dockerHost) -} diff --git a/e2e/stack/remove_test.go b/e2e/stack/remove_test.go index 2d39d02345..f579340793 100644 --- a/e2e/stack/remove_test.go +++ b/e2e/stack/remove_test.go @@ -4,14 +4,17 @@ import ( "fmt" "strings" "testing" - "time" + "github.com/docker/cli/internal/test/environment" shlex "github.com/flynn-archive/go-shlex" "github.com/gotestyourself/gotestyourself/golden" "github.com/gotestyourself/gotestyourself/icmd" + "github.com/gotestyourself/gotestyourself/poll" "github.com/stretchr/testify/require" ) +var pollSettings = environment.DefaultPollSettings + func TestRemove(t *testing.T) { stackname := "test-stack-remove" deployFullStack(t, stackname) @@ -29,21 +32,24 @@ func deployFullStack(t *testing.T, stackname string) { "docker stack deploy --compose-file=./testdata/full-stack.yml %s", stackname)) result.Assert(t, icmd.Success) - waitOn(t, taskCount(stackname, 2), 0) + poll.WaitOn(t, taskCount(stackname, 2), pollSettings) } func cleanupFullStack(t *testing.T, stackname string) { result := icmd.RunCmd(shell(t, "docker stack rm %s", stackname)) result.Assert(t, icmd.Success) - waitOn(t, taskCount(stackname, 0), 0) + poll.WaitOn(t, taskCount(stackname, 0), pollSettings) } -func taskCount(stackname string, expected int) func() (bool, error) { - return func() (bool, error) { +func taskCount(stackname string, expected int) func(t poll.LogT) poll.Result { + return func(poll.LogT) poll.Result { result := icmd.RunCommand( "docker", "stack", "ps", "-f=desired-state=running", stackname) count := lines(result.Stdout()) - 1 - return count == expected, nil + if count == expected { + return poll.Success() + } + return poll.Continue("task count is %d waiting for %d", count, expected) } } @@ -57,33 +63,3 @@ func shell(t *testing.T, format string, args ...interface{}) icmd.Cmd { require.NoError(t, err) return icmd.Cmd{Command: cmd} } - -// TODO: move to gotestyourself -func waitOn(t *testing.T, check func() (bool, error), timeout time.Duration) { - if timeout == time.Duration(0) { - timeout = defaultTimeout() - } - - after := time.After(timeout) - for { - select { - case <-after: - // TODO: include check function name in error message - t.Fatalf("timeout hit after %s", timeout) - default: - // TODO: maybe return a failure message as well? - done, err := check() - if done { - return - } - if err != nil { - t.Fatal(err.Error()) - } - } - } -} - -func defaultTimeout() time.Duration { - // TODO: support override from environment variable - return 10 * time.Second -} diff --git a/internal/test/environment/testenv.go b/internal/test/environment/testenv.go new file mode 100644 index 0000000000..d30f22537e --- /dev/null +++ b/internal/test/environment/testenv.go @@ -0,0 +1,21 @@ +package environment + +import ( + "os" + "time" + + "github.com/gotestyourself/gotestyourself/poll" + "github.com/pkg/errors" +) + +// Setup a new environment +func Setup() error { + dockerHost := os.Getenv("TEST_DOCKER_HOST") + if dockerHost == "" { + return errors.New("$TEST_DOCKER_HOST must be set") + } + return os.Setenv("DOCKER_HOST", dockerHost) +} + +// DefaultPollSettings used with gotestyourself/poll +var DefaultPollSettings = poll.WithDelay(100 * time.Millisecond) From c34360cc8ebb6c4661a497b543e384685241089e Mon Sep 17 00:00:00 2001 From: Daniel Nephin Date: Fri, 1 Sep 2017 17:52:41 -0400 Subject: [PATCH 3/3] Add an end-to-end test for container run for testing attach, remove, and pull image when missing. Signed-off-by: Daniel Nephin --- e2e/container/main_test.go | 17 ++++++++ e2e/container/run_test.go | 42 +++++++++++++++++++ ...run-attached-from-remote-and-remove.golden | 4 ++ scripts/test/e2e/run | 3 +- scripts/test/e2e/wrapper | 1 + 5 files changed, 66 insertions(+), 1 deletion(-) create mode 100644 e2e/container/main_test.go create mode 100644 e2e/container/run_test.go create mode 100644 e2e/container/testdata/run-attached-from-remote-and-remove.golden diff --git a/e2e/container/main_test.go b/e2e/container/main_test.go new file mode 100644 index 0000000000..4363c43888 --- /dev/null +++ b/e2e/container/main_test.go @@ -0,0 +1,17 @@ +package container + +import ( + "fmt" + "os" + "testing" + + "github.com/docker/cli/internal/test/environment" +) + +func TestMain(m *testing.M) { + if err := environment.Setup(); err != nil { + fmt.Println(err.Error()) + os.Exit(3) + } + os.Exit(m.Run()) +} diff --git a/e2e/container/run_test.go b/e2e/container/run_test.go new file mode 100644 index 0000000000..fa5f28b4fa --- /dev/null +++ b/e2e/container/run_test.go @@ -0,0 +1,42 @@ +package container + +import ( + "fmt" + "testing" + + shlex "github.com/flynn-archive/go-shlex" + "github.com/gotestyourself/gotestyourself/golden" + "github.com/gotestyourself/gotestyourself/icmd" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +const alpineImage = "registry:5000/alpine:3.6" + +func TestRunAttachedFromRemoteImageAndRemove(t *testing.T) { + image := createRemoteImage(t) + + result := icmd.RunCmd(shell(t, + "docker run --rm %s echo this is output", image)) + + result.Assert(t, icmd.Success) + assert.Equal(t, "this is output\n", result.Stdout()) + golden.Assert(t, result.Stderr(), "run-attached-from-remote-and-remove.golden") +} + +// TODO: create this with registry API instead of engine API +func createRemoteImage(t *testing.T) string { + image := "registry:5000/alpine:test-run-pulls" + icmd.RunCommand("docker", "pull", alpineImage).Assert(t, icmd.Success) + icmd.RunCommand("docker", "tag", alpineImage, image).Assert(t, icmd.Success) + icmd.RunCommand("docker", "push", image).Assert(t, icmd.Success) + 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...)) + require.NoError(t, err) + return icmd.Cmd{Command: cmd} +} diff --git a/e2e/container/testdata/run-attached-from-remote-and-remove.golden b/e2e/container/testdata/run-attached-from-remote-and-remove.golden new file mode 100644 index 0000000000..26377199bf --- /dev/null +++ b/e2e/container/testdata/run-attached-from-remote-and-remove.golden @@ -0,0 +1,4 @@ +Unable to find image 'registry:5000/alpine:test-run-pulls' locally +test-run-pulls: Pulling from alpine +Digest: sha256:0930dd4cc97ed5771ebe9be9caf3e8dc5341e0b5e32e8fb143394d7dfdfa100e +Status: Downloaded newer image for registry:5000/alpine:test-run-pulls diff --git a/scripts/test/e2e/run b/scripts/test/e2e/run index 936b5898fd..4010e756c4 100755 --- a/scripts/test/e2e/run +++ b/scripts/test/e2e/run @@ -39,11 +39,12 @@ function cleanup { function runtests { local engine_host=$1 + # shellcheck disable=SC2086 env -i \ TEST_DOCKER_HOST="$engine_host" \ GOPATH="$GOPATH" \ PATH="$PWD/build/" \ - "$(which go)" test -v ./e2e/... + "$(which go)" test -v ./e2e/... ${TESTFLAGS-} } export unique_id="${E2E_UNIQUE_ID:-cliendtoendsuite}" diff --git a/scripts/test/e2e/wrapper b/scripts/test/e2e/wrapper index a3a4f00b67..0f85f43283 100755 --- a/scripts/test/e2e/wrapper +++ b/scripts/test/e2e/wrapper @@ -27,6 +27,7 @@ testexit=0 docker run -i --rm \ -v "$PWD:/go/src/github.com/docker/cli" \ --network "${unique_id}_default" \ + -e TESTFLAGS \ "$dev_image" \ ./scripts/test/e2e/run test "$engine_host" || testexit="$?" run_in_env cleanup