From 6bb88775ab75ce72b9403cff8b848d4a49879b65 Mon Sep 17 00:00:00 2001 From: Michael Suchacz <203725896+ibetitsmike@users.noreply.github.com> Date: Mon, 11 May 2026 19:53:58 +0200 Subject: [PATCH] test(coderd/x/chatd): pin TestGetWorkspaceConn_StatusCheck to mock clock (#25130) The `TimedOutAgentCacheHit`, `CacheHitHealthyAgent`, and `CacheHitDBError` subtests of `TestGetWorkspaceConn_StatusCheck` built their `WorkspaceAgent` timestamps with `time.Now()` in the parent test's slice literal and then ran the actual check against the server's real wall clock (`quartz.NewReal()`). On slow Windows CI runners, more than `agentInactiveDisconnectTimeout` (30s) of wall time can elapse between slice construction and the parallel subtest body. In that window, the cached "healthy" agent gets reclassified as disconnected by `agentDisconnectedFor`, and `CacheHitHealthyAgent` fails with `errChatAgentDisconnected` instead of returning the cached connection. Build each agent inside the subtest with `quartz.NewMock(t)` and feed the same clock into the `Server` so the agent timestamps and the status math share a single frozen `now`. This matches the pattern already used by `TestGetWorkspaceConn_DialTimeoutDisconnectedRecoveryThreshold` in the same file. Closes https://github.com/coder/internal/issues/1522
Verification Inserting `time.Sleep(35 * time.Second)` at the top of each subtest's body reliably reproduces the original failure (`errChatAgentDisconnected` on `CacheHitHealthyAgent`) on the parent commit and passes with this change. After removing the synthetic sleep, `go test ./coderd/x/chatd -run TestGetWorkspaceConn_StatusCheck -count=50` passes cleanly.
> Generated by Coder Agents on behalf of the assignee. Co-authored-by: Coder Agents --- coderd/x/chatd/chatd_internal_test.go | 69 ++++++++++++++++----------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/coderd/x/chatd/chatd_internal_test.go b/coderd/x/chatd/chatd_internal_test.go index 1f80e6c81d..0e2d9c8242 100644 --- a/coderd/x/chatd/chatd_internal_test.go +++ b/coderd/x/chatd/chatd_internal_test.go @@ -4259,9 +4259,9 @@ func TestGetWorkspaceConn_StatusCheck(t *testing.T) { t.Parallel() type testCase struct { - name string - agent database.WorkspaceAgent - dbError bool + name string + buildAgent func(now time.Time) database.WorkspaceAgent + dbError bool } tests := []testCase{ @@ -4271,37 +4271,43 @@ func TestGetWorkspaceConn_StatusCheck(t *testing.T) { // recovery because the agent did not connect and // then disconnect. name: "TimedOutAgentCacheHit", - agent: database.WorkspaceAgent{ - CreatedAt: time.Now().Add(-10 * time.Minute), - ConnectionTimeoutSeconds: 60, + buildAgent: func(now time.Time) database.WorkspaceAgent { + return database.WorkspaceAgent{ + CreatedAt: now.Add(-10 * time.Minute), + ConnectionTimeoutSeconds: 60, + } }, }, { name: "CacheHitHealthyAgent", - agent: database.WorkspaceAgent{ - FirstConnectedAt: sql.NullTime{ - Time: time.Now().Add(-5 * time.Minute), - Valid: true, - }, - LastConnectedAt: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, + buildAgent: func(now time.Time) database.WorkspaceAgent { + return database.WorkspaceAgent{ + FirstConnectedAt: sql.NullTime{ + Time: now.Add(-5 * time.Minute), + Valid: true, + }, + LastConnectedAt: sql.NullTime{ + Time: now, + Valid: true, + }, + } }, }, { // When GetWorkspaceAgentByID returns an error on // cache hit, the cached connection should be returned. name: "CacheHitDBError", - agent: database.WorkspaceAgent{ - FirstConnectedAt: sql.NullTime{ - Time: time.Now().Add(-5 * time.Minute), - Valid: true, - }, - LastConnectedAt: sql.NullTime{ - Time: time.Now(), - Valid: true, - }, + buildAgent: func(now time.Time) database.WorkspaceAgent { + return database.WorkspaceAgent{ + FirstConnectedAt: sql.NullTime{ + Time: now.Add(-5 * time.Minute), + Valid: true, + }, + LastConnectedAt: sql.NullTime{ + Time: now, + Valid: true, + }, + } }, dbError: true, }, @@ -4329,8 +4335,17 @@ func TestGetWorkspaceConn_StatusCheck(t *testing.T) { }, } - // Stamp the agent with the generated ID. - agent := tc.agent + // Stamp the agent with the generated ID. Use the + // subtest's mock clock so the agent's timestamps are + // anchored to the same `now` the server uses. Using + // time.Now() at slice-literal construction time + // produced a Windows-CI flake because a slow scheduler + // could insert more than agentInactiveDisconnectTimeout + // of wall-clock delay between the literal and the + // subtest body. + clock := quartz.NewMock(t) + now := clock.Now() + agent := tc.buildAgent(now) agent.ID = agentID // Set up the DB mock for GetWorkspaceAgentByID. @@ -4349,7 +4364,7 @@ func TestGetWorkspaceConn_StatusCheck(t *testing.T) { server := &Server{ db: db, logger: slogtest.Make(t, &slogtest.Options{IgnoreErrors: true}), - clock: quartz.NewReal(), + clock: clock, agentInactiveDisconnectTimeout: 30 * time.Second, dialTimeout: defaultDialTimeout, }