diff --git a/.github/workflows/coder.yaml b/.github/workflows/coder.yaml index fb41b3f1f2924..a0e4b6a7e905a 100644 --- a/.github/workflows/coder.yaml +++ b/.github/workflows/coder.yaml @@ -269,7 +269,11 @@ jobs: echo ::set-output name=cover::false fi set -x - gotestsum --junitfile="gotests.xml" --packages="./..." -- -parallel=8 -timeout=5m -short -failfast $COVERAGE_FLAGS + test_timeout=5m + if [[ "${{ matrix.os }}" == windows* ]]; then + test_timeout=10m + fi + gotestsum --junitfile="gotests.xml" --packages="./..." -- -parallel=8 -timeout=$test_timeout -short -failfast $COVERAGE_FLAGS - name: Upload DataDog Trace if: github.actor != 'dependabot[bot]' && !github.event.pull_request.head.repo.fork diff --git a/agent/agent_test.go b/agent/agent_test.go index cefa54d037f31..e3d3a80886c02 100644 --- a/agent/agent_test.go +++ b/agent/agent_test.go @@ -38,6 +38,7 @@ import ( "github.com/coder/coder/peerbroker/proto" "github.com/coder/coder/provisionersdk" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) func TestMain(m *testing.M) { @@ -257,7 +258,7 @@ func TestAgent(t *testing.T) { } gotContent = string(content) return true - }, 15*time.Second, 100*time.Millisecond) + }, testutil.WaitMedium, testutil.IntervalMedium) require.Equal(t, content, strings.TrimSpace(gotContent)) }) diff --git a/agent/reaper/reaper_test.go b/agent/reaper/reaper_test.go index 8e23bec198c5c..88573636ee39c 100644 --- a/agent/reaper/reaper_test.go +++ b/agent/reaper/reaper_test.go @@ -12,6 +12,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/agent/reaper" + "github.com/coder/coder/testutil" ) func TestReap(t *testing.T) { @@ -52,10 +53,9 @@ func TestReap(t *testing.T) { expectedPIDs := []int{cmd.Process.Pid, cmd2.Process.Pid} - deadline := time.NewTimer(time.Second * 5) for i := 0; i < len(expectedPIDs); i++ { select { - case <-deadline.C: + case <-time.After(testutil.WaitShort): t.Fatalf("Timed out waiting for process") case pid := <-pids: require.Contains(t, expectedPIDs, pid) diff --git a/cli/cliui/prompt_test.go b/cli/cliui/prompt_test.go index eefc6ab4c8a5a..6c7f233c872e6 100644 --- a/cli/cliui/prompt_test.go +++ b/cli/cliui/prompt_test.go @@ -7,7 +7,6 @@ import ( "os" "os/exec" "testing" - "time" "github.com/spf13/cobra" "github.com/stretchr/testify/assert" @@ -16,6 +15,7 @@ import ( "github.com/coder/coder/cli/cliui" "github.com/coder/coder/pty" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) func TestPrompt(t *testing.T) { @@ -61,7 +61,7 @@ func TestPrompt(t *testing.T) { // Copy all data written out to a buffer. When we close the ptty, we can // no longer read from the ptty.Output(), but we can read what was // written to the buffer. - dataRead, doneReading := context.WithTimeout(context.Background(), time.Second*2) + dataRead, doneReading := context.WithTimeout(context.Background(), testutil.WaitShort) go func() { // This will throw an error sometimes. The underlying ptty // has its own cleanup routines in t.Cleanup. Instead of @@ -193,7 +193,7 @@ func TestPasswordTerminalState(t *testing.T) { require.Eventually(t, func() bool { echo, err := ptyWithFlags.EchoEnabled() return err == nil && !echo - }, 5*time.Second, 50*time.Millisecond, "echo is on while reading password") + }, testutil.WaitShort, testutil.IntervalMedium, "echo is on while reading password") err = process.Signal(os.Interrupt) require.NoError(t, err) @@ -203,7 +203,7 @@ func TestPasswordTerminalState(t *testing.T) { require.Eventually(t, func() bool { echo, err := ptyWithFlags.EchoEnabled() return err == nil && echo - }, 5*time.Second, 50*time.Millisecond, "echo is off after reading password") + }, testutil.WaitShort, testutil.IntervalMedium, "echo is off after reading password") } // nolint:unused diff --git a/cli/create_test.go b/cli/create_test.go index 5578518f351dc..cdb8f5e5e8019 100644 --- a/cli/create_test.go +++ b/cli/create_test.go @@ -16,6 +16,7 @@ import ( "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) func TestCreate(t *testing.T) { @@ -88,7 +89,7 @@ func TestCreate(t *testing.T) { member := coderdtest.CreateAnotherUser(t, client, user.OrganizationID) clitest.SetupConfig(t, member, root) - cmdCtx, done := context.WithTimeout(context.Background(), 10*time.Second) + cmdCtx, done := context.WithTimeout(context.Background(), testutil.WaitLong) go func() { defer done() err := cmd.ExecuteContext(cmdCtx) diff --git a/cli/list_test.go b/cli/list_test.go index 8343f1fead1ab..a5aaeb5f4da30 100644 --- a/cli/list_test.go +++ b/cli/list_test.go @@ -3,13 +3,13 @@ package cli_test import ( "context" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/coder/coder/cli/clitest" "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) func TestList(t *testing.T) { @@ -29,7 +29,7 @@ func TestList(t *testing.T) { cmd.SetIn(pty.Input()) cmd.SetOut(pty.Output()) - ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancelFunc() done := make(chan any) go func() { diff --git a/cli/portforward_test.go b/cli/portforward_test.go index 5f314a3658850..a3547ba4dface 100644 --- a/cli/portforward_test.go +++ b/cli/portforward_test.go @@ -23,6 +23,7 @@ import ( "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" ) func TestPortForward(t *testing.T) { @@ -170,7 +171,7 @@ func TestPortForward(t *testing.T) { // Open two connections simultaneously and test them out of // sync. - d := net.Dialer{Timeout: 3 * time.Second} + d := net.Dialer{Timeout: testutil.WaitShort} c1, err := d.DialContext(ctx, c.network, localAddress) require.NoError(t, err, "open connection 1 to 'local' listener") defer c1.Close() @@ -216,7 +217,7 @@ func TestPortForward(t *testing.T) { // Open a connection to both listener 1 and 2 simultaneously and // then test them out of order. - d := net.Dialer{Timeout: 3 * time.Second} + d := net.Dialer{Timeout: testutil.WaitShort} c1, err := d.DialContext(ctx, c.network, localAddress1) require.NoError(t, err, "open connection 1 to 'local' listener 1") defer c1.Close() @@ -269,7 +270,7 @@ func TestPortForward(t *testing.T) { // Open two connections simultaneously and test them out of // sync. - d := net.Dialer{Timeout: 3 * time.Second} + d := net.Dialer{Timeout: testutil.WaitShort} c1, err := d.DialContext(ctx, tcpCase.network, localAddress) require.NoError(t, err, "open connection 1 to 'local' listener") defer c1.Close() @@ -329,7 +330,7 @@ func TestPortForward(t *testing.T) { // Open connections to all items in the "dial" array. var ( - d = net.Dialer{Timeout: 3 * time.Second} + d = net.Dialer{Timeout: testutil.WaitShort} conns = make([]net.Conn, len(dials)) ) for i, a := range dials { @@ -488,7 +489,7 @@ func assertWritePayload(t *testing.T, w io.Writer, payload []byte) { func waitForPortForwardReady(t *testing.T, output *threadSafeBuffer) { t.Helper() for i := 0; i < 100; i++ { - time.Sleep(250 * time.Millisecond) + time.Sleep(testutil.IntervalMedium) data := output.String() if strings.Contains(data, "Ready!") { diff --git a/cli/resetpassword_test.go b/cli/resetpassword_test.go index 6499e30a2481b..420f3793b2f09 100644 --- a/cli/resetpassword_test.go +++ b/cli/resetpassword_test.go @@ -5,7 +5,6 @@ import ( "net/url" "runtime" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -14,6 +13,7 @@ import ( "github.com/coder/coder/coderd/database/postgres" "github.com/coder/coder/codersdk" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) // nolint:paralleltest @@ -53,7 +53,7 @@ func TestResetPassword(t *testing.T) { require.Eventually(t, func() bool { rawURL, err = cfg.URL().Read() return err == nil && rawURL != "" - }, 15*time.Second, 25*time.Millisecond) + }, testutil.WaitLong, testutil.IntervalFast) accessURL, err := url.Parse(rawURL) require.NoError(t, err) client := codersdk.New(accessURL) diff --git a/cli/server_test.go b/cli/server_test.go index 5501ed79b5c33..ed9676f98f391 100644 --- a/cli/server_test.go +++ b/cli/server_test.go @@ -25,10 +25,12 @@ import ( "go.uber.org/goleak" "github.com/coder/coder/cli/clitest" + "github.com/coder/coder/cli/config" "github.com/coder/coder/coderd/database/postgres" "github.com/coder/coder/coderd/telemetry" "github.com/coder/coder/codersdk" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) // This cannot be ran in parallel because it uses a signal. @@ -55,13 +57,7 @@ func TestServer(t *testing.T) { go func() { errC <- root.ExecuteContext(ctx) }() - var rawURL string - require.Eventually(t, func() bool { - rawURL, err = cfg.URL().Read() - return err == nil && rawURL != "" - }, time.Minute, 50*time.Millisecond) - accessURL, err := url.Parse(rawURL) - require.NoError(t, err) + accessURL := waitAccessURL(t, cfg) client := codersdk.New(accessURL) _, err = client.CreateFirstUser(ctx, codersdk.CreateFirstUserRequest{ @@ -94,10 +90,11 @@ func TestServer(t *testing.T) { go func() { errC <- root.ExecuteContext(ctx) }() + //nolint:gocritic // Embedded postgres take a while to fire up. require.Eventually(t, func() bool { - accessURLRaw, err := cfg.URL().Read() - return accessURLRaw != "" && err == nil - }, 3*time.Minute, 250*time.Millisecond) + rawURL, err := cfg.URL().Read() + return err == nil && rawURL != "" + }, 3*time.Minute, testutil.IntervalFast, "failed to get access URL") cancelFunc() require.ErrorIs(t, <-errC, context.Canceled) }) @@ -135,11 +132,7 @@ func TestServer(t *testing.T) { }() // Just wait for startup - require.Eventually(t, func() bool { - var err error - _, err = cfg.URL().Read() - return err == nil - }, 15*time.Second, 25*time.Millisecond) + _ = waitAccessURL(t, cfg) cancelFunc() require.ErrorIs(t, <-errC, context.Canceled) @@ -169,11 +162,7 @@ func TestServer(t *testing.T) { }() // Just wait for startup - require.Eventually(t, func() bool { - var err error - _, err = cfg.URL().Read() - return err == nil - }, 15*time.Second, 25*time.Millisecond) + _ = waitAccessURL(t, cfg) cancelFunc() require.ErrorIs(t, <-errC, context.Canceled) @@ -201,11 +190,7 @@ func TestServer(t *testing.T) { }() // Just wait for startup - require.Eventually(t, func() bool { - var err error - _, err = cfg.URL().Read() - return err == nil - }, 15*time.Second, 25*time.Millisecond) + _ = waitAccessURL(t, cfg) cancelFunc() require.ErrorIs(t, <-errC, context.Canceled) @@ -281,14 +266,7 @@ func TestServer(t *testing.T) { }() // Verify HTTPS - var accessURLRaw string - require.Eventually(t, func() bool { - var err error - accessURLRaw, err = cfg.URL().Read() - return accessURLRaw != "" && err == nil - }, 15*time.Second, 25*time.Millisecond) - accessURL, err := url.Parse(accessURLRaw) - require.NoError(t, err) + accessURL := waitAccessURL(t, cfg) require.Equal(t, "https", accessURL.Scheme) client := codersdk.New(accessURL) client.HTTPClient = &http.Client{ @@ -299,7 +277,7 @@ func TestServer(t *testing.T) { }, }, } - _, err = client.HasFirstUser(ctx) + _, err := client.HasFirstUser(ctx) require.NoError(t, err) cancelFunc() @@ -326,11 +304,7 @@ func TestServer(t *testing.T) { go func() { serverErr <- root.ExecuteContext(ctx) }() - require.Eventually(t, func() bool { - var err error - _, err = cfg.URL().Read() - return err == nil - }, 15*time.Second, 25*time.Millisecond) + _ = waitAccessURL(t, cfg) currentProcess, err := os.FindProcess(os.Getpid()) require.NoError(t, err) err = currentProcess.Signal(os.Interrupt) @@ -436,3 +410,19 @@ func generateTLSCertificate(t testing.TB) (certPath, keyPath string) { require.NoError(t, err) return certFile.Name(), keyFile.Name() } + +func waitAccessURL(t *testing.T, cfg config.Root) *url.URL { + t.Helper() + + var err error + var rawURL string + require.Eventually(t, func() bool { + rawURL, err = cfg.URL().Read() + return err == nil && rawURL != "" + }, testutil.WaitLong, testutil.IntervalFast, "failed to get access URL") + + accessURL, err := url.Parse(rawURL) + require.NoError(t, err, "failed to parse access URL") + + return accessURL +} diff --git a/cli/ssh_test.go b/cli/ssh_test.go index f88339c103fd9..6978e5da298ba 100644 --- a/cli/ssh_test.go +++ b/cli/ssh_test.go @@ -29,6 +29,7 @@ import ( "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" "github.com/coder/coder/pty/ptytest" + "github.com/coder/coder/testutil" ) func setupWorkspaceForSSH(t *testing.T) (*codersdk.Client, codersdk.Workspace, string) { @@ -77,7 +78,7 @@ func TestSSH(t *testing.T) { cmd.SetErr(pty.Output()) cmd.SetOut(pty.Output()) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() cmdDone := tGo(t, func() { @@ -124,7 +125,7 @@ func TestSSH(t *testing.T) { } }() - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() cmd, root := clitest.New(t, "ssh", "--stdio", workspace.Name) @@ -215,7 +216,7 @@ func TestSSH(t *testing.T) { } }) - ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() cmd, root := clitest.New(t, diff --git a/coderd/coderd_test.go b/coderd/coderd_test.go index c457f729f8676..83c6155e14037 100644 --- a/coderd/coderd_test.go +++ b/coderd/coderd_test.go @@ -40,6 +40,7 @@ import ( "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" ) func TestMain(m *testing.M) { @@ -157,7 +158,7 @@ func TestAuthorizeAllEndpoints(t *testing.T) { require.Eventually(t, func() bool { provisionerds, err := client.ProvisionerDaemons(ctx) return assert.NoError(t, err) && len(provisionerds) > 0 - }, time.Second*10, time.Second) + }, testutil.WaitLong, testutil.IntervalSlow) provisionerds, err := client.ProvisionerDaemons(ctx) require.NoError(t, err, "fetch provisioners") diff --git a/coderd/coderdtest/coderdtest.go b/coderd/coderdtest/coderdtest.go index c16f6f860dd11..6b0b2d425caaf 100644 --- a/coderd/coderdtest/coderdtest.go +++ b/coderd/coderdtest/coderdtest.go @@ -56,6 +56,7 @@ import ( "github.com/coder/coder/provisionerd" "github.com/coder/coder/provisionersdk" "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" ) type Options struct { @@ -180,7 +181,7 @@ func newWithCloser(t *testing.T, options *Options) (*codersdk.Client, io.Closer) AgentConnectionUpdateFrequency: 150 * time.Millisecond, // Force a long disconnection timeout to ensure // agents are not marked as disconnected during slow tests. - AgentInactiveDisconnectTimeout: 5 * time.Second, + AgentInactiveDisconnectTimeout: testutil.WaitShort, AccessURL: serverURL, Logger: slogtest.Make(t, nil).Leveled(slog.LevelDebug), CacheDir: t.TempDir(), @@ -414,7 +415,7 @@ func AwaitTemplateVersionJob(t *testing.T, client *codersdk.Client, version uuid var err error templateVersion, err = client.TemplateVersion(context.Background(), version) return assert.NoError(t, err) && templateVersion.Job.CompletedAt != nil - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) return templateVersion } @@ -428,7 +429,7 @@ func AwaitWorkspaceBuildJob(t *testing.T, client *codersdk.Client, build uuid.UU var err error workspaceBuild, err = client.WorkspaceBuild(context.Background(), build) return assert.NoError(t, err) && workspaceBuild.Job.CompletedAt != nil - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) return workspaceBuild } @@ -452,7 +453,7 @@ func AwaitWorkspaceAgents(t *testing.T, client *codersdk.Client, build uuid.UUID } } return true - }, 15*time.Second, 50*time.Millisecond) + }, testutil.WaitLong, testutil.IntervalMedium) return resources } diff --git a/coderd/devtunnel/tunnel_test.go b/coderd/devtunnel/tunnel_test.go index 70007cc257191..23dd92d966d54 100644 --- a/coderd/devtunnel/tunnel_test.go +++ b/coderd/devtunnel/tunnel_test.go @@ -25,6 +25,7 @@ import ( "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/coderd/devtunnel" + "github.com/coder/coder/testutil" ) const ( @@ -85,15 +86,15 @@ func TestTunnel(t *testing.T) { _, _ = io.Copy(io.Discard, res.Body) return res.StatusCode == http.StatusAccepted - }, time.Minute, time.Second) + }, testutil.WaitShort, testutil.IntervalSlow) assert.NoError(t, server.Close()) cancelTun() select { case <-errCh: - case <-time.After(10 * time.Second): - t.Error("tunnel did not close after 10 seconds") + case <-time.After(testutil.WaitLong): + t.Errorf("tunnel did not close after %s", testutil.WaitLong) } } @@ -226,7 +227,7 @@ func (f *fakeTunnelServer) requestHTTP() (*http.Response, error) { } client := &http.Client{ Transport: transport, - Timeout: 10 * time.Second, + Timeout: testutil.WaitLong, } return client.Get(fmt.Sprintf("http://[%s]:8090", clientIP)) } diff --git a/coderd/httpmw/ratelimit_test.go b/coderd/httpmw/ratelimit_test.go index c2e4ebd41fd78..132615d669ab7 100644 --- a/coderd/httpmw/ratelimit_test.go +++ b/coderd/httpmw/ratelimit_test.go @@ -4,12 +4,12 @@ import ( "net/http" "net/http/httptest" "testing" - "time" "github.com/go-chi/chi/v5" "github.com/stretchr/testify/require" "github.com/coder/coder/coderd/httpmw" + "github.com/coder/coder/testutil" ) func TestRateLimit(t *testing.T) { @@ -27,6 +27,6 @@ func TestRateLimit(t *testing.T) { rec := httptest.NewRecorder() rtr.ServeHTTP(rec, req) return rec.Result().StatusCode == http.StatusTooManyRequests - }, 5*time.Second, time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) }) } diff --git a/coderd/provisionerdaemons_test.go b/coderd/provisionerdaemons_test.go index 1a89bb4e701a8..709dc2eb0c092 100644 --- a/coderd/provisionerdaemons_test.go +++ b/coderd/provisionerdaemons_test.go @@ -5,7 +5,6 @@ import ( "crypto/rand" "runtime" "testing" - "time" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" @@ -13,6 +12,7 @@ import ( "github.com/coder/coder/coderd/coderdtest" "github.com/coder/coder/codersdk" "github.com/coder/coder/provisionersdk" + "github.com/coder/coder/testutil" ) func TestProvisionerDaemons(t *testing.T) { @@ -41,7 +41,7 @@ func TestProvisionerDaemons(t *testing.T) { var err error version, err = client.TemplateVersion(context.Background(), version.ID) return assert.NoError(t, err) && version.Job.Error != "" - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) }) } diff --git a/coderd/provisionerjobs_internal_test.go b/coderd/provisionerjobs_internal_test.go index 6a5cbcccfe5cd..4d215f6bb2a92 100644 --- a/coderd/provisionerjobs_internal_test.go +++ b/coderd/provisionerjobs_internal_test.go @@ -21,6 +21,7 @@ import ( "github.com/coder/coder/coderd/database" "github.com/coder/coder/coderd/database/databasefake" "github.com/coder/coder/codersdk" + "github.com/coder/coder/testutil" ) func TestProvisionerJobLogs_Unit(t *testing.T) { @@ -63,7 +64,7 @@ func TestProvisionerJobLogs_Unit(t *testing.T) { {ID: uuid.New(), JobID: jobID, Stage: "Stage3"}, } - ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancel() // wow there are a lot of DB rows we touch... diff --git a/coderd/templateversions_test.go b/coderd/templateversions_test.go index 8706c77d25019..9c6c76a927fdd 100644 --- a/coderd/templateversions_test.go +++ b/coderd/templateversions_test.go @@ -14,6 +14,7 @@ import ( "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" ) func TestTemplateVersion(t *testing.T) { @@ -120,7 +121,7 @@ func TestPatchCancelTemplateVersion(t *testing.T) { } t.Logf("Status: %s", version.Job.Status) return version.Job.Status == codersdk.ProvisionerJobRunning - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) err := client.CancelTemplateVersion(context.Background(), version.ID) require.NoError(t, err) err = client.CancelTemplateVersion(context.Background(), version.ID) @@ -131,7 +132,7 @@ func TestPatchCancelTemplateVersion(t *testing.T) { var err error version, err = client.TemplateVersion(context.Background(), version.ID) return assert.NoError(t, err) && version.Job.Status == codersdk.ProvisionerJobFailed - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) }) // TODO(Cian): until we are able to test cancellation properly, validating // Running -> Canceling is the best we can do for now. @@ -155,7 +156,7 @@ func TestPatchCancelTemplateVersion(t *testing.T) { } t.Logf("Status: %s", version.Job.Status) return version.Job.Status == codersdk.ProvisionerJobRunning - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) err := client.CancelTemplateVersion(context.Background(), version.ID) require.NoError(t, err) require.Eventually(t, func() bool { @@ -166,7 +167,7 @@ func TestPatchCancelTemplateVersion(t *testing.T) { // provision complete response. assert.Empty(t, version.Job.Error) && version.Job.Status == codersdk.ProvisionerJobCanceling - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) }) } @@ -541,7 +542,7 @@ func TestTemplateVersionDryRun(t *testing.T) { require.Eventually(t, func() bool { job, err := client.TemplateVersionDryRun(ctx, version.ID, job.ID) return assert.NoError(t, err) && job.Status == codersdk.ProvisionerJobSucceeded - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) <-logsDone @@ -618,7 +619,7 @@ func TestTemplateVersionDryRun(t *testing.T) { t.Logf("Status: %s", job.Status) return job.Status == codersdk.ProvisionerJobPending - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) err = client.CancelTemplateVersionDryRun(context.Background(), version.ID, job.ID) require.NoError(t, err) @@ -631,7 +632,7 @@ func TestTemplateVersionDryRun(t *testing.T) { t.Logf("Status: %s", job.Status) return job.Status == codersdk.ProvisionerJobCanceling - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) }) t.Run("AlreadyCompleted", func(t *testing.T) { @@ -655,7 +656,7 @@ func TestTemplateVersionDryRun(t *testing.T) { t.Logf("Status: %s", job.Status) return job.Status == codersdk.ProvisionerJobSucceeded - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) err = client.CancelTemplateVersionDryRun(context.Background(), version.ID, job.ID) var apiErr *codersdk.Error diff --git a/coderd/workspacebuilds_test.go b/coderd/workspacebuilds_test.go index 56f705940a72b..8b68cff559c94 100644 --- a/coderd/workspacebuilds_test.go +++ b/coderd/workspacebuilds_test.go @@ -17,6 +17,7 @@ import ( "github.com/coder/coder/codersdk" "github.com/coder/coder/provisioner/echo" "github.com/coder/coder/provisionersdk/proto" + "github.com/coder/coder/testutil" ) func TestWorkspaceBuild(t *testing.T) { @@ -222,7 +223,7 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) { var err error build, err = client.WorkspaceBuild(context.Background(), workspace.LatestBuild.ID) return assert.NoError(t, err) && build.Job.Status == codersdk.ProvisionerJobRunning - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) err := client.CancelWorkspaceBuild(context.Background(), build.ID) require.NoError(t, err) require.Eventually(t, func() bool { @@ -233,7 +234,7 @@ func TestPatchCancelWorkspaceBuild(t *testing.T) { // provision complete response. assert.Empty(t, build.Job.Error) && build.Job.Status == codersdk.ProvisionerJobCanceling - }, 5*time.Second, 25*time.Millisecond) + }, testutil.WaitShort, testutil.IntervalFast) } func TestWorkspaceBuildResources(t *testing.T) { diff --git a/peer/conn_test.go b/peer/conn_test.go index d1fbf63d15ab6..6e2de345f0d9e 100644 --- a/peer/conn_test.go +++ b/peer/conn_test.go @@ -21,6 +21,7 @@ import ( "cdr.dev/slog" "cdr.dev/slog/sloggers/slogtest" "github.com/coder/coder/peer" + "github.com/coder/coder/testutil" ) var ( @@ -109,7 +110,7 @@ func TestConn(t *testing.T) { t.Parallel() client, server, _ := createPair(t) exchange(t, client, server) - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() cch, err := client.CreateChannel(ctx, "hello", &peer.ChannelOptions{}) require.NoError(t, err) @@ -128,7 +129,7 @@ func TestConn(t *testing.T) { t.Parallel() client, server, wan := createPair(t) exchange(t, client, server) - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() cch, err := client.CreateChannel(ctx, "hello", &peer.ChannelOptions{}) require.NoError(t, err) @@ -148,7 +149,7 @@ func TestConn(t *testing.T) { t.Parallel() client, server, _ := createPair(t) exchange(t, client, server) - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() cch, err := client.CreateChannel(ctx, "hello", &peer.ChannelOptions{}) require.NoError(t, err) @@ -196,7 +197,7 @@ func TestConn(t *testing.T) { srv, err := net.Listen("tcp", "127.0.0.1:0") require.NoError(t, err) defer srv.Close() - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() go func() { sch, err := server.Accept(ctx) @@ -314,7 +315,7 @@ func TestConn(t *testing.T) { t.Parallel() client, server, _ := createPair(t) exchange(t, client, server) - ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second) + ctx, cancel := context.WithTimeout(context.Background(), testutil.WaitLong) defer cancel() go func() { channel, err := client.CreateChannel(ctx, "test", nil) diff --git a/provisionerd/provisionerd_test.go b/provisionerd/provisionerd_test.go index fd724babea853..05becb863ebd7 100644 --- a/provisionerd/provisionerd_test.go +++ b/provisionerd/provisionerd_test.go @@ -13,6 +13,7 @@ import ( "time" "github.com/coder/coder/provisionerd/runner" + "github.com/coder/coder/testutil" "github.com/hashicorp/yamux" "github.com/stretchr/testify/assert" @@ -69,7 +70,7 @@ func TestProvisionerd(t *testing.T) { defer close(completeChan) return nil, xerrors.New("an error") }, provisionerd.Provisioners{}) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, closer.Close()) }) @@ -92,7 +93,7 @@ func TestProvisionerd(t *testing.T) { updateJob: noopUpdateJob, }), nil }, provisionerd.Provisioners{}) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, closer.Close()) }) @@ -137,7 +138,7 @@ func TestProvisionerd(t *testing.T) { }), }) closerMutex.Unlock() - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, closer.Close()) }) @@ -175,7 +176,7 @@ func TestProvisionerd(t *testing.T) { }, provisionerd.Provisioners{ "someprovisioner": createProvisionerClient(t, provisionerTestServer{}), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, closer.Close()) }) @@ -218,7 +219,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, closer.Close()) }) @@ -323,7 +324,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.True(t, didLog.Load()) require.True(t, didComplete.Load()) require.True(t, didDryRun.Load()) @@ -403,7 +404,7 @@ func TestProvisionerd(t *testing.T) { }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.True(t, didLog.Load()) require.True(t, didComplete.Load()) require.NoError(t, closer.Close()) @@ -474,7 +475,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.True(t, didLog.Load()) require.True(t, didComplete.Load()) require.NoError(t, closer.Close()) @@ -529,7 +530,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.True(t, didFail.Load()) require.NoError(t, closer.Close()) }) @@ -602,10 +603,10 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(updateChan, 5*time.Second)) + require.Condition(t, closedWithin(updateChan, testutil.WaitShort)) err := server.Shutdown(context.Background()) require.NoError(t, err) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, server.Close()) }) @@ -685,8 +686,8 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(updateChan, 5*time.Second)) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(updateChan, testutil.WaitShort)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, server.Close()) }) @@ -758,7 +759,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, server.Close()) }) @@ -830,7 +831,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) require.NoError(t, server.Close()) }) @@ -915,7 +916,7 @@ func TestProvisionerd(t *testing.T) { }, }), }) - require.Condition(t, closedWithin(completeChan, 5*time.Second)) + require.Condition(t, closedWithin(completeChan, testutil.WaitShort)) m.Lock() defer m.Unlock() require.Equal(t, ops[len(ops)-1], "CompleteJob") diff --git a/pty/ptytest/ptytest.go b/pty/ptytest/ptytest.go index dc22351416c50..2c79ae1991d1d 100644 --- a/pty/ptytest/ptytest.go +++ b/pty/ptytest/ptytest.go @@ -17,6 +17,7 @@ import ( "golang.org/x/xerrors" "github.com/coder/coder/pty" + "github.com/coder/coder/testutil" ) func New(t *testing.T) *PTY { @@ -85,7 +86,7 @@ type PTY struct { func (p *PTY) ExpectMatch(str string) string { p.t.Helper() - timeout, cancel := context.WithTimeout(context.Background(), 10*time.Second) + timeout, cancel := context.WithTimeout(context.Background(), testutil.WaitMedium) defer cancel() var buffer bytes.Buffer diff --git a/scripts/rules.go b/scripts/rules.go index 16917101d16e9..d77289388ba1c 100644 --- a/scripts/rules.go +++ b/scripts/rules.go @@ -81,6 +81,43 @@ func doNotCallTFailNowInsideGoroutine(m dsl.Matcher) { Report("Do not call functions that may call t.FailNow in a goroutine, as this can cause data races (see testing.go:834)") } +// useStandardTimeoutsAndDelaysInTests ensures all tests use common +// constants for timeouts and delays in usual scenarios, this allows us +// to tweak them based on platform (important to avoid CI flakes). +//nolint:unused,deadcode,varnamelen +func useStandardTimeoutsAndDelaysInTests(m dsl.Matcher) { + m.Import("github.com/stretchr/testify/require") + m.Import("github.com/stretchr/testify/assert") + m.Import("github.com/coder/coder/testutil") + + m.Match(`context.WithTimeout($ctx, $duration)`). + Where(m.File().Imports("testing") && !m["duration"].Text.Matches("^testutil\\.")). + At(m["duration"]). + Report("Do not use magic numbers in test timeouts and delays. Use the standard testutil.Wait* or testutil.Interval* constants instead.") + + m.Match(` + $testify.$Eventually($t, func() bool { + $*_ + }, $timeout, $interval, $*_) + `). + Where((m["testify"].Text == "require" || m["testify"].Text == "assert") && + (m["Eventually"].Text == "Eventually" || m["Eventually"].Text == "Eventuallyf") && + !m["timeout"].Text.Matches("^testutil\\.")). + At(m["timeout"]). + Report("Do not use magic numbers in test timeouts and delays. Use the standard testutil.Wait* or testutil.Interval* constants instead.") + + m.Match(` + $testify.$Eventually($t, func() bool { + $*_ + }, $timeout, $interval, $*_) + `). + Where((m["testify"].Text == "require" || m["testify"].Text == "assert") && + (m["Eventually"].Text == "Eventually" || m["Eventually"].Text == "Eventuallyf") && + !m["interval"].Text.Matches("^testutil\\.")). + At(m["interval"]). + Report("Do not use magic numbers in test timeouts and delays. Use the standard testutil.Wait* or testutil.Interval* constants instead.") +} + // InTx checks to ensure the database used inside the transaction closure is the transaction // database, and not the original database that creates the tx. func InTx(m dsl.Matcher) { diff --git a/site/site_test.go b/site/site_test.go index 27b6edcbbf527..7b28ac91b3a6a 100644 --- a/site/site_test.go +++ b/site/site_test.go @@ -20,6 +20,7 @@ import ( "github.com/stretchr/testify/require" "github.com/coder/coder/site" + "github.com/coder/coder/testutil" ) func TestCaching(t *testing.T) { @@ -48,7 +49,7 @@ func TestCaching(t *testing.T) { defer srv.Close() // Create a context - ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancelFunc() testCases := []struct { @@ -108,7 +109,7 @@ func TestServingFiles(t *testing.T) { defer srv.Close() // Create a context - ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancelFunc() testCases := []struct { @@ -337,7 +338,7 @@ func TestServingBin(t *testing.T) { defer srv.Close() // Create a context - ctx, cancelFunc := context.WithTimeout(context.Background(), 5*time.Second) + ctx, cancelFunc := context.WithTimeout(context.Background(), testutil.WaitShort) defer cancelFunc() for _, tr := range tt.reqs { diff --git a/testutil/duration.go b/testutil/duration.go new file mode 100644 index 0000000000000..8e3210f155178 --- /dev/null +++ b/testutil/duration.go @@ -0,0 +1,21 @@ +//go:build !windows + +package testutil + +import "time" + +// Constants for timing out operations, usable for creating contexts +// that timeout or in require.Eventually. +const ( + WaitShort = 5 * time.Second + WaitMedium = 10 * time.Second + WaitLong = 15 * time.Second +) + +// Constants for delaying repeated operations, e.g. in +// require.Eventually. +const ( + IntervalFast = 25 * time.Millisecond + IntervalMedium = 250 * time.Millisecond + IntervalSlow = time.Second +) diff --git a/testutil/duration_windows.go b/testutil/duration_windows.go new file mode 100644 index 0000000000000..5707824800bbf --- /dev/null +++ b/testutil/duration_windows.go @@ -0,0 +1,23 @@ +package testutil + +import "time" + +// Constants for timing out operations, usable for creating contexts +// that timeout or in require.Eventually. +// +// Windows durations are adjusted for slow CI workers. +const ( + WaitShort = 10 * time.Second + WaitMedium = 20 * time.Second + WaitLong = 30 * time.Second +) + +// Constants for delaying repeated operations, e.g. in +// require.Eventually. +// +// Windows durations are adjusted for slow CI workers. +const ( + IntervalFast = 50 * time.Millisecond + IntervalMedium = 500 * time.Millisecond + IntervalSlow = 2 * time.Second +)