fix: Guard against CLI cmd running after test exit (#1658)

* fix: Guard against CLI cmd running after test exit

* fix: cli: avoid calling t.FailNow in non-test-main goroutine

* fix: cli: server_test: avoid calling t.FailNow outside main goroutine

* fix: cli: clitest_test: avoid calling t.FailNow outside main goroutine

* fix: cli: list_test: avoid calling t.FailNow outside main goroutine

* fix: TestGitSSH use-of-t-after-exit

* fix: TestGitSSH "too many authentication failures"

Due to local SSH keys being given

* chore: clitest: fix TestCli

* chore: Simplify TestTemplateInit

Co-authored-by: Cian Johnston <cian@coder.com>
This commit is contained in:
Mathias Fredriksson 2022-05-23 20:09:58 +03:00 committed by GitHub
parent fa957d6d65
commit c8ed213347
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
7 changed files with 87 additions and 86 deletions

View File

@ -49,13 +49,12 @@ func TestWorkspaceAgent(t *testing.T) {
cmd, _ := clitest.New(t, "agent", "--auth", "azure-instance-identity", "--agent-url", client.URL.String())
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
errC := make(chan error)
go func() {
// A linting error occurs for weakly typing the context value here,
// but it seems reasonable for a one-off test.
// nolint
ctx = context.WithValue(ctx, "azure-client", metadataClient)
err := cmd.ExecuteContext(ctx)
require.NoError(t, err)
// A linting error occurs for weakly typing the context value here.
//nolint // The above seems reasonable for a one-off test.
ctx := context.WithValue(ctx, "azure-client", metadataClient)
errC <- cmd.ExecuteContext(ctx)
}()
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
@ -66,6 +65,8 @@ func TestWorkspaceAgent(t *testing.T) {
_, err = dialer.Ping()
require.NoError(t, err)
cancelFunc()
err = <-errC
require.NoError(t, err)
})
t.Run("AWS", func(t *testing.T) {
@ -103,13 +104,12 @@ func TestWorkspaceAgent(t *testing.T) {
cmd, _ := clitest.New(t, "agent", "--auth", "aws-instance-identity", "--agent-url", client.URL.String())
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
errC := make(chan error)
go func() {
// A linting error occurs for weakly typing the context value here,
// but it seems reasonable for a one-off test.
// nolint
ctx = context.WithValue(ctx, "aws-client", metadataClient)
err := cmd.ExecuteContext(ctx)
require.NoError(t, err)
// A linting error occurs for weakly typing the context value here.
//nolint // The above seems reasonable for a one-off test.
ctx := context.WithValue(ctx, "aws-client", metadataClient)
errC <- cmd.ExecuteContext(ctx)
}()
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
@ -120,6 +120,8 @@ func TestWorkspaceAgent(t *testing.T) {
_, err = dialer.Ping()
require.NoError(t, err)
cancelFunc()
err = <-errC
require.NoError(t, err)
})
t.Run("GoogleCloud", func(t *testing.T) {
@ -157,13 +159,12 @@ func TestWorkspaceAgent(t *testing.T) {
cmd, _ := clitest.New(t, "agent", "--auth", "google-instance-identity", "--agent-url", client.URL.String())
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
errC := make(chan error)
go func() {
// A linting error occurs for weakly typing the context value here,
// but it seems reasonable for a one-off test.
// nolint
ctx = context.WithValue(ctx, "gcp-client", metadata)
err := cmd.ExecuteContext(ctx)
require.NoError(t, err)
// A linting error occurs for weakly typing the context value here.
//nolint // The above seems reasonable for a one-off test.
ctx := context.WithValue(ctx, "gcp-client", metadata)
errC <- cmd.ExecuteContext(ctx)
}()
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
resources, err := client.WorkspaceResourcesByBuild(ctx, workspace.LatestBuild.ID)
@ -174,5 +175,7 @@ func TestWorkspaceAgent(t *testing.T) {
_, err = dialer.Ping()
require.NoError(t, err)
cancelFunc()
err = <-errC
require.NoError(t, err)
})
}

View File

@ -3,7 +3,6 @@ package clitest_test
import (
"testing"
"github.com/stretchr/testify/require"
"go.uber.org/goleak"
"github.com/coder/coder/cli/clitest"
@ -25,8 +24,7 @@ func TestCli(t *testing.T) {
cmd.SetIn(pty.Input())
cmd.SetOut(pty.Output())
go func() {
err := cmd.Execute()
require.NoError(t, err)
_ = cmd.Execute()
}()
pty.ExpectMatch("coder")
}

View File

@ -63,9 +63,9 @@ func TestGitSSH(t *testing.T) {
clitest.SetupConfig(t, agentClient, root)
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
agentErrC := make(chan error)
go func() {
err := cmd.ExecuteContext(ctx)
require.NoError(t, err)
agentErrC <- cmd.ExecuteContext(ctx)
}()
coderdtest.AwaitWorkspaceAgents(t, client, workspace.LatestBuild.ID)
@ -85,13 +85,13 @@ func TestGitSSH(t *testing.T) {
return ssh.KeysEqual(publicKey, key)
})
var inc int64
sshErrC := make(chan error)
go func() {
// as long as we get a successful session we don't care if the server errors
_ = ssh.Serve(l, func(s ssh.Session) {
atomic.AddInt64(&inc, 1)
t.Log("got authenticated session")
err := s.Exit(0)
require.NoError(t, err)
sshErrC <- s.Exit(0)
}, publicKeyOption)
}()
@ -99,9 +99,16 @@ func TestGitSSH(t *testing.T) {
addr, ok := l.Addr().(*net.TCPAddr)
require.True(t, ok)
// set to agent config dir
cmd, _ = clitest.New(t, "gitssh", "--agent-url", agentClient.URL.String(), "--agent-token", agentToken, "--", fmt.Sprintf("-p%d", addr.Port), "-o", "StrictHostKeyChecking=no", "127.0.0.1")
err = cmd.ExecuteContext(context.Background())
gitsshCmd, _ := clitest.New(t, "gitssh", "--agent-url", agentClient.URL.String(), "--agent-token", agentToken, "--", fmt.Sprintf("-p%d", addr.Port), "-o", "StrictHostKeyChecking=no", "-o", "IdentitiesOnly=yes", "127.0.0.1")
err = gitsshCmd.ExecuteContext(context.Background())
require.NoError(t, err)
require.EqualValues(t, 1, inc)
err = <-sshErrC
require.NoError(t, err, "error in ssh session exit")
cancelFunc()
err = <-agentErrC
require.NoError(t, err, "error in agent execute")
})
}

View File

@ -1,7 +1,9 @@
package cli_test
import (
"context"
"testing"
"time"
"github.com/stretchr/testify/require"
@ -14,6 +16,8 @@ func TestList(t *testing.T) {
t.Parallel()
t.Run("Single", func(t *testing.T) {
t.Parallel()
ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second)
defer cancelFunc()
client := coderdtest.New(t, &coderdtest.Options{IncludeProvisionerD: true})
user := coderdtest.CreateFirstUser(t, client)
version := coderdtest.CreateTemplateVersion(t, client, user.OrganizationID, nil)
@ -23,17 +27,16 @@ func TestList(t *testing.T) {
coderdtest.AwaitWorkspaceBuildJob(t, client, workspace.LatestBuild.ID)
cmd, root := clitest.New(t, "ls")
clitest.SetupConfig(t, client, root)
doneChan := make(chan struct{})
pty := ptytest.New(t)
cmd.SetIn(pty.Input())
cmd.SetOut(pty.Output())
errC := make(chan error)
go func() {
defer close(doneChan)
err := cmd.Execute()
require.NoError(t, err)
errC <- cmd.ExecuteContext(ctx)
}()
pty.ExpectMatch(workspace.Name)
pty.ExpectMatch("Running")
<-doneChan
cancelFunc()
require.NoError(t, <-errC)
})
}

View File

@ -17,7 +17,6 @@ import (
"os"
"runtime"
"strings"
"sync"
"testing"
"time"
@ -45,12 +44,11 @@ func TestServer(t *testing.T) {
require.NoError(t, err)
defer closeFunc()
ctx, cancelFunc := context.WithCancel(context.Background())
done := make(chan struct{})
defer cancelFunc()
root, cfg := clitest.New(t, "server", "--address", ":0", "--postgres-url", connectionURL)
errC := make(chan error)
go func() {
defer close(done)
err = root.ExecuteContext(ctx)
require.ErrorIs(t, err, context.Canceled)
errC <- root.ExecuteContext(ctx)
}()
var client *codersdk.Client
require.Eventually(t, func() bool {
@ -71,8 +69,9 @@ func TestServer(t *testing.T) {
})
require.NoError(t, err)
cancelFunc()
<-done
require.ErrorIs(t, <-errC, context.Canceled)
})
t.Run("Development", func(t *testing.T) {
t.Parallel()
ctx, cancelFunc := context.WithCancel(context.Background())
@ -82,26 +81,12 @@ func TestServer(t *testing.T) {
root, cfg := clitest.New(t, "server", "--dev", "--tunnel=false", "--address", ":0")
var buf strings.Builder
errC := make(chan error)
root.SetOutput(&buf)
var wg sync.WaitGroup
wg.Add(1)
go func() {
defer wg.Done()
err := root.ExecuteContext(ctx)
require.ErrorIs(t, err, context.Canceled)
// Verify that credentials were output to the terminal.
assert.Contains(t, buf.String(), fmt.Sprintf("email: %s", wantEmail), "expected output %q; got no match", wantEmail)
// Check that the password line is output and that it's non-empty.
if _, after, found := strings.Cut(buf.String(), "password: "); found {
before, _, _ := strings.Cut(after, "\n")
before = strings.Trim(before, "\r") // Ensure no control character is left.
assert.NotEmpty(t, before, "expected non-empty password; got empty")
} else {
t.Error("expected password line output; got no match")
}
errC <- root.ExecuteContext(ctx)
}()
var token string
require.Eventually(t, func() bool {
var err error
@ -119,8 +104,20 @@ func TestServer(t *testing.T) {
require.NoError(t, err)
cancelFunc()
wg.Wait()
require.ErrorIs(t, <-errC, context.Canceled)
// Verify that credentials were output to the terminal.
assert.Contains(t, buf.String(), fmt.Sprintf("email: %s", wantEmail), "expected output %q; got no match", wantEmail)
// Check that the password line is output and that it's non-empty.
if _, after, found := strings.Cut(buf.String(), "password: "); found {
before, _, _ := strings.Cut(after, "\n")
before = strings.Trim(before, "\r") // Ensure no control character is left.
assert.NotEmpty(t, before, "expected non-empty password; got empty")
} else {
t.Error("expected password line output; got no match")
}
})
// Duplicated test from "Development" above to test setting email/password via env.
// Cannot run parallel due to os.Setenv.
//nolint:paralleltest
@ -136,18 +133,11 @@ func TestServer(t *testing.T) {
root, cfg := clitest.New(t, "server", "--dev", "--tunnel=false", "--address", ":0")
var buf strings.Builder
root.SetOutput(&buf)
var wg sync.WaitGroup
wg.Add(1)
errC := make(chan error)
go func() {
defer wg.Done()
err := root.ExecuteContext(ctx)
require.ErrorIs(t, err, context.Canceled)
// Verify that credentials were output to the terminal.
assert.Contains(t, buf.String(), fmt.Sprintf("email: %s", wantEmail), "expected output %q; got no match", wantEmail)
assert.Contains(t, buf.String(), fmt.Sprintf("password: %s", wantPassword), "expected output %q; got no match", wantPassword)
errC <- root.ExecuteContext(ctx)
}()
var token string
require.Eventually(t, func() bool {
var err error
@ -165,8 +155,12 @@ func TestServer(t *testing.T) {
require.NoError(t, err)
cancelFunc()
wg.Wait()
require.ErrorIs(t, <-errC, context.Canceled)
// Verify that credentials were output to the terminal.
assert.Contains(t, buf.String(), fmt.Sprintf("email: %s", wantEmail), "expected output %q; got no match", wantEmail)
assert.Contains(t, buf.String(), fmt.Sprintf("password: %s", wantPassword), "expected output %q; got no match", wantPassword)
})
t.Run("TLSBadVersion", func(t *testing.T) {
t.Parallel()
ctx, cancelFunc := context.WithCancel(context.Background())
@ -202,10 +196,12 @@ func TestServer(t *testing.T) {
certPath, keyPath := generateTLSCertificate(t)
root, cfg := clitest.New(t, "server", "--dev", "--tunnel=false", "--address", ":0",
"--tls-enable", "--tls-cert-file", certPath, "--tls-key-file", keyPath)
errC := make(chan error)
go func() {
err := root.ExecuteContext(ctx)
require.ErrorIs(t, err, context.Canceled)
errC <- root.ExecuteContext(ctx)
}()
// Verify HTTPS
var accessURLRaw string
require.Eventually(t, func() bool {
var err error
@ -226,6 +222,9 @@ func TestServer(t *testing.T) {
}
_, err = client.HasFirstUser(ctx)
require.NoError(t, err)
cancelFunc()
require.ErrorIs(t, <-errC, context.Canceled)
})
// This cannot be ran in parallel because it uses a signal.
//nolint:paralleltest
@ -284,14 +283,12 @@ func TestServer(t *testing.T) {
ctx, cancelFunc := context.WithCancel(context.Background())
defer cancelFunc()
root, _ := clitest.New(t, "server", "--dev", "--tunnel=false", "--address", ":0", "--trace=true")
done := make(chan struct{})
errC := make(chan error)
go func() {
defer close(done)
err := root.ExecuteContext(ctx)
require.ErrorIs(t, err, context.Canceled)
errC <- root.ExecuteContext(ctx)
}()
cancelFunc()
<-done
require.ErrorIs(t, <-errC, context.Canceled)
require.Error(t, goleak.Find())
})
}

View File

@ -16,16 +16,11 @@ func TestTemplateInit(t *testing.T) {
t.Parallel()
tempDir := t.TempDir()
cmd, _ := clitest.New(t, "templates", "init", tempDir)
doneChan := make(chan struct{})
pty := ptytest.New(t)
cmd.SetIn(pty.Input())
cmd.SetOut(pty.Output())
go func() {
defer close(doneChan)
err := cmd.Execute()
require.NoError(t, err)
}()
<-doneChan
err := cmd.Execute()
require.NoError(t, err)
files, err := os.ReadDir(tempDir)
require.NoError(t, err)
require.Greater(t, len(files), 0)

View File

@ -16,15 +16,13 @@ func TestUserList(t *testing.T) {
coderdtest.CreateFirstUser(t, client)
cmd, root := clitest.New(t, "users", "list")
clitest.SetupConfig(t, client, root)
doneChan := make(chan struct{})
pty := ptytest.New(t)
cmd.SetIn(pty.Input())
cmd.SetOut(pty.Output())
errC := make(chan error)
go func() {
defer close(doneChan)
err := cmd.Execute()
require.NoError(t, err)
errC <- cmd.Execute()
}()
require.NoError(t, <-errC)
pty.ExpectMatch("coder.com")
<-doneChan
}