fix the flaws in the test of the attach API

Signed-off-by: Shijiang Wei <mountkin@gmail.com>

1 files changed, 65 insertions, 111 deletions

diff --git a/integration-cli/docker_api_attach_test.go b/integration-cli/docker_api_attach_test.go
index d2ca4ba29d..283a0b53eb 100644
--- a/integration-cli/docker_api_attach_test.go
+++ b/integration-cli/docker_api_attach_test.go
@@ -1,10 +1,11 @@
 package main
 
 import (
+	"bufio"
 	"bytes"
 	"io"
+	"net"
 	"net/http"
-	"net/http/httputil"
 	"strings"
 	"time"
 
@@ -99,128 +100,81 @@ func (s *DockerSuite) TestGetContainersWsAttachContainerNotFound(c *check.C) {
 
 func (s *DockerSuite) TestPostContainersAttach(c *check.C) {
 	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "run", "-dit", "busybox", "cat")
-
-	r, w := io.Pipe()
-	defer r.Close()
-	defer w.Close()
-
-	conn, err := sockConn(time.Duration(10 * time.Second))
-	c.Assert(err, check.IsNil)
-
-	containerID := strings.TrimSpace(out)
 
-	req, err := http.NewRequest("POST", "/containers/"+containerID+"/attach?stream=1&stdin=1&stdout=1&stderr=1", bytes.NewReader([]byte{}))
-	c.Assert(err, check.IsNil)
-
-	client := httputil.NewClientConn(conn, nil)
-	defer client.Close()
-
-	// Do POST attach request
-	resp, err := client.Do(req)
-	c.Assert(resp.StatusCode, check.Equals, http.StatusOK)
-	// If we check the err, we get a ErrPersistEOF = &http.ProtocolError{ErrorString: "persistent connection closed"}
-	// This means that the remote requested this be the last request serviced, is this okay?
-
-	// Test read and write to the attached container
-	expected := []byte("hello")
-	actual := make([]byte, len(expected))
-
-	outChan := make(chan error)
-	go func() {
-		_, err := r.Read(actual)
-		outChan <- err
-		close(outChan)
-	}()
-
-	inChan := make(chan error)
-	go func() {
-		_, err := w.Write(expected)
-		inChan <- err
-		close(inChan)
-	}()
+	expectSuccess := func(conn net.Conn, br *bufio.Reader, stream string, tty bool) {
+		defer conn.Close()
+		expected := []byte("success")
+		_, err := conn.Write(expected)
+		c.Assert(err, check.IsNil)
 
-	select {
-	case err := <-inChan:
+		conn.SetReadDeadline(time.Now().Add(time.Second))
+		lenHeader := 0
+		if !tty {
+			lenHeader = 8
+		}
+		actual := make([]byte, len(expected)+lenHeader)
+		_, err = io.ReadFull(br, actual)
 		c.Assert(err, check.IsNil)
-	case <-time.After(5 * time.Second):
-		c.Fatal("Timeout writing to stdout")
+		if !tty {
+			fdMap := map[string]byte{
+				"stdin":  0,
+				"stdout": 1,
+				"stderr": 2,
+			}
+			c.Assert(actual[0], check.Equals, fdMap[stream])
+		}
+		c.Assert(actual[lenHeader:], check.DeepEquals, expected, check.Commentf("Attach didn't return the expected data from %s", stream))
 	}
 
-	select {
-	case err := <-outChan:
+	expectTimeout := func(conn net.Conn, br *bufio.Reader, stream string) {
+		defer conn.Close()
+		_, err := conn.Write([]byte{'t'})
 		c.Assert(err, check.IsNil)
-	case <-time.After(5 * time.Second):
-		c.Fatal("Timeout reading from stdin")
-	}
 
-	if !bytes.Equal(expected, actual) {
-		c.Fatal("Expected output to match input")
+		conn.SetReadDeadline(time.Now().Add(time.Second))
+		actual := make([]byte, 1)
+		_, err = io.ReadFull(br, actual)
+		opErr, ok := err.(*net.OpError)
+		c.Assert(ok, check.Equals, true, check.Commentf("Error is expected to be *net.OpError, got %v", err))
+		c.Assert(opErr.Timeout(), check.Equals, true, check.Commentf("Read from %s is expected to timeout", stream))
 	}
 
-	resp.Body.Close()
-}
-
-func (s *DockerSuite) TestPostContainersAttachStderr(c *check.C) {
-	testRequires(c, DaemonIsLinux)
-	out, _ := dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2")
-
-	r, w := io.Pipe()
-	defer r.Close()
-	defer w.Close()
-
-	conn, err := sockConn(time.Duration(10 * time.Second))
+	// Create a container that only emits stdout.
+	cid, _ := dockerCmd(c, "run", "-di", "busybox", "cat")
+	cid = strings.TrimSpace(cid)
+	// Attach to the container's stdout stream.
+	conn, br, err := sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain")
 	c.Assert(err, check.IsNil)
-
-	containerID := strings.TrimSpace(out)
-
-	req, err := http.NewRequest("POST", "/containers/"+containerID+"/attach?stream=1&stdin=1&stdout=1&stderr=1", bytes.NewReader([]byte{}))
+	// Check if the data from stdout can be received.
+	expectSuccess(conn, br, "stdout", false)
+	// Attach to the container's stderr stream.
+	conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain")
 	c.Assert(err, check.IsNil)
+	// Since the container only emits stdout, attaching to stderr should return nothing.
+	expectTimeout(conn, br, "stdout")
 
-	client := httputil.NewClientConn(conn, nil)
-	defer client.Close()
-
-	// Do POST attach request
-	resp, err := client.Do(req)
-	c.Assert(resp.StatusCode, check.Equals, http.StatusOK)
-	// If we check the err, we get a ErrPersistEOF = &http.ProtocolError{ErrorString: "persistent connection closed"}
-	// This means that the remote requested this be the last request serviced, is this okay?
-
-	// Test read and write to the attached container
-	expected := []byte("hello")
-	actual := make([]byte, len(expected))
-
-	outChan := make(chan error)
-	go func() {
-		_, err := r.Read(actual)
-		outChan <- err
-		close(outChan)
-	}()
-
-	inChan := make(chan error)
-	go func() {
-		_, err := w.Write(expected)
-		inChan <- err
-		close(inChan)
-	}()
-
-	select {
-	case err := <-inChan:
-		c.Assert(err, check.IsNil)
-	case <-time.After(5 * time.Second):
-		c.Fatal("Timeout writing to stdout")
-	}
-
-	select {
-	case err := <-outChan:
-		c.Assert(err, check.IsNil)
-	case <-time.After(5 * time.Second):
-		c.Fatal("Timeout reading from stdin")
-	}
+	// Test the simlar functions of the stderr stream.
+	cid, _ = dockerCmd(c, "run", "-di", "busybox", "/bin/sh", "-c", "cat >&2")
+	cid = strings.TrimSpace(cid)
+	conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain")
+	c.Assert(err, check.IsNil)
+	expectSuccess(conn, br, "stderr", false)
+	conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain")
+	c.Assert(err, check.IsNil)
+	expectTimeout(conn, br, "stderr")
 
-	if !bytes.Equal(expected, actual) {
-		c.Fatal("Expected output to match input")
-	}
+	// Test with tty.
+	cid, _ = dockerCmd(c, "run", "-dit", "busybox", "/bin/sh", "-c", "cat >&2")
+	cid = strings.TrimSpace(cid)
+	// Attach to stdout only.
+	conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stdout=1", nil, "text/plain")
+	c.Assert(err, check.IsNil)
+	expectSuccess(conn, br, "stdout", true)
 
-	resp.Body.Close()
+	// Attach without stdout stream.
+	conn, br, err = sockRequestHijack("POST", "/containers/"+cid+"/attach?stream=1&stdin=1&stderr=1", nil, "text/plain")
+	c.Assert(err, check.IsNil)
+	// Nothing should be received because both the stdout and stderr of the container will be
+	// sent to the client as stdout when tty is enabled.
+	expectTimeout(conn, br, "stdout")
 }