Skip to content

Commit

Permalink
fix: Run expect tests on Windows with conpty pseudo-terminal (#276)
Browse files Browse the repository at this point in the history
This brings together a bunch of random, partially implemented packages for support of the new(ish) Windows [`conpty`](https://devblogs.microsoft.com/commandline/windows-command-line-introducing-the-windows-pseudo-console-conpty/) API - such that we can leverage the `expect` style of CLI tests, but in a way that works in Linux/OSX `pty`s and Windows `conpty`.

These include:
- Vendoring the `go-expect` library from Netflix w/ some tweaks to work cross-platform
- Vendoring the `pty` cross-platform implementation from [waypoint-plugin-sdk](https://github.com/hashicorp/waypoint-plugin-sdk/tree/b55c787a65ff9b7d2b32cfae80681b78f8f2275e/internal/pkg/pty)
- Vendoring the `conpty` Windows-specific implementation from [waypoint-plugin-sdk](https://github.com/hashicorp/waypoint-plugin-sdk/tree/b55c787a65ff9b7d2b32cfae80681b78f8f2275e/internal/pkg/conpty)
- Adjusting the `pty` interface to work with `go-expect` + the cross-plat version

There were several limitations with the current packages:
- `go-expect` requires the same `os.File` (TTY) for input / output, but `conhost` requires separate file handles
- `conpty` does not handle input, only output
- The cross-platform `pty` didn't expose the full set of primitives needed for `console`

Therefore, the following changes were made:
- Handling of `stdin` was added to the `conpty` interface
- We weren't using the full extent of the `go-expect` interface, so some portions were removed (ie, exec'ing a process) to simplify our implementation and make it easier to extend cross-platform
- Instead of `console` exposing just a `Tty`, it exposes an `InTty` and `OutTty`, to help encapsulate the difference on Windows (on Linux, these point to the same pipe)

Future improvements:
- The `isatty` implementation doesn't support accurate detection of `conhost` pty's without an associated process. In lieu of a more robust check, I've added a `--force-tty` flag intended for test case use - that forces the CLI to run in tty mode.
- It seems the windows implementation doesn't support setting a deadline. This is needed for the expect.Timeout API, but isn't used by us yet.

Fixes #241
  • Loading branch information
bryphe-coder committed Feb 15, 2022
1 parent 64c14de commit c9c0312
Show file tree
Hide file tree
Showing 19 changed files with 1,173 additions and 50 deletions.
34 changes: 0 additions & 34 deletions cli/clitest/clitest.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,13 @@ package clitest

import (
"archive/tar"
"bufio"
"bytes"
"errors"
"io"
"os"
"path/filepath"
"regexp"
"testing"

"github.com/Netflix/go-expect"
"github.com/spf13/cobra"
"github.com/stretchr/testify/require"

Expand All @@ -21,12 +18,6 @@ import (
"github.com/coder/coder/provisioner/echo"
)

var (
// Used to ensure terminal output doesn't have anything crazy!
// See: https://stackoverflow.com/a/29497680
stripAnsi = regexp.MustCompile("[\u001B\u009B][[\\]()#;?]*(?:(?:(?:[a-zA-Z\\d]*(?:;[a-zA-Z\\d]*)*)?\u0007)|(?:(?:\\d{1,4}(?:;\\d{0,4})*)?[\\dA-PRZcf-ntqry=><~]))")
)

// New creates a CLI instance with a configuration pointed to a
// temporary testing directory.
func New(t *testing.T, args ...string) (*cobra.Command, config.Root) {
Expand Down Expand Up @@ -55,31 +46,6 @@ func CreateProjectVersionSource(t *testing.T, responses *echo.Responses) string
return directory
}

// NewConsole creates a new TTY bound to the command provided.
// All ANSI escape codes are stripped to provide clean output.
func NewConsole(t *testing.T, cmd *cobra.Command) *expect.Console {
reader, writer := io.Pipe()
scanner := bufio.NewScanner(reader)
t.Cleanup(func() {
_ = reader.Close()
_ = writer.Close()
})
go func() {
for scanner.Scan() {
if scanner.Err() != nil {
return
}
t.Log(stripAnsi.ReplaceAllString(scanner.Text(), ""))
}
}()

console, err := expect.NewConsole(expect.WithStdout(writer))
require.NoError(t, err)
cmd.SetIn(console.Tty())
cmd.SetOut(console.Tty())
return console
}

func extractTar(t *testing.T, data []byte, directory string) {
reader := tar.NewReader(bytes.NewBuffer(data))
for {
Expand Down
5 changes: 2 additions & 3 deletions cli/clitest/clitest_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
//go:build !windows

package clitest_test

import (
"testing"

"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/expect"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
)
Expand All @@ -21,7 +20,7 @@ func TestCli(t *testing.T) {
client := coderdtest.New(t)
cmd, config := clitest.New(t)
clitest.SetupConfig(t, client, config)
console := clitest.NewConsole(t, cmd)
console := expect.NewTestConsole(t, cmd)
go func() {
err := cmd.Execute()
require.NoError(t, err)
Expand Down
3 changes: 2 additions & 1 deletion cli/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ func login() *cobra.Command {
Args: cobra.ExactArgs(1),
RunE: func(cmd *cobra.Command, args []string) error {
rawURL := args[0]

if !strings.HasPrefix(rawURL, "http://") && !strings.HasPrefix(rawURL, "https://") {
scheme := "https"
if strings.HasPrefix(rawURL, "localhost") {
Expand All @@ -44,7 +45,7 @@ func login() *cobra.Command {
return xerrors.Errorf("has initial user: %w", err)
}
if !hasInitialUser {
if !isTTY(cmd.InOrStdin()) {
if !isTTY(cmd) {
return xerrors.New("the initial user cannot be created in non-interactive mode. use the API")
}
_, _ = fmt.Fprintf(cmd.OutOrStdout(), "%s Your Coder deployment hasn't been set up!\n", color.HiBlackString(">"))
Expand Down
10 changes: 6 additions & 4 deletions cli/login_test.go
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
//go:build !windows

package cli_test

import (
"testing"

"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/expect"
"github.com/coder/coder/coderd/coderdtest"
"github.com/stretchr/testify/require"
)
Expand All @@ -23,8 +22,11 @@ func TestLogin(t *testing.T) {
t.Run("InitialUserTTY", func(t *testing.T) {
t.Parallel()
client := coderdtest.New(t)
root, _ := clitest.New(t, "login", client.URL.String())
console := clitest.NewConsole(t, root)
// The --force-tty flag is required on Windows, because the `isatty` library does not
// accurately detect Windows ptys when they are not attached to a process:
// https://github.com/mattn/go-isatty/issues/59
root, _ := clitest.New(t, "login", client.URL.String(), "--force-tty")
console := expect.NewTestConsole(t, root)
go func() {
err := root.Execute()
require.NoError(t, err)
Expand Down
7 changes: 3 additions & 4 deletions cli/projectcreate_test.go
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
//go:build !windows

package cli_test

import (
Expand All @@ -10,6 +8,7 @@ import (
"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/database"
"github.com/coder/coder/expect"
"github.com/coder/coder/provisioner/echo"
"github.com/coder/coder/provisionersdk/proto"
)
Expand All @@ -27,7 +26,7 @@ func TestProjectCreate(t *testing.T) {
cmd, root := clitest.New(t, "projects", "create", "--directory", source, "--provisioner", string(database.ProvisionerTypeEcho))
clitest.SetupConfig(t, client, root)
_ = coderdtest.NewProvisionerDaemon(t, client)
console := clitest.NewConsole(t, cmd)
console := expect.NewTestConsole(t, cmd)
closeChan := make(chan struct{})
go func() {
err := cmd.Execute()
Expand Down Expand Up @@ -74,7 +73,7 @@ func TestProjectCreate(t *testing.T) {
cmd, root := clitest.New(t, "projects", "create", "--directory", source, "--provisioner", string(database.ProvisionerTypeEcho))
clitest.SetupConfig(t, client, root)
coderdtest.NewProvisionerDaemon(t, client)
console := clitest.NewConsole(t, cmd)
console := expect.NewTestConsole(t, cmd)
closeChan := make(chan struct{})
go func() {
err := cmd.Execute()
Expand Down
18 changes: 17 additions & 1 deletion cli/root.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import (

const (
varGlobalConfig = "global-config"
varForceTty = "force-tty"
)

func Root() *cobra.Command {
Expand Down Expand Up @@ -65,6 +66,12 @@ func Root() *cobra.Command {
cmd.AddCommand(users())

cmd.PersistentFlags().String(varGlobalConfig, configdir.LocalConfig("coder"), "Path to the global `coder` config directory")
cmd.PersistentFlags().Bool(varForceTty, false, "Force the `coder` command to run as if connected to a TTY")
err := cmd.PersistentFlags().MarkHidden(varForceTty)
if err != nil {
// This should never return an error, because we just added the `--force-tty`` flag prior to calling MarkHidden.
panic(err)
}

return cmd
}
Expand Down Expand Up @@ -113,7 +120,16 @@ func createConfig(cmd *cobra.Command) config.Root {
// isTTY returns whether the passed reader is a TTY or not.
// This accepts a reader to work with Cobra's "InOrStdin"
// function for simple testing.
func isTTY(reader io.Reader) bool {
func isTTY(cmd *cobra.Command) bool {
// If the `--force-tty` command is available, and set,
// assume we're in a tty. This is primarily for cases on Windows
// where we may not be able to reliably detect this automatically (ie, tests)
forceTty, err := cmd.Flags().GetBool(varForceTty)
if forceTty && err == nil {
return true
}

reader := cmd.InOrStdin()
file, ok := reader.(*os.File)
if !ok {
return false
Expand Down
5 changes: 2 additions & 3 deletions cli/workspacecreate_test.go
Original file line number Diff line number Diff line change
@@ -1,12 +1,11 @@
//go:build !windows

package cli_test

import (
"testing"

"github.com/coder/coder/cli/clitest"
"github.com/coder/coder/coderd/coderdtest"
"github.com/coder/coder/expect"
"github.com/coder/coder/provisioner/echo"
"github.com/coder/coder/provisionersdk/proto"
"github.com/stretchr/testify/require"
Expand Down Expand Up @@ -37,7 +36,7 @@ func TestWorkspaceCreate(t *testing.T) {
cmd, root := clitest.New(t, "workspaces", "create", project.Name)
clitest.SetupConfig(t, client, root)

console := clitest.NewConsole(t, cmd)
console := expect.NewTestConsole(t, cmd)
closeChan := make(chan struct{})
go func() {
err := cmd.Execute()
Expand Down
107 changes: 107 additions & 0 deletions expect/conpty/conpty.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
//go:build windows
// +build windows

// Original copyright 2020 ActiveState Software. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file

package conpty

import (
"fmt"
"io"
"os"

"golang.org/x/sys/windows"
)

// ConPty represents a windows pseudo console.
type ConPty struct {
hpCon windows.Handle
outPipePseudoConsoleSide windows.Handle
outPipeOurSide windows.Handle
inPipeOurSide windows.Handle
inPipePseudoConsoleSide windows.Handle
consoleSize uintptr
outFilePseudoConsoleSide *os.File
outFileOurSide *os.File
inFilePseudoConsoleSide *os.File
inFileOurSide *os.File
closed bool
}

// New returns a new ConPty pseudo terminal device
func New(columns int16, rows int16) (*ConPty, error) {
c := &ConPty{
consoleSize: uintptr(columns) + (uintptr(rows) << 16),
}

return c, c.createPseudoConsoleAndPipes()
}

// Close closes the pseudo-terminal and cleans up all attached resources
func (c *ConPty) Close() error {
// Trying to close these pipes multiple times will result in an
// access violation
if c.closed {
return nil
}

err := closePseudoConsole(c.hpCon)
c.outFilePseudoConsoleSide.Close()
c.outFileOurSide.Close()
c.inFilePseudoConsoleSide.Close()
c.inFileOurSide.Close()
c.closed = true
return err
}

// OutPipe returns the output pipe of the pseudo terminal
func (c *ConPty) OutPipe() *os.File {
return c.outFilePseudoConsoleSide
}

func (c *ConPty) Reader() io.Reader {
return c.outFileOurSide
}

// 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.inFilePseudoConsoleSide
}

func (c *ConPty) WriteString(str string) (int, error) {
return c.inFileOurSide.WriteString(str)
}

func (c *ConPty) createPseudoConsoleAndPipes() error {
// Create the stdin pipe
if err := windows.CreatePipe(&c.inPipePseudoConsoleSide, &c.inPipeOurSide, nil, 0); err != nil {
return err
}

// Create the stdout pipe
if err := windows.CreatePipe(&c.outPipeOurSide, &c.outPipePseudoConsoleSide, nil, 0); err != nil {
return err
}

// Create the pty with our stdin/stdout
if err := createPseudoConsole(c.consoleSize, c.inPipePseudoConsoleSide, c.outPipePseudoConsoleSide, &c.hpCon); err != nil {
return fmt.Errorf("failed to create pseudo console: %d, %v", uintptr(c.hpCon), err)
}

c.outFilePseudoConsoleSide = os.NewFile(uintptr(c.outPipePseudoConsoleSide), "|0")
c.outFileOurSide = os.NewFile(uintptr(c.outPipeOurSide), "|1")

c.inFilePseudoConsoleSide = os.NewFile(uintptr(c.inPipePseudoConsoleSide), "|2")
c.inFileOurSide = os.NewFile(uintptr(c.inPipeOurSide), "|3")
c.closed = false

return nil
}

func (c *ConPty) Resize(cols uint16, rows uint16) error {
return resizePseudoConsole(c.hpCon, uintptr(cols)+(uintptr(rows)<<16))
}
53 changes: 53 additions & 0 deletions expect/conpty/syscall.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
//go:build windows
// +build windows

// Copyright 2020 ActiveState Software. All rights reserved.
// Use of this source code is governed by a BSD-style
// license that can be found in the LICENSE file

package conpty

import (
"unsafe"

"golang.org/x/sys/windows"
)

var (
kernel32 = windows.NewLazySystemDLL("kernel32.dll")
procResizePseudoConsole = kernel32.NewProc("ResizePseudoConsole")
procCreatePseudoConsole = kernel32.NewProc("CreatePseudoConsole")
procClosePseudoConsole = kernel32.NewProc("ClosePseudoConsole")
)

func createPseudoConsole(consoleSize uintptr, ptyIn windows.Handle, ptyOut windows.Handle, hpCon *windows.Handle) (err error) {
r1, _, e1 := procCreatePseudoConsole.Call(
consoleSize,
uintptr(ptyIn),
uintptr(ptyOut),
0,
uintptr(unsafe.Pointer(hpCon)),
)

if r1 != 0 { // !S_OK
err = e1
}
return
}

func resizePseudoConsole(handle windows.Handle, consoleSize uintptr) (err error) {
r1, _, e1 := procResizePseudoConsole.Call(uintptr(handle), consoleSize)
if r1 != 0 { // !S_OK
err = e1
}
return
}

func closePseudoConsole(handle windows.Handle) (err error) {
r1, _, e1 := procClosePseudoConsole.Call(uintptr(handle))
if r1 == 0 {
err = e1
}

return
}

0 comments on commit c9c0312

Please sign in to comment.