From fe1a505cbf555cf31019e46a6e7248871d53079e Mon Sep 17 00:00:00 2001 From: Sebastiaan van Stijn Date: Fri, 31 Oct 2025 14:11:43 +0100 Subject: [PATCH] simplify some commit tests, and work around change in CLI behavior These tests were failing because the "docker run" did not print the ID of the container, and instead detached immediately without printing; ``` === Failed === FAIL: amd64.integration-cli TestDockerCLICommitSuite/TestCommitAfterContainerIsDone (0.27s) docker_cli_commit_test.go:32: assertion failed: Command: /usr/local/cli-integration/docker wait ExitCode: 1 Error: exit status 1 Stdout: Stderr: invalid container name or ID: value is empty Failures: ExitCode was 1 expected 0 Expected no error --- FAIL: TestDockerCLICommitSuite/TestCommitAfterContainerIsDone (0.27s) === FAIL: amd64.integration-cli TestDockerCLICommitSuite/TestCommitWithoutPause (0.20s) docker_cli_commit_test.go:47: assertion failed: Command: /usr/local/cli-integration/docker wait ExitCode: 1 Error: exit status 1 Stdout: Stderr: invalid container name or ID: value is empty Failures: ExitCode was 1 expected 0 Expected no error --- FAIL: TestDockerCLICommitSuite/TestCommitWithoutPause (0.20s) ``` What happens is that it starts a container with only `stdin` attached, but no `stdout`, and the behavior changed between versions of the CLI, which may be either a bugfix or a regression; docker 28 cli doesn't stay attached: ```bash Status: Downloaded newer image for docker:28-cli / # docker run -i -a stdin busybox echo foo / # ``` docker 27 cli stays attached, but has the "three strikes, you're out" handling: ```bash docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:27-cli sh Status: Downloaded newer image for docker:27-cli / # docker run -i -a stdin busybox echo foo 9dbb29080a72225593885bc4880d8f4f22f36803100179f9725468bda1d52b4f ^C^C^C got 3 SIGTERM/SIGINTs, forcefully exiting / # ^C ``` docker 26 cli (and older) don't forward the signal to the container, and detach-keys don't work (or in this case, are handled by the CLI container)?: ```bash docker run -it --rm -v /var/run/docker.sock:/var/run/docker.sock docker:26-cli sh Status: Downloaded newer image for docker:26-cli / # docker run -i -a stdin busybox echo foo 21963ce1b9a7bb7eccef3618693b09a106fb29084b484e31c69cd4a26ee44777 ^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C^C p,q ``` As these tests were not testing that part, I simplified the tests, but we should probably look into the change of behavior to see if it was intentional (and if it was correct). Signed-off-by: Sebastiaan van Stijn --- integration-cli/docker_cli_commit_test.go | 44 +++++++++-------------- 1 file changed, 17 insertions(+), 27 deletions(-) diff --git a/integration-cli/docker_cli_commit_test.go b/integration-cli/docker_cli_commit_test.go index fa24154d71..c21c54262b 100644 --- a/integration-cli/docker_cli_commit_test.go +++ b/integration-cli/docker_cli_commit_test.go @@ -25,45 +25,35 @@ func (s *DockerCLICommitSuite) OnTimeout(t *testing.T) { func (s *DockerCLICommitSuite) TestCommitAfterContainerIsDone(c *testing.T) { skip.If(c, RuntimeIsWindowsContainerd(), "FIXME: Broken on Windows + containerd combination") - out := cli.DockerCmd(c, "run", "-i", "-a", "stdin", "busybox", "echo", "foo").Combined() - - cleanedContainerID := strings.TrimSpace(out) - - cli.DockerCmd(c, "wait", cleanedContainerID) - - out = cli.DockerCmd(c, "commit", cleanedContainerID).Combined() - - cleanedImageID := strings.TrimSpace(out) - - cli.DockerCmd(c, "inspect", cleanedImageID) + cID := cli.DockerCmd(c, "run", "-d", "busybox", "echo", c.Name()).Combined() + cID = strings.TrimSpace(cID) + imageID := cli.DockerCmd(c, "commit", cID).Combined() + imageID = strings.TrimSpace(imageID) + cli.DockerCmd(c, "inspect", imageID) } func (s *DockerCLICommitSuite) TestCommitWithoutPause(c *testing.T) { testRequires(c, DaemonIsLinux) - out := cli.DockerCmd(c, "run", "-i", "-a", "stdin", "busybox", "echo", "foo").Combined() - - cleanedContainerID := strings.TrimSpace(out) - - cli.DockerCmd(c, "wait", cleanedContainerID) - - out = cli.DockerCmd(c, "commit", "-p=false", cleanedContainerID).Combined() - - cleanedImageID := strings.TrimSpace(out) - - cli.DockerCmd(c, "inspect", cleanedImageID) + cID := cli.DockerCmd(c, "run", "-dit", "busybox").Combined() + cID = strings.TrimSpace(cID) + imageID := cli.DockerCmd(c, "commit", "-p=false", cID).Combined() + imageID = strings.TrimSpace(imageID) + cli.DockerCmd(c, "inspect", imageID) } // TestCommitPausedContainer tests that a paused container is not unpaused after being committed func (s *DockerCLICommitSuite) TestCommitPausedContainer(c *testing.T) { testRequires(c, DaemonIsLinux) - containerID := cli.DockerCmd(c, "run", "-i", "-d", "busybox").Stdout() - containerID = strings.TrimSpace(containerID) + cID := cli.DockerCmd(c, "run", "-dit", "busybox").Combined() + cID = strings.TrimSpace(cID) + cli.DockerCmd(c, "pause", cID) - cli.DockerCmd(c, "pause", containerID) - cli.DockerCmd(c, "commit", containerID) + imageID := cli.DockerCmd(c, "commit", cID).Combined() + imageID = strings.TrimSpace(imageID) + cli.DockerCmd(c, "inspect", imageID) - out := inspectField(c, containerID, "State.Paused") // commit should not unpause a paused container + out := inspectField(c, cID, "State.Paused") assert.Assert(c, is.Contains(out, "true")) }