Rewrite detach tests without unbuffer

Signed-off-by: apostasie <[email protected]>

Author: apostasie

cmd/nerdctl/container/container_attach_linux_test.go

@@ -17,9 +17,11 @@
 package container
 
 import (
-	"bytes"
+	"errors"
+	"os"
 	"strings"
 	"testing"
+	"time"
 
 	"gotest.tools/v3/assert"
 
@@ -28,133 +30,185 @@ import (
 	"github.com/containerd/nerdctl/v2/pkg/testutil/test"
 )
 
-// skipAttachForDocker should be called by attach-related tests that assert 'read detach keys' in stdout.
-func skipAttachForDocker(t *testing.T) {
-	t.Helper()
-	if testutil.GetTarget() == testutil.Docker {
-		t.Skip("When detaching from a container, for a session started with 'docker attach'" +
-			", it prints 'read escape sequence', but for one started with 'docker (run|start)', it prints nothing." +
-			" However, the flag is called '--detach-keys' in all cases" +
-			", so nerdctl prints 'read detach keys' for all cases" +
-			", and that's why this test is skipped for Docker.")
-	}
-}
+/*
+Important notes:
+- for both docker and nerdctl, you can run+detach of a container and exit 0, while the container would actually fail starting
+- nerdctl (not docker): on run, detach will race anything on stdin before the detach sequence from reaching the container
+- nerdctl AND docker: on attach ^
+- exit code variants: https://github.com/containerd/nerdctl/issues/3571
+*/
 
-// prepareContainerToAttach spins up a container (entrypoint = shell) with `-it` and detaches from it
-// so that it can be re-attached to later.
-func prepareContainerToAttach(base *testutil.Base, containerName string) {
-	opts := []func(*testutil.Cmd){
-		testutil.WithStdin(testutil.NewDelayOnceReader(bytes.NewReader(
-			[]byte{16, 17}, // ctrl+p,ctrl+q, see https://www.physics.udel.edu/~watson/scen103/ascii.html
-		))),
+func TestAttach(t *testing.T) {
+	// In nerdctl the detach return code from the container after attach is 0, but in docker the return code is 1.
+	// This behaviour is reported in https://github.com/containerd/nerdctl/issues/3571
+	ex := 0
+	if nerdtest.IsDocker() {
+		ex = 1
 	}
-	// unbuffer(1) emulates tty, which is required by `nerdctl run -t`.
-	// unbuffer(1) can be installed with `apt-get install expect`.
-	//
-	// "-p" is needed because we need unbuffer to read from stdin, and from [1]:
-	// "Normally, unbuffer does not read from stdin. This simplifies use of unbuffer in some situations.
-	//  To use unbuffer in a pipeline, use the -p flag."
-	//
-	// [1] https://linux.die.net/man/1/unbuffer
-	base.CmdWithHelper([]string{"unbuffer", "-p"}, "run", "-it", "--name", containerName, testutil.CommonImage).
-		CmdOption(opts...).AssertOutContains("read detach keys")
-	container := base.InspectContainer(containerName)
-	assert.Equal(base.T, container.State.Running, true)
-}
 
-func TestAttach(t *testing.T) {
-	t.Parallel()
+	testCase := nerdtest.Setup()
 
-	t.Skip("This test is very unstable and currently skipped. See https://github.com/containerd/nerdctl/issues/3558")
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
 
-	skipAttachForDocker(t)
+	testCase.Setup = func(data test.Data, helpers test.Helpers) {
+		cmd := helpers.Command("run", "--rm", "-it", "--name", data.Identifier(), testutil.CommonImage)
+		cmd.WithPseudoTTY(func(f *os.File) error {
+			_, err := f.Write([]byte{16, 17})
+			return err
+		})
+
+		cmd.Run(&test.Expected{
+			ExitCode: 0,
+			Errors:   []error{errors.New("read detach keys")},
+			Output: func(stdout string, info string, t *testing.T) {
+				assert.Assert(t, strings.Contains(helpers.Capture("inspect", "--format", "json", data.Identifier()), "\"Running\":true"))
+			},
+		})
+	}
 
-	base := testutil.NewBase(t)
-	containerName := testutil.Identifier(t)
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		// Run interactively and detach
+		cmd := helpers.Command("attach", data.Identifier())
+		cmd.WithPseudoTTY(func(f *os.File) error {
+			_, _ = f.WriteString("echo mark${NON}mark\n")
+			// Interestingly, and unlike with run, on attach, docker (like nerdctl) ALSO needs a pause so that the
+			// container can read stdin before we detach
+			time.Sleep(time.Second)
+			_, err := f.Write([]byte{16, 17})
 
-	defer base.Cmd("container", "rm", "-f", containerName).AssertOK()
-	prepareContainerToAttach(base, containerName)
+			return err
+		})
 
-	opts := []func(*testutil.Cmd){
-		testutil.WithStdin(testutil.NewDelayOnceReader(strings.NewReader("expr 1 + 1\nexit\n"))),
+		return cmd
 	}
-	// `unbuffer -p` returns 0 even if the underlying nerdctl process returns a non-zero exit code,
-	// so the exit code cannot be easily tested here.
-	base.CmdWithHelper([]string{"unbuffer", "-p"}, "attach", containerName).CmdOption(opts...).AssertOutContains("2")
-	container := base.InspectContainer(containerName)
-	assert.Equal(base.T, container.State.Running, false)
+
+	testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected {
+		return &test.Expected{
+			ExitCode: ex,
+			Errors:   []error{errors.New("read detach keys")},
+			Output: test.All(
+				test.Contains("markmark"),
+				func(stdout string, info string, t *testing.T) {
+					assert.Assert(t, strings.Contains(helpers.Capture("inspect", "--format", "json", data.Identifier()), "\"Running\":true"))
+				},
+			),
+		}
+	}
+
+	testCase.Run(t)
 }
 
 func TestAttachDetachKeys(t *testing.T) {
-	t.Parallel()
+	// In nerdctl the detach return code from the container after attach is 0, but in docker the return code is 1.
+	// This behaviour is reported in https://github.com/containerd/nerdctl/issues/3571
+	ex := 0
+	if nerdtest.IsDocker() {
+		ex = 1
+	}
 
-	skipAttachForDocker(t)
+	testCase := nerdtest.Setup()
 
-	base := testutil.NewBase(t)
-	containerName := testutil.Identifier(t)
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
 
-	defer base.Cmd("container", "rm", "-f", containerName).AssertOK()
-	prepareContainerToAttach(base, containerName)
+	testCase.Setup = func(data test.Data, helpers test.Helpers) {
+		cmd := helpers.Command("run", "--rm", "-it", "--detach-keys=ctrl-q", "--name", data.Identifier(), testutil.CommonImage)
+		cmd.WithPseudoTTY(func(f *os.File) error {
+			_, err := f.Write([]byte{17})
+			return err
+		})
+
+		cmd.Run(&test.Expected{
+			ExitCode: 0,
+			Errors:   []error{errors.New("read detach keys")},
+			Output: func(stdout string, info string, t *testing.T) {
+				assert.Assert(t, strings.Contains(helpers.Capture("inspect", "--format", "json", data.Identifier()), "\"Running\":true"))
+			},
+		})
+	}
 
-	opts := []func(*testutil.Cmd){
-		testutil.WithStdin(testutil.NewDelayOnceReader(bytes.NewReader(
-			[]byte{1, 2}, // https://www.physics.udel.edu/~watson/scen103/ascii.html
-		))),
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		// Run interactively and detach
+		cmd := helpers.Command("attach", "--detach-keys=ctrl-a,ctrl-b", data.Identifier())
+		cmd.WithPseudoTTY(func(f *os.File) error {
+			_, _ = f.WriteString("echo mark${NON}mark\n")
+			// Interestingly, and unlike with run, on attach, docker (like nerdctl) ALSO needs a pause so that the
+			// container can read stdin before we detach
+			time.Sleep(time.Second)
+			_, err := f.Write([]byte{1, 2})
+
+			return err
+		})
+
+		return cmd
+	}
+
+	testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected {
+		return &test.Expected{
+			ExitCode: ex,
+			Errors:   []error{errors.New("read detach keys")},
+			Output: test.All(
+				test.Contains("markmark"),
+				func(stdout string, info string, t *testing.T) {
+					assert.Assert(t, strings.Contains(helpers.Capture("inspect", "--format", "json", data.Identifier()), "\"Running\":true"))
+				},
+			),
+		}
 	}
-	base.CmdWithHelper([]string{"unbuffer", "-p"}, "attach", "--detach-keys=ctrl-a,ctrl-b", containerName).
-		CmdOption(opts...).AssertOutContains("read detach keys")
-	container := base.InspectContainer(containerName)
-	assert.Equal(base.T, container.State.Running, true)
+
+	testCase.Run(t)
 }
 
 // TestIssue3568 tests https://github.com/containerd/nerdctl/issues/3568
-func TestDetachAttachKeysForAutoRemovedContainer(t *testing.T) {
+func TestAttachForAutoRemovedContainer(t *testing.T) {
 	testCase := nerdtest.Setup()
 
-	testCase.SubTests = []*test.Case{
-		{
-			Description: "Issue #3568 - A container should be deleted when detaching and attaching a container started with the --rm option.",
-			// In nerdctl the detach return code from the container is 0, but in docker the return code is 1.
-			// This behaviour is reported in https://github.com/containerd/nerdctl/issues/3571 so this test is skipped for Docker.
-			Require: test.Require(
-				test.Not(nerdtest.Docker),
-			),
-			Setup: func(data test.Data, helpers test.Helpers) {
-				cmd := helpers.Command("run", "--rm", "-it", "--detach-keys=ctrl-a,ctrl-b", "--name", data.Identifier(), testutil.CommonImage)
-				// unbuffer(1) can be installed with `apt-get install expect`.
-				//
-				// "-p" is needed because we need unbuffer to read from stdin, and from [1]:
-				// "Normally, unbuffer does not read from stdin. This simplifies use of unbuffer in some situations.
-				//  To use unbuffer in a pipeline, use the -p flag."
-				//
-				// [1] https://linux.die.net/man/1/unbuffer
-				cmd.WithWrapper("unbuffer", "-p")
-				cmd.WithStdin(testutil.NewDelayOnceReader(bytes.NewReader([]byte{1, 2}))) // https://www.physics.udel.edu/~watson/scen103/ascii.html
-				cmd.Run(&test.Expected{
-					ExitCode: 0,
-				})
-			},
-			Command: func(data test.Data, helpers test.Helpers) test.TestableCommand {
-				cmd := helpers.Command("attach", data.Identifier())
-				cmd.WithWrapper("unbuffer", "-p")
-				cmd.WithStdin(testutil.NewDelayOnceReader(strings.NewReader("exit\n")))
-				return cmd
-			},
-			Cleanup: func(data test.Data, helpers test.Helpers) {
-				helpers.Anyhow("rm", "-f", data.Identifier())
-			},
-			Expected: func(data test.Data, helpers test.Helpers) *test.Expected {
-				return &test.Expected{
-					ExitCode: 0,
-					Errors:   []error{},
-					Output: test.All(
-						func(stdout string, info string, t *testing.T) {
-							assert.Assert(t, !strings.Contains(helpers.Capture("ps", "-a"), data.Identifier()))
-						},
-					),
-				}
+	testCase.Description = "Issue #3568 - A container should be deleted when detaching and attaching a container started with the --rm option."
+
+	testCase.Cleanup = func(data test.Data, helpers test.Helpers) {
+		helpers.Anyhow("rm", "-f", data.Identifier())
+	}
+
+	testCase.Setup = func(data test.Data, helpers test.Helpers) {
+		cmd := helpers.Command("run", "--rm", "-it", "--detach-keys=ctrl-a,ctrl-b", "--name", data.Identifier(), testutil.CommonImage)
+		cmd.WithPseudoTTY(func(f *os.File) error {
+			_, err := f.Write([]byte{1, 2})
+			return err
+		})
+
+		cmd.Run(&test.Expected{
+			ExitCode: 0,
+			Errors:   []error{errors.New("read detach keys")},
+			Output: func(stdout string, info string, t *testing.T) {
+				assert.Assert(t, strings.Contains(helpers.Capture("inspect", "--format", "json", data.Identifier()), "\"Running\":true"), info)
 			},
-		},
+		})
+	}
+
+	testCase.Command = func(data test.Data, helpers test.Helpers) test.TestableCommand {
+		// Run interactively and detach
+		cmd := helpers.Command("attach", data.Identifier())
+		cmd.WithPseudoTTY(func(f *os.File) error {
+			_, err := f.WriteString("echo mark${NON}mark\nexit 42\n")
+			return err
+		})
+
+		return cmd
+	}
+
+	testCase.Expected = func(data test.Data, helpers test.Helpers) *test.Expected {
+		return &test.Expected{
+			ExitCode: 42,
+			Output: test.All(
+				test.Contains("markmark"),
+				func(stdout string, info string, t *testing.T) {
+					assert.Assert(t, !strings.Contains(helpers.Capture("ps", "-a"), data.Identifier()))
+				},
+			),
+		}
 	}
 
 	testCase.Run(t)