mirror of
https://github.com/coder/coder.git
synced 2026-09-24 15:04:27 +08:00
fix: Run expect tests on Windows with conpty pseudo-terminal (#276)
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
This commit is contained in:
@@ -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"
|
||||
|
||||
@@ -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) {
|
||||
@@ -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 {
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
//go:build !windows
|
||||
|
||||
package clitest_test
|
||||
|
||||
import (
|
||||
@@ -7,6 +5,7 @@ import (
|
||||
|
||||
"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"
|
||||
)
|
||||
@@ -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)
|
||||
|
||||
+2
-1
@@ -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") {
|
||||
@@ -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(">"))
|
||||
|
||||
+6
-4
@@ -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"
|
||||
)
|
||||
@@ -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)
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
//go:build !windows
|
||||
|
||||
package cli_test
|
||||
|
||||
import (
|
||||
@@ -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"
|
||||
)
|
||||
@@ -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()
|
||||
@@ -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()
|
||||
|
||||
+17
-1
@@ -21,6 +21,7 @@ import (
|
||||
|
||||
const (
|
||||
varGlobalConfig = "global-config"
|
||||
varForceTty = "force-tty"
|
||||
)
|
||||
|
||||
func Root() *cobra.Command {
|
||||
@@ -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
|
||||
}
|
||||
@@ -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
|
||||
|
||||
@@ -1,5 +1,3 @@
|
||||
//go:build !windows
|
||||
|
||||
package cli_test
|
||||
|
||||
import (
|
||||
@@ -7,6 +5,7 @@ import (
|
||||
|
||||
"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"
|
||||
@@ -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()
|
||||
|
||||
Reference in New Issue
Block a user