diff --git a/cli/agent_test.go b/cli/agent_test.go index 71bc7b1d6949c..6058b601bc2dc 100644 --- a/cli/agent_test.go +++ b/cli/agent_test.go @@ -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) }) } diff --git a/cli/clitest/clitest_test.go b/cli/clitest/clitest_test.go index a71c220783fe9..441b84048d05e 100644 --- a/cli/clitest/clitest_test.go +++ b/cli/clitest/clitest_test.go @@ -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") } diff --git a/cli/gitssh_test.go b/cli/gitssh_test.go index 084407c9e3a72..999c6ce7314df 100644 --- a/cli/gitssh_test.go +++ b/cli/gitssh_test.go @@ -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") }) } diff --git a/cli/list_test.go b/cli/list_test.go index 76408ea3f1f33..994b364dd7489 100644 --- a/cli/list_test.go +++ b/cli/list_test.go @@ -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) }) } diff --git a/cli/server_test.go b/cli/server_test.go index 1f2525975a836..cd67bc6df4ac4 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -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()) }) } diff --git a/cli/templateinit_test.go b/cli/templateinit_test.go index a5fada4310906..46969ea7c287a 100644 --- a/cli/templateinit_test.go +++ b/cli/templateinit_test.go @@ -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) diff --git a/cli/userlist_test.go b/cli/userlist_test.go index 37baed59d3d67..76f9488ec996a 100644 --- a/cli/userlist_test.go +++ b/cli/userlist_test.go @@ -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 }