Get windows tests working with conpty

coder · bryphe-coder · Feb 15, 2022 · Feb 11, 2022 · Feb 11, 2022 · Feb 11, 2022
commit 8a711580ac4271cd1751ced290079310fd592377
diff --git a/expect/conpty/conpty.go b/expect/conpty/conpty.go
@@ -19,9 +19,14 @@ type ConPty struct {
 	hpCon       windows.Handle
 	pipeFdIn    windows.Handle
 	pipeFdOut   windows.Handle
+	pipe3 windows.Handle
+	pipe4 windows.Handle
 	consoleSize uintptr
-	inPipe      *os.File
-	outPipe     *os.File
+	outputR      *os.File
+	outputW     *os.File
+	inputR      *os.File
+	inputW     *os.File
+	closed     bool
 }
 
 // New returns a new ConPty pseudo terminal device
@@ -35,38 +40,45 @@ func New(columns int16, rows int16) (*ConPty, error) {
 
 // Close closes the pseudo-terminal and cleans up all attached resources
 func (c *ConPty) Close() error {
+	if (c.closed) {
+		return nil
+	}
+
 	err := closePseudoConsole(c.hpCon)
-	c.inPipe.Close()
-	c.outPipe.Close()
+	c.outputR.Close()
+	c.outputW.Close()
+	c.inputR.Close()
+	c.inputW.Close()
+	c.closed = true
 	return err
 }
 
 // OutPipe returns the output pipe of the pseudo terminal
 func (c *ConPty) OutPipe() *os.File {
-	return c.inPipe
+	return c.outputR
 }
 
 func (c *ConPty) Reader() io.Reader {
-	return c.outPipe
+	return c.outputW
 }
 
 // InPipe returns input pipe of the pseudo terminal
 // Note: It is safer to use the Write method to prevent partially-written VT sequences
 // from corrupting the terminal
 func (c *ConPty) InPipe() *os.File {
-	return c.outPipe
+	return c.inputR
 }
 
 func (c *ConPty) WriteString(str string) (int, error) {
-	return c.inPipe.WriteString(str)
+	return c.inputW.WriteString(str)
 }
 
 func (c *ConPty) createPseudoConsoleAndPipes() error {
 	// These are the readers/writers for "stdin", but we only need this to
 	// successfully call CreatePseudoConsole. After, we can throw it away.
 	var hPipeInW, hPipeInR windows.Handle
 
-	// Create the stdin pipe although we never use this.
+	// Create the stdin pipe
 	if err := windows.CreatePipe(&hPipeInR, &hPipeInW, nil, 0); err != nil {
 		return err
 	}
@@ -81,16 +93,15 @@ func (c *ConPty) createPseudoConsoleAndPipes() error {
 		return fmt.Errorf("failed to create pseudo console: %d, %v", uintptr(c.hpCon), err)
 	}
 
-	// Close our stdin cause we're never going to use it
-	if hPipeInR != windows.InvalidHandle {
-		windows.CloseHandle(hPipeInR)
-	}
-	if hPipeInW != windows.InvalidHandle {
-		windows.CloseHandle(hPipeInW)
-	}
+	c.pipe3 = hPipeInR
+	c.pipe4 = hPipeInW
+
+	c.outputR = os.NewFile(uintptr(c.pipeFdIn), "|0")
+	c.outputW = os.NewFile(uintptr(c.pipeFdOut), "|1")
 
-	c.inPipe = os.NewFile(uintptr(c.pipeFdIn), "|0")
-	c.outPipe = os.NewFile(uintptr(c.pipeFdOut), "|1")
+	c.inputR = os.NewFile(uintptr(c.pipe3), "|2")
+	c.inputW = os.NewFile(uintptr(c.pipe4), "|3")
+	c.closed = false
 
 	return nil
 }

diff --git a/expect/console.go b/expect/console.go
@@ -21,7 +21,6 @@ import (
 	"io/ioutil"
 	"log"
 	"os"
-	"time"
 	"unicode/utf8"
 
 	"github.com/coder/coder/expect/pty"
@@ -49,7 +48,6 @@ type ConsoleOpts struct {
 	Closers         []io.Closer
 	ExpectObservers []ExpectObserver
 	SendObservers   []SendObserver
-	ReadTimeout     *time.Duration
 }
 
 // ExpectObserver provides an interface for a function callback that will
@@ -114,14 +112,6 @@ func WithSendObserver(observers ...SendObserver) ConsoleOpt {
 	}
 }
 
-// WithDefaultTimeout sets a default read timeout during Expect statements.
-func WithDefaultTimeout(timeout time.Duration) ConsoleOpt {
-	return func(opts *ConsoleOpts) error {
-		opts.ReadTimeout = &timeout
-		return nil
-	}
-}
-
 // NewConsole returns a new Console with the given options.
 func NewConsole(opts ...ConsoleOpt) (*Console, error) {
 	options := ConsoleOpts{

diff --git a/expect/expect.go b/expect/expect.go
@@ -19,7 +19,6 @@ import (
 	"bytes"
 	"fmt"
 	"io"
-	"time"
 	"unicode/utf8"
 )
 
@@ -59,11 +58,6 @@ func (c *Console) Expect(opts ...ExpectOpt) (string, error) {
 	writer := io.MultiWriter(append(c.opts.Stdouts, buf)...)
 	runeWriter := bufio.NewWriterSize(writer, utf8.UTFMax)
 
-	readTimeout := c.opts.ReadTimeout
-	if options.ReadTimeout != nil {
-		readTimeout = options.ReadTimeout
-	}
-
 	var matcher Matcher
 	var err error
 
@@ -78,13 +72,6 @@ func (c *Console) Expect(opts ...ExpectOpt) (string, error) {
 	}()
 
 	for {
-		if readTimeout != nil {
-			err = c.passthroughPipe.SetReadDeadline(time.Now().Add(*readTimeout))
-			if err != nil {
-				return buf.String(), err
-			}
-		}
-
 		var r rune
 		r, _, err = c.runeReader.ReadRune()
 		if err != nil {

diff --git a/expect/expect_test.go b/expect/expect_test.go
@@ -18,16 +18,11 @@ import (
 	"bufio"
 	"errors"
 	"fmt"
-	 "io"
-	// "io/ioutil"
-	//"log"
-	// "os"
-	// "os/exec"
+	"io"
 	"runtime/debug"
 	"strings"
 	"sync"
 	"testing"
-	"time"
 )
 
 var (
@@ -70,7 +65,6 @@ func newTestConsole(t *testing.T, opts ...ConsoleOpt) (*Console, error) {
 	opts = append([]ConsoleOpt{
 		expectNoError(t),
 		sendNoError(t),
-		WithDefaultTimeout(time.Second),
 	}, opts...)
 	return NewTestConsole(t, opts...)
 }
@@ -131,14 +125,12 @@ func TestExpectf(t *testing.T) {
 		c.SendLine("2")
 		c.Expectf("What is %s backwards?", "Netflix")
 		c.SendLine("xilfteN")
-		c.ExpectEOF()
 	}()
 
 	err = Prompt(c.InTty(), c.OutTty())
 	if err != nil {
 		t.Errorf("Expected no error but got '%s'", err)
 	}
-	testCloser(t, c)
 	wg.Wait()
 }
 
@@ -159,15 +151,12 @@ func TestExpect(t *testing.T) {
 		c.SendLine("2")
 		c.ExpectString("What is Netflix backwards?")
 		c.SendLine("xilfteN")
-		//c.ExpectEOF()
 	}()
 
 	err = Prompt(c.InTty(), c.OutTty())
 	if err != nil {
 		t.Errorf("Expected no error but got '%s'", err)
 	}
-	// close the pts so we can expect EOF
-	testCloser(t, c)
 	wg.Wait()
 }
 
@@ -187,174 +176,11 @@ func TestExpectOutput(t *testing.T) {
 		defer wg.Done()
 		c.ExpectString("What is 1+1?")
 		c.SendLine("3")
-		//c.ExpectEOF()
 	}()
 
 	err = Prompt(c.InTty(), c.OutTty())
 	if err == nil || err != ErrWrongAnswer {
 		t.Errorf("Expected error '%s' but got '%s' instead", ErrWrongAnswer, err)
 	}
-	testCloser(t, c)
 	wg.Wait()
 }
-
-// TODO: Needs to be updated to work on Windows
-// func TestExpectDefaultTimeout(t *testing.T) {
-// 	t.Parallel()
-
-// 	c, err := NewTestConsole(t, WithDefaultTimeout(0))
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got'%s'", err)
-// 	}
-// 	defer testCloser(t, c)
-
-// 	var wg sync.WaitGroup
-// 	wg.Add(1)
-// 	go func() {
-// 		defer wg.Done()
-// 		Prompt(c.InTty(), c.OutTty())
-// 	}()
-
-// 	_, err = c.ExpectString("What is 1+2?")
-// 	if err == nil || !strings.Contains(err.Error(), "i/o timeout") {
-// 		t.Errorf("Expected error to contain 'i/o timeout' but got '%s' instead", err)
-// 	}
-
-// 	//Close to unblock Prompt and wait for the goroutine to exit.
-// 	c.Close()
-// 	wg.Wait()
-// }
-
-// func TestExpectTimeout(t *testing.T) {
-// 	t.Parallel()
-
-// 	c, err := NewTestConsole(t)
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got'%s'", err)
-// 	}
-// 	defer testCloser(t, c)
-
-// 	var wg sync.WaitGroup
-// 	wg.Add(1)
-// 	go func() {
-// 		defer wg.Done()
-// 		Prompt(c.InTty(), c.OutTty())
-// 	}()
-
-// 	_, err = c.Expect(String("What is 1+2?"), WithTimeout(0))
-// 	if err == nil || !strings.Contains(err.Error(), "i/o timeout") {
-// 		t.Errorf("Expected error to contain 'i/o timeout' but got '%s' instead", err)
-// 	}
-
-// 	//Close to unblock Prompt and wait for the goroutine to exit.
-// 	c.Close()
-// 	wg.Wait()
-// }
-
-// func TestExpectDefaultTimeoutOverride(t *testing.T) {
-// 	t.Parallel()
-
-// 	c, err := newTestConsole(t, WithDefaultTimeout(100*time.Millisecond))
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got'%s'", err)
-// 	}
-// 	defer testCloser(t, c)
-
-// 	var wg sync.WaitGroup
-// 	wg.Add(1)
-// 	go func() {
-// 		defer wg.Done()
-// 		err = Prompt(c.InTty(), c.OutTty())
-// 		if err != nil {
-// 			t.Errorf("Expected no error but got '%s'", err)
-// 		}
-// 		time.Sleep(200 * time.Millisecond)
-// 		c.Close()
-// 	}()
-
-// 	c.ExpectString("What is 1+1?")
-// 	c.SendLine("2")
-// 	c.ExpectString("What is Netflix backwards?")
-// 	c.SendLine("xilfteN")
-// 	c.Expect(EOF, PTSClosed, WithTimeout(time.Second))
-
-// 	wg.Wait()
-// }
-
-// func TestEditor(t *testing.T) {
-// 	if _, err := exec.LookPath("vi"); err != nil {
-// 		t.Skip("vi not found in PATH")
-// 	}
-// 	t.Parallel()
-
-// 	c, err := NewConsole(expectNoError(t), sendNoError(t))
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got '%s'", err)
-// 	}
-// 	defer testCloser(t, c)
-
-// 	file, err := ioutil.TempFile("", "")
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got '%s'", err)
-// 	}
-
-// 	cmd := exec.Command("vi", file.Name())
-// 	cmd.Stdin = c.InTty()
-// 	cmd.Stdout = c.OutTty()
-// 	cmd.Stderr = c.OutTty()
-
-// 	var wg sync.WaitGroup
-// 	wg.Add(1)
-// 	go func() {
-// 		defer wg.Done()
-// 		c.Send("iHello world\x1b")
-// 		c.SendLine(":wq!")
-// 		c.ExpectEOF()
-// 	}()
-
-// 	err = cmd.Run()
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got '%s'", err)
-// 	}
-
-// 	testCloser(t, c)
-// 	wg.Wait()
-
-// 	data, err := ioutil.ReadFile(file.Name())
-// 	if err != nil {
-// 		t.Errorf("Expected no error but got '%s'", err)
-// 	}
-// 	if string(data) != "Hello world\n" {
-// 		t.Errorf("Expected '%s' to equal '%s'", string(data), "Hello world\n")
-// 	}
-// }
-
-// func ExampleConsole_echo() {
-// 	c, err := NewConsole(WithStdout(os.Stdout))
-// 	if err != nil {
-// 		log.Fatal(err)
-// 	}
-// 	defer c.Close()
-
-// 	cmd := exec.Command("echo")
-// 	cmd.Stdin = c.InTty()
-// 	cmd.Stdout = c.OutTty()
-// 	cmd.Stderr = c.OutTty()
-
-// 	err = cmd.Start()
-// 	if err != nil {
-// 		log.Fatal(err)
-// 	}
-
-// 	c.Send("Hello world")
-// 	c.ExpectString("Hello world")
-// 	c.Close()
-// 	c.ExpectEOF()
-
-// 	err = cmd.Wait()
-// 	if err != nil {
-// 		log.Fatal(err)
-// 	}
-
-	// Output: Hello world
-// }