From 648485ea1783e7571a4bd6c2e7f58484415d4394 Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 12 Dec 2022 10:00:14 +0000 Subject: [PATCH 1/2] chore: Refactor agent tests to avoid `t.Run` when not needed It turns out that writing tests that contain subtests should probably be limited to table-based tests and tests that share a common setup shared between tests. Writing tests with a subtest like this: ``` func TestSomething(t *testing.T) { t.Run("Subtest", func(t *testing.t) {}) } ``` Has the following disadvantages: - It can lead to multiple tests failing with `(unknown)` status when only one of the subtests hang (never exit) - In Go 1.20rc1, using `t.Setenv` is no longer allowed if the parent test is parallel --- agent/agent_test.go | 1156 +++++++++++++++++++-------------------- agent/apphealth_test.go | 257 +++++---- 2 files changed, 704 insertions(+), 709 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index f54a699f654af..dfeeb76fec8d4 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -52,656 +52,649 @@ func TestMain(m *testing.M) { goleak.VerifyTestMain(m) } -func TestAgent(t *testing.T) { +func TestAgent_Stats_SSH(t *testing.T) { t.Parallel() - t.Run("Stats", func(t *testing.T) { - t.Parallel() - - t.Run("SSH", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - conn, stats, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() - sshClient, err := conn.SSHClient(ctx) - require.NoError(t, err) - defer sshClient.Close() - session, err := sshClient.NewSession() - require.NoError(t, err) - defer session.Close() - require.NoError(t, session.Run("echo test")) - - var s *codersdk.AgentStats - require.Eventuallyf(t, func() bool { - var ok bool - s, ok = <-stats - return ok && s.NumConns > 0 && s.RxBytes > 0 && s.TxBytes > 0 - }, testutil.WaitLong, testutil.IntervalFast, - "never saw stats: %+v", s, - ) - }) + conn, stats, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) - t.Run("ReconnectingPTY", func(t *testing.T) { - t.Parallel() + sshClient, err := conn.SSHClient(ctx) + require.NoError(t, err) + defer sshClient.Close() + session, err := sshClient.NewSession() + require.NoError(t, err) + defer session.Close() + require.NoError(t, session.Run("echo test")) + + var s *codersdk.AgentStats + require.Eventuallyf(t, func() bool { + var ok bool + s, ok = <-stats + return ok && s.NumConns > 0 && s.RxBytes > 0 && s.TxBytes > 0 + }, testutil.WaitLong, testutil.IntervalFast, + "never saw stats: %+v", s, + ) +} - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() +func TestAgent_Stats_ReconnectingPTY(t *testing.T) { + t.Parallel() - conn, stats, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() - ptyConn, err := conn.ReconnectingPTY(ctx, uuid.New(), 128, 128, "/bin/bash") - require.NoError(t, err) - defer ptyConn.Close() + conn, stats, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) - data, err := json.Marshal(codersdk.ReconnectingPTYRequest{ - Data: "echo test\r\n", - }) - require.NoError(t, err) - _, err = ptyConn.Write(data) - require.NoError(t, err) + ptyConn, err := conn.ReconnectingPTY(ctx, uuid.New(), 128, 128, "/bin/bash") + require.NoError(t, err) + defer ptyConn.Close() - var s *codersdk.AgentStats - require.Eventuallyf(t, func() bool { - var ok bool - s, ok = <-stats - return ok && s.NumConns > 0 && s.RxBytes > 0 && s.TxBytes > 0 - }, testutil.WaitLong, testutil.IntervalFast, - "never saw stats: %+v", s, - ) - }) + data, err := json.Marshal(codersdk.ReconnectingPTYRequest{ + Data: "echo test\r\n", }) + require.NoError(t, err) + _, err = ptyConn.Write(data) + require.NoError(t, err) - t.Run("SessionExec", func(t *testing.T) { - t.Parallel() - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) + var s *codersdk.AgentStats + require.Eventuallyf(t, func() bool { + var ok bool + s, ok = <-stats + return ok && s.NumConns > 0 && s.RxBytes > 0 && s.TxBytes > 0 + }, testutil.WaitLong, testutil.IntervalFast, + "never saw stats: %+v", s, + ) +} - command := "echo test" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo test" - } - output, err := session.Output(command) - require.NoError(t, err) - require.Equal(t, "test", strings.TrimSpace(string(output))) - }) +func TestAgent_SessionExec(t *testing.T) { + t.Parallel() + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) - t.Run("GitSSH", func(t *testing.T) { - t.Parallel() - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) - command := "sh -c 'echo $GIT_SSH_COMMAND'" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo %GIT_SSH_COMMAND%" - } - output, err := session.Output(command) - require.NoError(t, err) - require.True(t, strings.HasSuffix(strings.TrimSpace(string(output)), "gitssh --")) - }) + command := "echo test" + if runtime.GOOS == "windows" { + command = "cmd.exe /c echo test" + } + output, err := session.Output(command) + require.NoError(t, err) + require.Equal(t, "test", strings.TrimSpace(string(output))) +} - t.Run("SessionTTYShell", func(t *testing.T) { - t.Parallel() - if runtime.GOOS == "windows" { - // This might be our implementation, or ConPTY itself. - // It's difficult to find extensive tests for it, so - // it seems like it could be either. - t.Skip("ConPTY appears to be inconsistent on Windows.") - } - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) - command := "bash" - if runtime.GOOS == "windows" { - command = "cmd.exe" - } - err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) - require.NoError(t, err) - ptty := ptytest.New(t) - session.Stdout = ptty.Output() - session.Stderr = ptty.Output() - session.Stdin = ptty.Input() - err = session.Start(command) - require.NoError(t, err) - caret := "$" - if runtime.GOOS == "windows" { - caret = ">" - } - ptty.ExpectMatch(caret) - ptty.WriteLine("echo test") - ptty.ExpectMatch("test") - ptty.WriteLine("exit") - err = session.Wait() - require.NoError(t, err) - }) +func TestAgent_GitSSH(t *testing.T) { + t.Parallel() + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) + command := "sh -c 'echo $GIT_SSH_COMMAND'" + if runtime.GOOS == "windows" { + command = "cmd.exe /c echo %GIT_SSH_COMMAND%" + } + output, err := session.Output(command) + require.NoError(t, err) + require.True(t, strings.HasSuffix(strings.TrimSpace(string(output)), "gitssh --")) +} - t.Run("SessionTTYExitCode", func(t *testing.T) { - t.Parallel() - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) - command := "areallynotrealcommand" - err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) - require.NoError(t, err) - ptty := ptytest.New(t) - session.Stdout = ptty.Output() - session.Stderr = ptty.Output() - session.Stdin = ptty.Input() - err = session.Start(command) - require.NoError(t, err) - err = session.Wait() - exitErr := &ssh.ExitError{} - require.True(t, xerrors.As(err, &exitErr)) - if runtime.GOOS == "windows" { - assert.Equal(t, 1, exitErr.ExitStatus()) - } else { - assert.Equal(t, 127, exitErr.ExitStatus()) - } - }) +func TestAgent_SessionTTYShell(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + // This might be our implementation, or ConPTY itself. + // It's difficult to find extensive tests for it, so + // it seems like it could be either. + t.Skip("ConPTY appears to be inconsistent on Windows.") + } + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) + command := "bash" + if runtime.GOOS == "windows" { + command = "cmd.exe" + } + err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) + require.NoError(t, err) + ptty := ptytest.New(t) + session.Stdout = ptty.Output() + session.Stderr = ptty.Output() + session.Stdin = ptty.Input() + err = session.Start(command) + require.NoError(t, err) + caret := "$" + if runtime.GOOS == "windows" { + caret = ">" + } + ptty.ExpectMatch(caret) + ptty.WriteLine("echo test") + ptty.ExpectMatch("test") + ptty.WriteLine("exit") + err = session.Wait() + require.NoError(t, err) +} - //nolint:paralleltest // This test sets an environment variable. - t.Run("Session TTY MOTD", func(t *testing.T) { - if runtime.GOOS == "windows" { - // This might be our implementation, or ConPTY itself. - // It's difficult to find extensive tests for it, so - // it seems like it could be either. - t.Skip("ConPTY appears to be inconsistent on Windows.") - } +func TestAgent_SessionTTYExitCode(t *testing.T) { + t.Parallel() + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) + command := "areallynotrealcommand" + err := session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) + require.NoError(t, err) + ptty := ptytest.New(t) + session.Stdout = ptty.Output() + session.Stderr = ptty.Output() + session.Stdin = ptty.Input() + err = session.Start(command) + require.NoError(t, err) + err = session.Wait() + exitErr := &ssh.ExitError{} + require.True(t, xerrors.As(err, &exitErr)) + if runtime.GOOS == "windows" { + assert.Equal(t, 1, exitErr.ExitStatus()) + } else { + assert.Equal(t, 127, exitErr.ExitStatus()) + } +} + +//nolint:paralleltest // This test sets an environment variable. +func TestAgent_Session_TTY_MOTD(t *testing.T) { + if runtime.GOOS == "windows" { + // This might be our implementation, or ConPTY itself. + // It's difficult to find extensive tests for it, so + // it seems like it could be either. + t.Skip("ConPTY appears to be inconsistent on Windows.") + } - wantMOTD := "Welcome to your Coder workspace!" + wantMOTD := "Welcome to your Coder workspace!" - tmpdir := t.TempDir() - name := filepath.Join(tmpdir, "motd") - err := os.WriteFile(name, []byte(wantMOTD), 0o600) - require.NoError(t, err, "write motd file") + tmpdir := t.TempDir() + name := filepath.Join(tmpdir, "motd") + err := os.WriteFile(name, []byte(wantMOTD), 0o600) + require.NoError(t, err, "write motd file") - // Set HOME so we can ensure no ~/.hushlogin is present. - t.Setenv("HOME", tmpdir) + // Set HOME so we can ensure no ~/.hushlogin is present. + t.Setenv("HOME", tmpdir) - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ - MOTDFile: name, - }) - err = session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) - require.NoError(t, err) - - ptty := ptytest.New(t) - var stdout bytes.Buffer - session.Stdout = &stdout - session.Stderr = ptty.Output() - session.Stdin = ptty.Input() - err = session.Shell() - require.NoError(t, err) - - ptty.WriteLine("exit 0") - err = session.Wait() - require.NoError(t, err) - - require.Contains(t, stdout.String(), wantMOTD, "should show motd") + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ + MOTDFile: name, }) + err = session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) + require.NoError(t, err) - //nolint:paralleltest // This test sets an environment variable. - t.Run("Session TTY Hushlogin", func(t *testing.T) { - if runtime.GOOS == "windows" { - // This might be our implementation, or ConPTY itself. - // It's difficult to find extensive tests for it, so - // it seems like it could be either. - t.Skip("ConPTY appears to be inconsistent on Windows.") - } + ptty := ptytest.New(t) + var stdout bytes.Buffer + session.Stdout = &stdout + session.Stderr = ptty.Output() + session.Stdin = ptty.Input() + err = session.Shell() + require.NoError(t, err) - wantNotMOTD := "Welcome to your Coder workspace!" + ptty.WriteLine("exit 0") + err = session.Wait() + require.NoError(t, err) - tmpdir := t.TempDir() - name := filepath.Join(tmpdir, "motd") - err := os.WriteFile(name, []byte(wantNotMOTD), 0o600) - require.NoError(t, err, "write motd file") + require.Contains(t, stdout.String(), wantMOTD, "should show motd") +} - // Create hushlogin to silence motd. - f, err := os.Create(filepath.Join(tmpdir, ".hushlogin")) - require.NoError(t, err, "create .hushlogin file") - err = f.Close() - require.NoError(t, err, "close .hushlogin file") +//nolint:paralleltest // This test sets an environment variable. +func TestAgent_Session_TTY_Hushlogin(t *testing.T) { + if runtime.GOOS == "windows" { + // This might be our implementation, or ConPTY itself. + // It's difficult to find extensive tests for it, so + // it seems like it could be either. + t.Skip("ConPTY appears to be inconsistent on Windows.") + } - // Set HOME so we can ensure ~/.hushlogin is present. - t.Setenv("HOME", tmpdir) + wantNotMOTD := "Welcome to your Coder workspace!" - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ - MOTDFile: name, - }) - err = session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) - require.NoError(t, err) - - ptty := ptytest.New(t) - var stdout bytes.Buffer - session.Stdout = &stdout - session.Stderr = ptty.Output() - session.Stdin = ptty.Input() - err = session.Shell() - require.NoError(t, err) - - ptty.WriteLine("exit 0") - err = session.Wait() - require.NoError(t, err) - - require.NotContains(t, stdout.String(), wantNotMOTD, "should not show motd") - }) + tmpdir := t.TempDir() + name := filepath.Join(tmpdir, "motd") + err := os.WriteFile(name, []byte(wantNotMOTD), 0o600) + require.NoError(t, err, "write motd file") - //nolint:paralleltest // This test reserves a port. - t.Run("LocalForwarding", func(t *testing.T) { - random, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - _ = random.Close() - tcpAddr, valid := random.Addr().(*net.TCPAddr) - require.True(t, valid) - randomPort := tcpAddr.Port - - local, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err) - defer local.Close() - tcpAddr, valid = local.Addr().(*net.TCPAddr) - require.True(t, valid) - localPort := tcpAddr.Port - done := make(chan struct{}) - go func() { - defer close(done) - conn, err := local.Accept() - if !assert.NoError(t, err) { - return - } - _ = conn.Close() - }() + // Create hushlogin to silence motd. + f, err := os.Create(filepath.Join(tmpdir, ".hushlogin")) + require.NoError(t, err, "create .hushlogin file") + err = f.Close() + require.NoError(t, err, "close .hushlogin file") - err = setupSSHCommand(t, []string{"-L", fmt.Sprintf("%d:127.0.0.1:%d", randomPort, localPort)}, []string{"echo", "test"}).Start() - require.NoError(t, err) + // Set HOME so we can ensure ~/.hushlogin is present. + t.Setenv("HOME", tmpdir) - conn, err := net.Dial("tcp", "127.0.0.1:"+strconv.Itoa(localPort)) - require.NoError(t, err) - conn.Close() - <-done + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ + MOTDFile: name, }) + err = session.RequestPty("xterm", 128, 128, ssh.TerminalModes{}) + require.NoError(t, err) - t.Run("SFTP", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - u, err := user.Current() - require.NoError(t, err, "get current user") - home := u.HomeDir - if runtime.GOOS == "windows" { - home = "/" + strings.ReplaceAll(home, "\\", "/") - } - conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) - sshClient, err := conn.SSHClient(ctx) - require.NoError(t, err) - defer sshClient.Close() - client, err := sftp.NewClient(sshClient) - require.NoError(t, err) - defer client.Close() - wd, err := client.Getwd() - require.NoError(t, err, "get working directory") - require.Equal(t, home, wd, "working directory should be home user home") - tempFile := filepath.Join(t.TempDir(), "sftp") - // SFTP only accepts unix-y paths. - remoteFile := filepath.ToSlash(tempFile) - if !path.IsAbs(remoteFile) { - // On Windows, e.g. "/C:/Users/...". - remoteFile = path.Join("/", remoteFile) - } - file, err := client.Create(remoteFile) - require.NoError(t, err) - err = file.Close() - require.NoError(t, err) - _, err = os.Stat(tempFile) - require.NoError(t, err) - }) + ptty := ptytest.New(t) + var stdout bytes.Buffer + session.Stdout = &stdout + session.Stderr = ptty.Output() + session.Stdin = ptty.Input() + err = session.Shell() + require.NoError(t, err) - t.Run("SCP", func(t *testing.T) { - t.Parallel() - - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - - conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) - sshClient, err := conn.SSHClient(ctx) - require.NoError(t, err) - defer sshClient.Close() - scpClient, err := scp.NewClientBySSH(sshClient) - require.NoError(t, err) - defer scpClient.Close() - tempFile := filepath.Join(t.TempDir(), "scp") - content := "hello world" - err = scpClient.CopyFile(context.Background(), strings.NewReader(content), tempFile, "0755") - require.NoError(t, err) - _, err = os.Stat(tempFile) - require.NoError(t, err) - }) + ptty.WriteLine("exit 0") + err = session.Wait() + require.NoError(t, err) - t.Run("EnvironmentVariables", func(t *testing.T) { - t.Parallel() - key := "EXAMPLE" - value := "value" - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ - EnvironmentVariables: map[string]string{ - key: value, - }, - }) - command := "sh -c 'echo $" + key + "'" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo %" + key + "%" - } - output, err := session.Output(command) - require.NoError(t, err) - require.Equal(t, value, strings.TrimSpace(string(output))) - }) + require.NotContains(t, stdout.String(), wantNotMOTD, "should not show motd") +} - t.Run("EnvironmentVariableExpansion", func(t *testing.T) { - t.Parallel() - key := "EXAMPLE" - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ - EnvironmentVariables: map[string]string{ - key: "$SOMETHINGNOTSET", - }, - }) - command := "sh -c 'echo $" + key + "'" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo %" + key + "%" - } - output, err := session.Output(command) - require.NoError(t, err) - expect := "" - if runtime.GOOS == "windows" { - expect = "%EXAMPLE%" +//nolint:paralleltest // This test reserves a port. +func TestAgent_LocalForwarding(t *testing.T) { + random, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + _ = random.Close() + tcpAddr, valid := random.Addr().(*net.TCPAddr) + require.True(t, valid) + randomPort := tcpAddr.Port + + local, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err) + defer local.Close() + tcpAddr, valid = local.Addr().(*net.TCPAddr) + require.True(t, valid) + localPort := tcpAddr.Port + done := make(chan struct{}) + go func() { + defer close(done) + conn, err := local.Accept() + if !assert.NoError(t, err) { + return } - // Output should be empty, because the variable is not set! - require.Equal(t, expect, strings.TrimSpace(string(output))) - }) + _ = conn.Close() + }() - t.Run("Coder env vars", func(t *testing.T) { - t.Parallel() + err = setupSSHCommand(t, []string{"-L", fmt.Sprintf("%d:127.0.0.1:%d", randomPort, localPort)}, []string{"echo", "test"}).Start() + require.NoError(t, err) - for _, key := range []string{"CODER"} { - key := key - t.Run(key, func(t *testing.T) { - t.Parallel() + conn, err := net.Dial("tcp", "127.0.0.1:"+strconv.Itoa(localPort)) + require.NoError(t, err) + conn.Close() + <-done +} - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) - command := "sh -c 'echo $" + key + "'" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo %" + key + "%" - } - output, err := session.Output(command) - require.NoError(t, err) - require.NotEmpty(t, strings.TrimSpace(string(output))) - }) - } +func TestAgent_SFTP(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + u, err := user.Current() + require.NoError(t, err, "get current user") + home := u.HomeDir + if runtime.GOOS == "windows" { + home = "/" + strings.ReplaceAll(home, "\\", "/") + } + conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) + sshClient, err := conn.SSHClient(ctx) + require.NoError(t, err) + defer sshClient.Close() + client, err := sftp.NewClient(sshClient) + require.NoError(t, err) + defer client.Close() + wd, err := client.Getwd() + require.NoError(t, err, "get working directory") + require.Equal(t, home, wd, "working directory should be home user home") + tempFile := filepath.Join(t.TempDir(), "sftp") + // SFTP only accepts unix-y paths. + remoteFile := filepath.ToSlash(tempFile) + if !path.IsAbs(remoteFile) { + // On Windows, e.g. "/C:/Users/...". + remoteFile = path.Join("/", remoteFile) + } + file, err := client.Create(remoteFile) + require.NoError(t, err) + err = file.Close() + require.NoError(t, err) + _, err = os.Stat(tempFile) + require.NoError(t, err) +} + +func TestAgent_SCP(t *testing.T) { + t.Parallel() + + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + + conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) + sshClient, err := conn.SSHClient(ctx) + require.NoError(t, err) + defer sshClient.Close() + scpClient, err := scp.NewClientBySSH(sshClient) + require.NoError(t, err) + defer scpClient.Close() + tempFile := filepath.Join(t.TempDir(), "scp") + content := "hello world" + err = scpClient.CopyFile(context.Background(), strings.NewReader(content), tempFile, "0755") + require.NoError(t, err) + _, err = os.Stat(tempFile) + require.NoError(t, err) +} + +func TestAgent_EnvironmentVariables(t *testing.T) { + t.Parallel() + key := "EXAMPLE" + value := "value" + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ + EnvironmentVariables: map[string]string{ + key: value, + }, }) + command := "sh -c 'echo $" + key + "'" + if runtime.GOOS == "windows" { + command = "cmd.exe /c echo %" + key + "%" + } + output, err := session.Output(command) + require.NoError(t, err) + require.Equal(t, value, strings.TrimSpace(string(output))) +} - t.Run("SSH connection env vars", func(t *testing.T) { - t.Parallel() - - // Note: the SSH_TTY environment variable should only be set for TTYs. - // For some reason this test produces a TTY locally and a non-TTY in CI - // so we don't test for the absence of SSH_TTY. - for _, key := range []string{"SSH_CONNECTION", "SSH_CLIENT"} { - key := key - t.Run(key, func(t *testing.T) { - t.Parallel() - - session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) - command := "sh -c 'echo $" + key + "'" - if runtime.GOOS == "windows" { - command = "cmd.exe /c echo %" + key + "%" - } - output, err := session.Output(command) - require.NoError(t, err) - require.NotEmpty(t, strings.TrimSpace(string(output))) - }) - } +func TestAgent_EnvironmentVariableExpansion(t *testing.T) { + t.Parallel() + key := "EXAMPLE" + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{ + EnvironmentVariables: map[string]string{ + key: "$SOMETHINGNOTSET", + }, }) + command := "sh -c 'echo $" + key + "'" + if runtime.GOOS == "windows" { + command = "cmd.exe /c echo %" + key + "%" + } + output, err := session.Output(command) + require.NoError(t, err) + expect := "" + if runtime.GOOS == "windows" { + expect = "%EXAMPLE%" + } + // Output should be empty, because the variable is not set! + require.Equal(t, expect, strings.TrimSpace(string(output))) +} - t.Run("StartupScript", func(t *testing.T) { - t.Parallel() - if runtime.GOOS == "windows" { - t.Skip("This test doesn't work on Windows for some reason...") - } - content := "output" - _, _, fs := setupAgent(t, codersdk.WorkspaceAgentMetadata{ - StartupScript: "echo " + content, - }, 0) - var gotContent string - require.Eventually(t, func() bool { - outputPath := filepath.Join(os.TempDir(), "coder-startup-script.log") - content, err := afero.ReadFile(fs, outputPath) - if err != nil { - t.Logf("read file %q: %s", outputPath, err) - return false - } - if len(content) == 0 { - t.Logf("no content in %q", outputPath) - return false +func TestAgent_CoderEnvVars(t *testing.T) { + t.Parallel() + + for _, key := range []string{"CODER"} { + key := key + t.Run(key, func(t *testing.T) { + t.Parallel() + + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) + command := "sh -c 'echo $" + key + "'" + if runtime.GOOS == "windows" { + command = "cmd.exe /c echo %" + key + "%" } + output, err := session.Output(command) + require.NoError(t, err) + require.NotEmpty(t, strings.TrimSpace(string(output))) + }) + } +} + +func TestAgent_SSHConnectionEnvVars(t *testing.T) { + t.Parallel() + + // Note: the SSH_TTY environment variable should only be set for TTYs. + // For some reason this test produces a TTY locally and a non-TTY in CI + // so we don't test for the absence of SSH_TTY. + for _, key := range []string{"SSH_CONNECTION", "SSH_CLIENT"} { + key := key + t.Run(key, func(t *testing.T) { + t.Parallel() + + session := setupSSHSession(t, codersdk.WorkspaceAgentMetadata{}) + command := "sh -c 'echo $" + key + "'" if runtime.GOOS == "windows" { - // Windows uses UTF16! 🪟🪟🪟 - content, _, err = transform.Bytes(unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewDecoder(), content) - if !assert.NoError(t, err) { - return false - } + command = "cmd.exe /c echo %" + key + "%" } - gotContent = string(content) - return true - }, testutil.WaitShort, testutil.IntervalMedium) - require.Equal(t, content, strings.TrimSpace(gotContent)) - }) + output, err := session.Output(command) + require.NoError(t, err) + require.NotEmpty(t, strings.TrimSpace(string(output))) + }) + } +} - t.Run("ReconnectingPTY", func(t *testing.T) { - t.Parallel() +func TestAgent_StartupScript(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + t.Skip("This test doesn't work on Windows for some reason...") + } + content := "output" + _, _, fs := setupAgent(t, codersdk.WorkspaceAgentMetadata{ + StartupScript: "echo " + content, + }, 0) + var gotContent string + require.Eventually(t, func() bool { + outputPath := filepath.Join(os.TempDir(), "coder-startup-script.log") + content, err := afero.ReadFile(fs, outputPath) + if err != nil { + t.Logf("read file %q: %s", outputPath, err) + return false + } + if len(content) == 0 { + t.Logf("no content in %q", outputPath) + return false + } if runtime.GOOS == "windows" { - // This might be our implementation, or ConPTY itself. - // It's difficult to find extensive tests for it, so - // it seems like it could be either. - t.Skip("ConPTY appears to be inconsistent on Windows.") + // Windows uses UTF16! 🪟🪟🪟 + content, _, err = transform.Bytes(unicode.UTF16(unicode.LittleEndian, unicode.UseBOM).NewDecoder(), content) + if !assert.NoError(t, err) { + return false + } } + gotContent = string(content) + return true + }, testutil.WaitShort, testutil.IntervalMedium) + require.Equal(t, content, strings.TrimSpace(gotContent)) +} - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() +func TestAgent_ReconnectingPTY(t *testing.T) { + t.Parallel() + if runtime.GOOS == "windows" { + // This might be our implementation, or ConPTY itself. + // It's difficult to find extensive tests for it, so + // it seems like it could be either. + t.Skip("ConPTY appears to be inconsistent on Windows.") + } - conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) - id := uuid.New() - netConn, err := conn.ReconnectingPTY(ctx, id, 100, 100, "/bin/bash") - require.NoError(t, err) - defer netConn.Close() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() - bufRead := bufio.NewReader(netConn) + conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) + id := uuid.New() + netConn, err := conn.ReconnectingPTY(ctx, id, 100, 100, "/bin/bash") + require.NoError(t, err) + defer netConn.Close() - // Brief pause to reduce the likelihood that we send keystrokes while - // the shell is simultaneously sending a prompt. - time.Sleep(100 * time.Millisecond) + bufRead := bufio.NewReader(netConn) - data, err := json.Marshal(codersdk.ReconnectingPTYRequest{ - Data: "echo test\r\n", - }) - require.NoError(t, err) - _, err = netConn.Write(data) - require.NoError(t, err) - - expectLine := func(matcher func(string) bool) { - for { - line, err := bufRead.ReadString('\n') - require.NoError(t, err) - if matcher(line) { - break - } + // Brief pause to reduce the likelihood that we send keystrokes while + // the shell is simultaneously sending a prompt. + time.Sleep(100 * time.Millisecond) + + data, err := json.Marshal(codersdk.ReconnectingPTYRequest{ + Data: "echo test\r\n", + }) + require.NoError(t, err) + _, err = netConn.Write(data) + require.NoError(t, err) + + expectLine := func(matcher func(string) bool) { + for { + line, err := bufRead.ReadString('\n') + require.NoError(t, err) + if matcher(line) { + break } } + } - matchEchoCommand := func(line string) bool { - return strings.Contains(line, "echo test") - } - matchEchoOutput := func(line string) bool { - return strings.Contains(line, "test") && !strings.Contains(line, "echo") - } + matchEchoCommand := func(line string) bool { + return strings.Contains(line, "echo test") + } + matchEchoOutput := func(line string) bool { + return strings.Contains(line, "test") && !strings.Contains(line, "echo") + } - // Once for typing the command... - expectLine(matchEchoCommand) - // And another time for the actual output. - expectLine(matchEchoOutput) + // Once for typing the command... + expectLine(matchEchoCommand) + // And another time for the actual output. + expectLine(matchEchoOutput) - _ = netConn.Close() - netConn, err = conn.ReconnectingPTY(ctx, id, 100, 100, "/bin/bash") - require.NoError(t, err) - defer netConn.Close() + _ = netConn.Close() + netConn, err = conn.ReconnectingPTY(ctx, id, 100, 100, "/bin/bash") + require.NoError(t, err) + defer netConn.Close() - bufRead = bufio.NewReader(netConn) + bufRead = bufio.NewReader(netConn) - // Same output again! - expectLine(matchEchoCommand) - expectLine(matchEchoOutput) - }) + // Same output again! + expectLine(matchEchoCommand) + expectLine(matchEchoOutput) +} + +func TestAgent_Dial(t *testing.T) { + t.Parallel() - t.Run("Dial", func(t *testing.T) { - t.Parallel() - - cases := []struct { - name string - setup func(t *testing.T) net.Listener - }{ - { - name: "TCP", - setup: func(t *testing.T) net.Listener { - l, err := net.Listen("tcp", "127.0.0.1:0") - require.NoError(t, err, "create TCP listener") - return l - }, + cases := []struct { + name string + setup func(t *testing.T) net.Listener + }{ + { + name: "TCP", + setup: func(t *testing.T) net.Listener { + l, err := net.Listen("tcp", "127.0.0.1:0") + require.NoError(t, err, "create TCP listener") + return l }, - { - name: "UDP", - setup: func(t *testing.T) net.Listener { - addr := net.UDPAddr{ - IP: net.ParseIP("127.0.0.1"), - Port: 0, - } - l, err := udp.Listen("udp", &addr) - require.NoError(t, err, "create UDP listener") - return l - }, + }, + { + name: "UDP", + setup: func(t *testing.T) net.Listener { + addr := net.UDPAddr{ + IP: net.ParseIP("127.0.0.1"), + Port: 0, + } + l, err := udp.Listen("udp", &addr) + require.NoError(t, err, "create UDP listener") + return l }, - } + }, + } + + for _, c := range cases { + c := c + t.Run(c.name, func(t *testing.T) { + t.Parallel() - for _, c := range cases { - c := c - t.Run(c.name, func(t *testing.T) { - t.Parallel() - - // Setup listener - l := c.setup(t) - defer l.Close() - go func() { - for { - c, err := l.Accept() - if err != nil { - return - } - - go testAccept(t, c) + // Setup listener + l := c.setup(t) + defer l.Close() + go func() { + for { + c, err := l.Accept() + if err != nil { + return } - }() - - conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) - require.True(t, conn.AwaitReachable(context.Background())) - conn1, err := conn.DialContext(context.Background(), l.Addr().Network(), l.Addr().String()) - require.NoError(t, err) - defer conn1.Close() - conn2, err := conn.DialContext(context.Background(), l.Addr().Network(), l.Addr().String()) - require.NoError(t, err) - defer conn2.Close() - testDial(t, conn2) - testDial(t, conn1) - time.Sleep(150 * time.Millisecond) - }) - } - }) - t.Run("Speedtest", func(t *testing.T) { - t.Parallel() - t.Skip("This test is relatively flakey because of Tailscale's speedtest code...") - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - derpMap := tailnettest.RunDERPAndSTUN(t) - conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{ - DERPMap: derpMap, - }, 0) - defer conn.Close() - res, err := conn.Speedtest(ctx, speedtest.Upload, 250*time.Millisecond) - require.NoError(t, err) - t.Logf("%.2f MBits/s", res[len(res)-1].MBitsPerSecond()) - }) + go testAccept(t, c) + } + }() - t.Run("Reconnect", func(t *testing.T) { - t.Parallel() - // After the agent is disconnected from a coordinator, it's supposed - // to reconnect! - coordinator := tailnet.NewCoordinator() - defer coordinator.Close() - - agentID := uuid.New() - statsCh := make(chan *codersdk.AgentStats) - derpMap := tailnettest.RunDERPAndSTUN(t) - client := &client{ - t: t, - agentID: agentID, - metadata: codersdk.WorkspaceAgentMetadata{ - DERPMap: derpMap, - }, - statsChan: statsCh, - coordinator: coordinator, - } - initialized := atomic.Int32{} - closer := agent.New(agent.Options{ - ExchangeToken: func(ctx context.Context) (string, error) { - initialized.Add(1) - return "", nil - }, - Client: client, - Logger: slogtest.Make(t, nil).Leveled(slog.LevelInfo), + conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{}, 0) + require.True(t, conn.AwaitReachable(context.Background())) + conn1, err := conn.DialContext(context.Background(), l.Addr().Network(), l.Addr().String()) + require.NoError(t, err) + defer conn1.Close() + conn2, err := conn.DialContext(context.Background(), l.Addr().Network(), l.Addr().String()) + require.NoError(t, err) + defer conn2.Close() + testDial(t, conn2) + testDial(t, conn1) + time.Sleep(150 * time.Millisecond) }) - defer closer.Close() - - require.Eventually(t, func() bool { - return coordinator.Node(agentID) != nil - }, testutil.WaitShort, testutil.IntervalFast) - client.lastWorkspaceAgent() - require.Eventually(t, func() bool { - return initialized.Load() == 2 - }, testutil.WaitShort, testutil.IntervalFast) - }) + } +} - t.Run("WriteVSCodeConfigs", func(t *testing.T) { - t.Parallel() +func TestAgent_Speedtest(t *testing.T) { + t.Parallel() + t.Skip("This test is relatively flakey because of Tailscale's speedtest code...") + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + derpMap := tailnettest.RunDERPAndSTUN(t) + conn, _, _ := setupAgent(t, codersdk.WorkspaceAgentMetadata{ + DERPMap: derpMap, + }, 0) + defer conn.Close() + res, err := conn.Speedtest(ctx, speedtest.Upload, 250*time.Millisecond) + require.NoError(t, err) + t.Logf("%.2f MBits/s", res[len(res)-1].MBitsPerSecond()) +} - coordinator := tailnet.NewCoordinator() - defer coordinator.Close() +func TestAgent_Reconnect(t *testing.T) { + t.Parallel() + // After the agent is disconnected from a coordinator, it's supposed + // to reconnect! + coordinator := tailnet.NewCoordinator() + defer coordinator.Close() - client := &client{ - t: t, - agentID: uuid.New(), - metadata: codersdk.WorkspaceAgentMetadata{ - GitAuthConfigs: 1, - DERPMap: &tailcfg.DERPMap{}, - }, - statsChan: make(chan *codersdk.AgentStats), - coordinator: coordinator, - } - filesystem := afero.NewMemMapFs() - closer := agent.New(agent.Options{ - ExchangeToken: func(ctx context.Context) (string, error) { - return "", nil - }, - Client: client, - Logger: slogtest.Make(t, nil).Leveled(slog.LevelInfo), - Filesystem: filesystem, - }) - defer closer.Close() - - home, err := os.UserHomeDir() - require.NoError(t, err) - path := filepath.Join(home, ".vscode-server", "data", "Machine", "settings.json") - require.Eventually(t, func() bool { - _, err := filesystem.Stat(path) - return err == nil - }, testutil.WaitShort, testutil.IntervalFast) + agentID := uuid.New() + statsCh := make(chan *codersdk.AgentStats) + derpMap := tailnettest.RunDERPAndSTUN(t) + client := &client{ + t: t, + agentID: agentID, + metadata: codersdk.WorkspaceAgentMetadata{ + DERPMap: derpMap, + }, + statsChan: statsCh, + coordinator: coordinator, + } + initialized := atomic.Int32{} + closer := agent.New(agent.Options{ + ExchangeToken: func(ctx context.Context) (string, error) { + initialized.Add(1) + return "", nil + }, + Client: client, + Logger: slogtest.Make(t, nil).Leveled(slog.LevelInfo), + }) + defer closer.Close() + + require.Eventually(t, func() bool { + return coordinator.Node(agentID) != nil + }, testutil.WaitShort, testutil.IntervalFast) + client.lastWorkspaceAgent() + require.Eventually(t, func() bool { + return initialized.Load() == 2 + }, testutil.WaitShort, testutil.IntervalFast) +} + +func TestAgent_WriteVSCodeConfigs(t *testing.T) { + t.Parallel() + + coordinator := tailnet.NewCoordinator() + defer coordinator.Close() + + client := &client{ + t: t, + agentID: uuid.New(), + metadata: codersdk.WorkspaceAgentMetadata{ + GitAuthConfigs: 1, + DERPMap: &tailcfg.DERPMap{}, + }, + statsChan: make(chan *codersdk.AgentStats), + coordinator: coordinator, + } + filesystem := afero.NewMemMapFs() + closer := agent.New(agent.Options{ + ExchangeToken: func(ctx context.Context) (string, error) { + return "", nil + }, + Client: client, + Logger: slogtest.Make(t, nil).Leveled(slog.LevelInfo), + Filesystem: filesystem, }) + defer closer.Close() + + home, err := os.UserHomeDir() + require.NoError(t, err) + path := filepath.Join(home, ".vscode-server", "data", "Machine", "settings.json") + require.Eventually(t, func() bool { + _, err := filesystem.Stat(path) + return err == nil + }, testutil.WaitShort, testutil.IntervalFast) } func setupSSHCommand(t *testing.T, beforeArgs []string, afterArgs []string) *exec.Cmd { @@ -806,12 +799,17 @@ func setupAgent(t *testing.T, metadata codersdk.WorkspaceAgentMetadata, ptyTimeo }) require.NoError(t, err) clientConn, serverConn := net.Pipe() + serveClientDone := make(chan struct{}) t.Cleanup(func() { _ = clientConn.Close() _ = serverConn.Close() _ = conn.Close() + <-serveClientDone }) - go coordinator.ServeClient(serverConn, uuid.New(), agentID) + go func() { + defer close(serveClientDone) + coordinator.ServeClient(serverConn, uuid.New(), agentID) + }() sendNode, _ := tailnet.ServeCoordinator(clientConn, func(node []*tailnet.Node) error { return conn.UpdateNodes(node) }) diff --git a/agent/apphealth_test.go b/agent/apphealth_test.go index 51887afd6472b..14965f18ff45b 100644 --- a/agent/apphealth_test.go +++ b/agent/apphealth_test.go @@ -19,148 +19,145 @@ import ( "github.com/coder/coder/testutil" ) -func TestAppHealth(t *testing.T) { +func TestAppHealth_Healthy(t *testing.T) { t.Parallel() - t.Run("Healthy", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - apps := []codersdk.WorkspaceApp{ - { - Slug: "app1", - Healthcheck: codersdk.Healthcheck{}, - Health: codersdk.WorkspaceAppHealthDisabled, + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + apps := []codersdk.WorkspaceApp{ + { + Slug: "app1", + Healthcheck: codersdk.Healthcheck{}, + Health: codersdk.WorkspaceAppHealthDisabled, + }, + { + Slug: "app2", + Healthcheck: codersdk.Healthcheck{ + // URL: We don't set the URL for this test because the setup will + // create a httptest server for us and set it for us. + Interval: 1, + Threshold: 1, }, - { - Slug: "app2", - Healthcheck: codersdk.Healthcheck{ - // URL: We don't set the URL for this test because the setup will - // create a httptest server for us and set it for us. - Interval: 1, - Threshold: 1, - }, - Health: codersdk.WorkspaceAppHealthInitializing, - }, - } - handlers := []http.Handler{ - nil, - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - httpapi.Write(r.Context(), w, http.StatusOK, nil) - }), - } - getApps, closeFn := setupAppReporter(ctx, t, apps, handlers) - defer closeFn() + Health: codersdk.WorkspaceAppHealthInitializing, + }, + } + handlers := []http.Handler{ + nil, + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusOK, nil) + }), + } + getApps, closeFn := setupAppReporter(ctx, t, apps, handlers) + defer closeFn() + apps, err := getApps(ctx) + require.NoError(t, err) + require.EqualValues(t, codersdk.WorkspaceAppHealthDisabled, apps[0].Health) + require.Eventually(t, func() bool { apps, err := getApps(ctx) - require.NoError(t, err) - require.EqualValues(t, codersdk.WorkspaceAppHealthDisabled, apps[0].Health) - require.Eventually(t, func() bool { - apps, err := getApps(ctx) - if err != nil { - return false - } + if err != nil { + return false + } - return apps[1].Health == codersdk.WorkspaceAppHealthHealthy - }, testutil.WaitLong, testutil.IntervalSlow) - }) - - t.Run("500", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - apps := []codersdk.WorkspaceApp{ - { - Slug: "app2", - Healthcheck: codersdk.Healthcheck{ - // URL: We don't set the URL for this test because the setup will - // create a httptest server for us and set it for us. - Interval: 1, - Threshold: 1, - }, - Health: codersdk.WorkspaceAppHealthInitializing, + return apps[1].Health == codersdk.WorkspaceAppHealthHealthy + }, testutil.WaitLong, testutil.IntervalSlow) +} + +func TestAppHealth_500(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + apps := []codersdk.WorkspaceApp{ + { + Slug: "app2", + Healthcheck: codersdk.Healthcheck{ + // URL: We don't set the URL for this test because the setup will + // create a httptest server for us and set it for us. + Interval: 1, + Threshold: 1, }, + Health: codersdk.WorkspaceAppHealthInitializing, + }, + } + handlers := []http.Handler{ + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + httpapi.Write(r.Context(), w, http.StatusInternalServerError, nil) + }), + } + getApps, closeFn := setupAppReporter(ctx, t, apps, handlers) + defer closeFn() + require.Eventually(t, func() bool { + apps, err := getApps(ctx) + if err != nil { + return false } - handlers := []http.Handler{ - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - httpapi.Write(r.Context(), w, http.StatusInternalServerError, nil) - }), - } - getApps, closeFn := setupAppReporter(ctx, t, apps, handlers) - defer closeFn() - require.Eventually(t, func() bool { - apps, err := getApps(ctx) - if err != nil { - return false - } - return apps[0].Health == codersdk.WorkspaceAppHealthUnhealthy - }, testutil.WaitLong, testutil.IntervalSlow) - }) - - t.Run("Timeout", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - apps := []codersdk.WorkspaceApp{ - { - Slug: "app2", - Healthcheck: codersdk.Healthcheck{ - // URL: We don't set the URL for this test because the setup will - // create a httptest server for us and set it for us. - Interval: 1, - Threshold: 1, - }, - Health: codersdk.WorkspaceAppHealthInitializing, + return apps[0].Health == codersdk.WorkspaceAppHealthUnhealthy + }, testutil.WaitLong, testutil.IntervalSlow) +} + +func TestAppHealth_Timeout(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + apps := []codersdk.WorkspaceApp{ + { + Slug: "app2", + Healthcheck: codersdk.Healthcheck{ + // URL: We don't set the URL for this test because the setup will + // create a httptest server for us and set it for us. + Interval: 1, + Threshold: 1, }, + Health: codersdk.WorkspaceAppHealthInitializing, + }, + } + handlers := []http.Handler{ + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + // sleep longer than the interval to cause the health check to time out + time.Sleep(2 * time.Second) + httpapi.Write(r.Context(), w, http.StatusOK, nil) + }), + } + getApps, closeFn := setupAppReporter(ctx, t, apps, handlers) + defer closeFn() + require.Eventually(t, func() bool { + apps, err := getApps(ctx) + if err != nil { + return false } - handlers := []http.Handler{ - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - // sleep longer than the interval to cause the health check to time out - time.Sleep(2 * time.Second) - httpapi.Write(r.Context(), w, http.StatusOK, nil) - }), - } - getApps, closeFn := setupAppReporter(ctx, t, apps, handlers) - defer closeFn() - require.Eventually(t, func() bool { - apps, err := getApps(ctx) - if err != nil { - return false - } - return apps[0].Health == codersdk.WorkspaceAppHealthUnhealthy - }, testutil.WaitLong, testutil.IntervalSlow) - }) - - t.Run("NotSpamming", func(t *testing.T) { - t.Parallel() - ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) - defer cancel() - apps := []codersdk.WorkspaceApp{ - { - Slug: "app2", - Healthcheck: codersdk.Healthcheck{ - // URL: We don't set the URL for this test because the setup will - // create a httptest server for us and set it for us. - Interval: 1, - Threshold: 1, - }, - Health: codersdk.WorkspaceAppHealthInitializing, + return apps[0].Health == codersdk.WorkspaceAppHealthUnhealthy + }, testutil.WaitLong, testutil.IntervalSlow) +} + +func TestAppHealth_NotSpamming(t *testing.T) { + t.Parallel() + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) + defer cancel() + apps := []codersdk.WorkspaceApp{ + { + Slug: "app2", + Healthcheck: codersdk.Healthcheck{ + // URL: We don't set the URL for this test because the setup will + // create a httptest server for us and set it for us. + Interval: 1, + Threshold: 1, }, - } + Health: codersdk.WorkspaceAppHealthInitializing, + }, + } - var counter = new(int32) - handlers := []http.Handler{ - http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { - atomic.AddInt32(counter, 1) - }), - } - _, closeFn := setupAppReporter(ctx, t, apps, handlers) - defer closeFn() - // Ensure we haven't made more than 2 (expected 1 + 1 for buffer) requests in the last second. - // if there is a bug where we are spamming the healthcheck route this will catch it. - time.Sleep(time.Second) - require.LessOrEqual(t, *counter, int32(2)) - }) + counter := new(int32) + handlers := []http.Handler{ + http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) { + atomic.AddInt32(counter, 1) + }), + } + _, closeFn := setupAppReporter(ctx, t, apps, handlers) + defer closeFn() + // Ensure we haven't made more than 2 (expected 1 + 1 for buffer) requests in the last second. + // if there is a bug where we are spamming the healthcheck route this will catch it. + time.Sleep(time.Second) + require.LessOrEqual(t, *counter, int32(2)) } func setupAppReporter(ctx context.Context, t *testing.T, apps []codersdk.WorkspaceApp, handlers []http.Handler) (agent.WorkspaceAgentApps, func()) { From 7669ec3e9e1c95ec0a61ec82798edbef562d74fd Mon Sep 17 00:00:00 2001 From: Mathias Fredriksson Date: Mon, 12 Dec 2022 10:15:16 +0000 Subject: [PATCH 2/2] fix: Lint shadowed import --- agent/agent_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/agent/agent_test.go b/agent/agent_test.go index dfeeb76fec8d4..25f1f86e95062 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -690,9 +690,9 @@ func TestAgent_WriteVSCodeConfigs(t *testing.T) { home, err := os.UserHomeDir() require.NoError(t, err) - path := filepath.Join(home, ".vscode-server", "data", "Machine", "settings.json") + name := filepath.Join(home, ".vscode-server", "data", "Machine", "settings.json") require.Eventually(t, func() bool { - _, err := filesystem.Stat(path) + _, err := filesystem.Stat(name) return err == nil }, testutil.WaitShort, testutil.IntervalFast) }