From 5028ff1f402088c95f8ea099d9c45b2414e1cfa6 Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Wed, 17 Sep 2025 12:01:02 +0200 Subject: [PATCH] integration-cli: remove startContainerGetOutput, runCommandWithOutput Signed-off-by: Sebastiaan van Stijn --- .../docker_cli_cp_to_container_unix_test.go | 9 +++--- integration-cli/docker_cli_cp_utils_test.go | 26 +++++------------ integration-cli/docker_cli_exec_test.go | 29 ++++++++++++++----- integration-cli/docker_cli_run_test.go | 14 +++++---- integration-cli/docker_cli_save_load_test.go | 14 ++++----- integration-cli/utils_test.go | 14 --------- 6 files changed, 49 insertions(+), 57 deletions(-) diff --git a/integration-cli/docker_cli_cp_to_container_unix_test.go b/integration-cli/docker_cli_cp_to_container_unix_test.go index 3a6f5d8e3d..83a6dcb5af 100644 --- a/integration-cli/docker_cli_cp_to_container_unix_test.go +++ b/integration-cli/docker_cli_cp_to_container_unix_test.go @@ -5,7 +5,6 @@ package main import ( "fmt" "os" - "os/exec" "path/filepath" "strconv" "strings" @@ -14,6 +13,7 @@ import ( "github.com/moby/moby/v2/integration-cli/cli" "gotest.tools/v3/assert" + "gotest.tools/v3/icmd" ) func (s *DockerCLICpSuite) TestCpToContainerWithPermissions(c *testing.T) { @@ -33,11 +33,12 @@ func (s *DockerCLICpSuite) TestCpToContainerWithPermissions(c *testing.T) { srcPath := cpPath(tmpDir, "permdirtest") dstPath := containerCpPath(containerName, "/") - args := []string{"cp", "-a", srcPath, dstPath} - out, _, err := runCommandWithOutput(exec.Command(dockerBinary, args...)) + res := icmd.RunCommand(dockerBinary, "cp", "-a", srcPath, dstPath) + out, err := res.Combined(), res.Error assert.NilError(c, err, "output: %v", out) - out, err = startContainerGetOutput(c, containerName) + res = icmd.RunCommand(dockerBinary, "start", "-a", containerName) + out, err = res.Combined(), res.Error assert.NilError(c, err, "output: %v", out) assert.Equal(c, strings.TrimSpace(out), "2 2 700\n65534 65534 400", "output: %v", out) } diff --git a/integration-cli/docker_cli_cp_utils_test.go b/integration-cli/docker_cli_cp_utils_test.go index 4f1e9d6f65..61112ad5e1 100644 --- a/integration-cli/docker_cli_cp_utils_test.go +++ b/integration-cli/docker_cli_cp_utils_test.go @@ -5,7 +5,6 @@ import ( "fmt" "io" "os" - "os/exec" "path/filepath" "runtime" "strings" @@ -15,6 +14,7 @@ import ( "github.com/moby/moby/v2/integration-cli/cli" "gotest.tools/v3/assert" is "gotest.tools/v3/assert/cmp" + "gotest.tools/v3/icmd" ) type fileType uint32 @@ -195,26 +195,14 @@ func containerCpPathTrailingSep(containerID string, pathElements ...string) stri func runDockerCp(t *testing.T, src, dst string) error { t.Helper() - args := []string{"cp", src, dst} - if out, _, err := runCommandWithOutput(exec.Command(dockerBinary, args...)); err != nil { + res := icmd.RunCommand(dockerBinary, "cp", src, dst) + out, err := res.Combined(), res.Error + if err != nil { return fmt.Errorf("error executing `docker cp` command: %s: %s", err, out) } return nil } -func startContainerGetOutput(t *testing.T, containerID string) (string, error) { - t.Helper() - - args := []string{"start", "-a", containerID} - - out, _, err := runCommandWithOutput(exec.Command(dockerBinary, args...)) - if err != nil { - return "", fmt.Errorf("error executing `docker start` command: %s: %s", err, out) - } - - return out, nil -} - func getTestDir(t *testing.T, label string) (tmpDir string) { t.Helper() var err error @@ -269,18 +257,18 @@ func symlinkTargetEquals(t *testing.T, symlink, expectedTarget string) error { return nil } +// TODO(thaJeztah): deprecate and replace uses with [icmd.RunCommand.Assert(icmd.Expected)] func containerStartOutputEquals(t *testing.T, containerID, contents string) error { t.Helper() - out, err := startContainerGetOutput(t, containerID) + res := icmd.RunCommand(dockerBinary, "start", "-a", containerID) + out, err := res.Combined(), res.Error if err != nil { return err } - if out != contents { return fmt.Errorf("output contents not equal - expected %q, got %q", contents, out) } - return nil } diff --git a/integration-cli/docker_cli_exec_test.go b/integration-cli/docker_cli_exec_test.go index 502ef9d6f7..8fba911d71 100644 --- a/integration-cli/docker_cli_exec_test.go +++ b/integration-cli/docker_cli_exec_test.go @@ -8,7 +8,6 @@ import ( "os" "os/exec" "reflect" - "runtime" "sort" "strings" "sync" @@ -165,10 +164,18 @@ func (s *DockerCLIExecSuite) TestExecTTYCloseStdin(c *testing.T) { stdinRw, err := cmd.StdinPipe() assert.NilError(c, err) - stdinRw.Write([]byte("test")) - stdinRw.Close() + _, err = stdinRw.Write([]byte("test")) + assert.Check(c, err) + _ = stdinRw.Close() - out, _, err := runCommandWithOutput(cmd) + res := icmd.RunCmd(icmd.Cmd{ + Command: cmd.Args, + Env: cmd.Env, + Dir: cmd.Dir, + Stdin: cmd.Stdin, + Stdout: cmd.Stdout, + }) + out, err := res.Combined(), res.Error assert.NilError(c, err, out) out = cli.DockerCmd(c, "top", "exec_tty_stdin").Combined() @@ -193,10 +200,16 @@ func (s *DockerCLIExecSuite) TestExecTTYWithoutStdin(c *testing.T) { } expected := "the input device is not a TTY" - if runtime.GOOS == "windows" { - expected += ". If you are using mintty, try prefixing the command with 'winpty'" - } - if out, _, err := runCommandWithOutput(cmd); err == nil { + + res := icmd.RunCmd(icmd.Cmd{ + Command: cmd.Args, + Env: cmd.Env, + Dir: cmd.Dir, + Stdin: cmd.Stdin, + Stdout: cmd.Stdout, + }) + out, err := res.Combined(), res.Error + if err == nil { errChan <- errors.New("exec should have failed") return } else if !strings.Contains(out, expected) { diff --git a/integration-cli/docker_cli_run_test.go b/integration-cli/docker_cli_run_test.go index 1bd78320f7..02c87c5199 100644 --- a/integration-cli/docker_cli_run_test.go +++ b/integration-cli/docker_cli_run_test.go @@ -15,7 +15,6 @@ import ( "path/filepath" "reflect" "regexp" - "runtime" "sort" "strconv" "strings" @@ -2407,10 +2406,15 @@ func (s *DockerCLIRunSuite) TestRunTTYWithPipe(c *testing.T) { } expected := "the input device is not a TTY" - if runtime.GOOS == "windows" { - expected += ". If you are using mintty, try prefixing the command with 'winpty'" - } - if out, _, err := runCommandWithOutput(cmd); err == nil { + res := icmd.RunCmd(icmd.Cmd{ + Command: cmd.Args, + Env: cmd.Env, + Dir: cmd.Dir, + Stdin: cmd.Stdin, + Stdout: cmd.Stdout, + }) + out, err := res.Combined(), res.Error + if err == nil { errChan <- errors.New("run should have failed") return } else if !strings.Contains(out, expected) { diff --git a/integration-cli/docker_cli_save_load_test.go b/integration-cli/docker_cli_save_load_test.go index 05be37ace7..32b9ec8202 100644 --- a/integration-cli/docker_cli_save_load_test.go +++ b/integration-cli/docker_cli_save_load_test.go @@ -128,15 +128,15 @@ func (s *DockerCLISaveLoadSuite) TestSaveImageId(c *testing.T) { assert.Assert(c, cleanedLongImageID != "", "Id should not be empty.") assert.Assert(c, cleanedShortImageID != "", "Id should not be empty.") + // TODO(thaJeztah): this fails with full image ID saveCmd := exec.Command(dockerBinary, "save", cleanedShortImageID) tarCmd := exec.Command("tar", "t") var err error tarCmd.Stdin, err = saveCmd.StdoutPipe() - assert.Assert(c, err == nil, "cannot set stdout pipe for tar: %v", err) - grepCmd := exec.Command("grep", cleanedLongImageID) - grepCmd.Stdin, err = tarCmd.StdoutPipe() - assert.Assert(c, err == nil, "cannot set stdout pipe for grep: %v", err) + assert.NilError(c, err, "cannot set stdin pipe for tar: %v", err) + tarStream, err := tarCmd.StdoutPipe() + assert.NilError(c, err, "get tar output pipe: %v", err) assert.Assert(c, tarCmd.Start() == nil, "tar failed with error: %v", err) assert.Assert(c, saveCmd.Start() == nil, "docker save failed with error: %v", err) @@ -146,9 +146,9 @@ func (s *DockerCLISaveLoadSuite) TestSaveImageId(c *testing.T) { cli.DockerCmd(c, "rmi", imgRepoName) }() - out, _, err = runCommandWithOutput(grepCmd) - - assert.Assert(c, err == nil, "failed to save repo with image ID: %s, %v", out, err) + res := icmd.RunCmd(icmd.Command("grep", "blobs/sha256/"+cleanedLongImageID), icmd.WithStdin(tarStream)) + out, err = res.Combined(), res.Error + assert.NilError(c, err, "failed to save repo with image ID: %s, %v", out, err) } // save a repo and try to load it using flags diff --git a/integration-cli/utils_test.go b/integration-cli/utils_test.go index d8e8c3f54a..c741859557 100644 --- a/integration-cli/utils_test.go +++ b/integration-cli/utils_test.go @@ -11,7 +11,6 @@ import ( "github.com/moby/moby/v2/integration-cli/cli" "github.com/moby/moby/v2/internal/testutil" "github.com/pkg/errors" - "gotest.tools/v3/icmd" ) func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) { @@ -21,19 +20,6 @@ func getPrefixAndSlashFromDaemonPlatform() (prefix, slash string) { return "", "/" } -// TODO: update code to call cmd.RunCmd directly, and remove this function -// Deprecated: use gotest.tools/icmd -func runCommandWithOutput(execCmd *exec.Cmd) (string, int, error) { - result := icmd.RunCmd(icmd.Cmd{ - Command: execCmd.Args, - Env: execCmd.Env, - Dir: execCmd.Dir, - Stdin: execCmd.Stdin, - Stdout: execCmd.Stdout, - }) - return result.Combined(), result.ExitCode, result.Error -} - // ParseCgroupPaths parses 'procCgroupData', which is output of '/proc//cgroup', and returns // a map which cgroup name as key and path as value. func ParseCgroupPaths(procCgroupData string) map[string]string {