From 9066a11ecb52442ce60dacf5ede25d0a8a8d5f35 Mon Sep 17 00:00:00 2001 From: apostasie Date: Fri, 18 Oct 2024 09:18:38 -0700 Subject: [PATCH] Fix EnsureContainerStarted logic Signed-off-by: apostasie --- docs/testing/tools.md | 4 ++- pkg/testutil/nerdtest/utilities.go | 47 ++++++++++++++++++++---------- pkg/testutil/test/command.go | 5 +++- 3 files changed, 38 insertions(+), 18 deletions(-) diff --git a/docs/testing/tools.md b/docs/testing/tools.md index d1f64cd4c80..2f53441b30c 100644 --- a/docs/testing/tools.md +++ b/docs/testing/tools.md @@ -48,7 +48,9 @@ You already saw two (`test.Expects` and `test.Contains`): First, `test.Expects(exitCode int, errors []error, outputCompare Comparator)`, which is convenient to quickly describe what you expect overall. -`exitCode` is obvious (note that passing -1 as an exit code will just verify the commands does fail without comparing the code). +`exitCode` is obvious (note that passing -1 as an exit code will just +verify the commands does fail without comparing the code, and -2 will not verify the exit +code at all). `errors` is a slice of go `error`, that allows you to compare what is seen on stderr with existing errors (for example: `errdefs.ErrNotFound`), or more generally diff --git a/pkg/testutil/nerdtest/utilities.go b/pkg/testutil/nerdtest/utilities.go index 384f5132110..9dc13c3f1e2 100644 --- a/pkg/testutil/nerdtest/utilities.go +++ b/pkg/testutil/nerdtest/utilities.go @@ -87,28 +87,43 @@ func InspectImage(helpers test.Helpers, name string) dockercompat.Image { } const ( - maxRetry = 5 + maxRetry = 10 sleep = time.Second ) func EnsureContainerStarted(helpers test.Helpers, con string) { - for i := 0; i < maxRetry; i++ { - count := i - cmd := helpers.Command("container", "inspect", con) - cmd.Run(&test.Expected{ + started := false + for i := 0; i < maxRetry && !started; i++ { + helpers.Command("container", "inspect", con). + Run(&test.Expected{ + ExitCode: -2, + Output: func(stdout string, info string, t *testing.T) { + var dc []dockercompat.Container + err := json.Unmarshal([]byte(stdout), &dc) + if err != nil || len(dc) == 0 { + return + } + assert.Equal(t, len(dc), 1, "Unexpectedly got multiple results\n"+info) + started = dc[0].State.Running + }, + }) + time.Sleep(sleep) + } + + if !started { + var dckr string + helpers.Custom("sudo", "journalctl", "-u", "docker", "--no-pager").Run(&test.Expected{ Output: func(stdout string, info string, t *testing.T) { - var dc []dockercompat.Container - err := json.Unmarshal([]byte(stdout), &dc) - assert.NilError(t, err, "Unable to unmarshal output\n"+info) - assert.Equal(t, 1, len(dc), "Unexpectedly got multiple results\n"+info) - if dc[0].State.Running { - return - } - if count == maxRetry-1 { - t.Fatalf("container %s still not running after %d retries", con, count) - } - time.Sleep(sleep) + dckr = stdout }, }) + ins := helpers.Capture("container", "inspect", con) + lgs := helpers.Capture("logs", con) + ps := helpers.Capture("ps", "-a") + helpers.T().Log(ins) + helpers.T().Log(dckr) + helpers.T().Log(lgs) + helpers.T().Log(ps) + helpers.T().Fatalf("container %s still not running after %d retries", con, maxRetry) } } diff --git a/pkg/testutil/test/command.go b/pkg/testutil/test/command.go index a5b1de747d0..4516975fa97 100644 --- a/pkg/testutil/test/command.go +++ b/pkg/testutil/test/command.go @@ -117,7 +117,10 @@ func (gc *GenericCommand) Run(expect *Expected) { // Build the debug string - additionally attach the env (which iCmd does not do) debug := result.String() + "Env:\n" + strings.Join(env, "\n") // ExitCode goes first - if expect.ExitCode == -1 { + if expect.ExitCode == -2 { //nolint:revive + // -2 means we do not care at all about exit code + } else if expect.ExitCode == -1 { + // -1 means any error assert.Assert(gc.t, result.ExitCode != 0, "Expected exit code to be different than 0\n"+debug) } else {