Add tty test and patch tty stderr/stdout

Author: cmoog

client.go

@@ -200,5 +200,21 @@ func (r remoteProcess) Wait() error {
 }
 
 func (r remoteProcess) Close() error {
-	return r.conn.Close(websocket.StatusAbnormalClosure, "kill process")
+	err := r.conn.Close(websocket.StatusNormalClosure, "")
+	err1 := r.stderr.w.Close()
+	err2 := r.stdout.w.Close()
+	return joinErrs(err, err1, err2)
+}
+
+func joinErrs(errs ...error) error {
+	var str string
+	for _, e := range errs {
+		if e != nil {
+			if str != "" {
+				str += ", "
+			}
+			str += e.Error()
+		}
+	}
+	return xerrors.New(str)
 }

client_test.go

@@ -8,16 +8,13 @@ import (
 	"net"
 	"net/http"
 	"net/http/httptest"
-	"os"
 	"strings"
-	"sync"
 	"testing"
 	"time"
 
 	"cdr.dev/slog/sloggers/slogtest/assert"
 	"cdr.dev/wsep/internal/proto"
 	"github.com/google/go-cmp/cmp"
-	"go.coder.com/flog"
 	"nhooyr.io/websocket"
 )
 
@@ -51,7 +48,7 @@ func TestRemoteStdin(t *testing.T) {
 	}
 }
 
-func TestRemoteExec(t *testing.T) {
+func mockConn(ctx context.Context, t *testing.T) (*websocket.Conn, *httptest.Server) {
 	mockServerHandler := func(w http.ResponseWriter, r *http.Request) {
 		ws, err := websocket.Accept(w, r, nil)
 		if err != nil {
@@ -60,54 +57,28 @@ func TestRemoteExec(t *testing.T) {
 		}
 		err = Serve(r.Context(), ws, LocalExecer{})
 		if err != nil {
-			flog.Error("failed to serve execer: %v", err)
+			t.Errorf("failed to serve execer: %v", err)
 			ws.Close(websocket.StatusAbnormalClosure, "failed to serve execer")
 			return
 		}
 		ws.Close(websocket.StatusNormalClosure, "normal closure")
 	}
 
-	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
-	defer cancel()
-
 	server := httptest.NewServer(http.HandlerFunc(mockServerHandler))
-	defer server.Close()
 
 	ws, _, err := websocket.Dial(ctx, "ws"+strings.TrimPrefix(server.URL, "http"), nil)
 	assert.Success(t, "dial websocket server", err)
-	defer ws.Close(websocket.StatusAbnormalClosure, "abnormal closure")
-
-	execer := RemoteExecer(ws)
-	process, err := execer.Start(ctx, Command{
-		Command: "pwd",
-	})
-	assert.Success(t, "start process", err)
-
-	assert.Equal(t, "pid", false, process.Pid() == 0)
-
-	var wg sync.WaitGroup
-	go func() {
-		wg.Add(1)
-		defer wg.Done()
-
-		stdout, err := ioutil.ReadAll(process.Stdout())
-		assert.Success(t, "read stdout", err)
-		wd, err := os.Getwd()
-		assert.Success(t, "get real working dir", err)
-
-		assert.Equal(t, "stdout", wd, strings.TrimSuffix(string(stdout), "\n"))
-	}()
-	go func() {
-		wg.Add(1)
-		defer wg.Done()
+	return ws, server
+}
 
-		stderr, err := ioutil.ReadAll(process.Stderr())
-		assert.Success(t, "read stderr", err)
-		assert.Equal(t, "len stderr", 0, len(stderr))
-	}()
+func TestRemoteExec(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
 
-	err = process.Wait()
-	assert.Success(t, "wait for process to complete", err)
+	ws, server := mockConn(ctx, t)
+	defer ws.Close(websocket.StatusAbnormalClosure, "abnormal closure")
+	defer server.Close()
 
-	wg.Wait()
+	execer := RemoteExecer(ws)
+	testExecer(ctx, t, execer)
 }

dev/client/main.go

@@ -78,8 +78,6 @@ func do(fl *pflag.FlagSet, tty bool) {
 	err = process.Wait()
 	if err != nil {
 		flog.Error("process failed: %v", err)
-	} else {
-		flog.Info("process finished successfully")
 	}
 	conn.Close(websocket.StatusNormalClosure, "normal closure")
 }

localexec.go

@@ -1,8 +1,10 @@
 package wsep
 
 import (
+	"bytes"
 	"context"
 	"io"
+	"io/ioutil"
 	"os"
 	"os/exec"
 	"syscall"
@@ -93,7 +95,7 @@ func (l LocalExecer) Start(ctx context.Context, c Command) (Process, error) {
 			return nil, xerrors.Errorf("start command with pty: %w", err)
 		}
 		process.stdout = process.tty
-		process.stderr = process.tty
+		process.stderr = ioutil.NopCloser(bytes.NewReader(nil))
 		process.stdin = process.tty
 	} else {
 		process.stdin, err = process.cmd.StdinPipe()

localexec_test.go

@@ -16,7 +16,10 @@ func TestLocalExec(t *testing.T) {
 	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
 	defer cancel()
 
-	execer := LocalExecer{}
+	testExecer(ctx, t, LocalExecer{})
+}
+
+func testExecer(ctx context.Context, t *testing.T, execer Execer) {
 	process, err := execer.Start(ctx, Command{
 		Command: "pwd",
 	})

tty_test.go

@@ -0,0 +1,56 @@
+package wsep
+
+import (
+	"context"
+	"io/ioutil"
+	"strings"
+	"sync"
+	"testing"
+	"time"
+
+	"cdr.dev/slog/sloggers/slogtest/assert"
+	"nhooyr.io/websocket"
+)
+
+func TestTTY(t *testing.T) {
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	ws, server := mockConn(ctx, t)
+	defer ws.Close(websocket.StatusInternalError, "")
+	defer server.Close()
+
+	execer := RemoteExecer(ws)
+	testTTY(ctx, t, execer)
+}
+
+func testTTY(ctx context.Context, t *testing.T, e Execer) {
+	process, err := e.Start(ctx, Command{
+		Command: "bash",
+		TTY:     true,
+	})
+	assert.Success(t, "start bash", err)
+	var wg sync.WaitGroup
+	go func() {
+		wg.Add(1)
+		defer wg.Done()
+
+		stdout, err := ioutil.ReadAll(process.Stdout())
+		assert.Success(t, "read stdout", err)
+		t.Logf("bash tty stdout = %s", stdout)
+		assert.True(t, `bash "$" prompt found`, strings.HasSuffix(string(stdout), "$ "))
+	}()
+	go func() {
+		wg.Add(1)
+		defer wg.Done()
+
+		stderr, err := ioutil.ReadAll(process.Stderr())
+		assert.Success(t, "read stderr", err)
+		t.Logf("bash tty stderr = %s", stderr)
+		assert.True(t, "stderr is empty", len(stderr) == 0)
+	}()
+	time.Sleep(5 * time.Second)
+
+	process.Close()
+	wg.Wait()
+}