Skip to content

Commit 1f02f06

Browse files
mafredrimafredri
committed
fix(cli): use quartz clock in waitForTaskIdle for immediate first poll
waitForTaskIdle used time.NewTicker(5s) which delays the first poll by 5 seconds. Debugger tracing proved the failure mechanism: on slow CI (Windows), the first poll at 5s sees "working" (idle patch has not landed due to goroutine scheduling), needs poll #2 at 10s, but the 25s context expires before it fires. Two changes: 1. Use r.clock.NewTicker (quartz) with time.Nanosecond initial interval and Reset(5s) for immediate first poll. Tests inject a mock clock via clitest.NewWithClock for deterministic control. 2. Rewrite WaitsForWorkingAppState test with quartz traps (NewTicker + TickerReset) for deterministic synchronization instead of racing goroutines. Fix PausedDuringWaitForReady sync point. Closes https://linear.app/codercom/issue/DEVEX-381
1 parent a4afb9d commit 1f02f06

2 files changed

Lines changed: 32 additions & 7 deletions

File tree

‎cli/task_send.go‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ import (
1111

1212
"github.com/coder/coder/v2/cli/cliui"
1313
"github.com/coder/coder/v2/codersdk"
14+
"github.com/coder/quartz"
1415
"github.com/coder/serpent"
1516
)
1617

@@ -107,7 +108,7 @@ func (r *RootCmd) taskSend() *serpent.Command {
107108
return xerrors.Errorf("task %q has status %s and cannot be sent input", display, task.Status)
108109
}
109110

110-
if err := waitForTaskIdle(ctx, inv, client, task, workspaceBuildID); err != nil {
111+
if err := waitForTaskIdle(ctx, inv, r.clock, client, task, workspaceBuildID); err != nil {
111112
return xerrors.Errorf("wait for task %q to be idle: %w", display, err)
112113
}
113114

@@ -126,7 +127,7 @@ func (r *RootCmd) taskSend() *serpent.Command {
126127
// then polls until the task becomes active and its app state is idle.
127128
// This merges build-watching and idle-polling into a single loop so
128129
// that status changes (e.g. paused) are never missed between phases.
129-
func waitForTaskIdle(ctx context.Context, inv *serpent.Invocation, client *codersdk.Client, task codersdk.Task, workspaceBuildID uuid.UUID) error {
130+
func waitForTaskIdle(ctx context.Context, inv *serpent.Invocation, clk quartz.Clock, client *codersdk.Client, task codersdk.Task, workspaceBuildID uuid.UUID) error {
130131
if workspaceBuildID != uuid.Nil {
131132
if err := cliui.WorkspaceBuild(ctx, inv.Stdout, client, workspaceBuildID); err != nil {
132133
return xerrors.Errorf("watch workspace build: %w", err)
@@ -162,13 +163,15 @@ func waitForTaskIdle(ctx context.Context, inv *serpent.Invocation, client *coder
162163
// TODO(DanielleMaywood):
163164
// When we have a streaming Task API, this should be converted
164165
// away from polling.
165-
ticker := time.NewTicker(5 * time.Second)
166+
const pollInterval = 5 * time.Second
167+
ticker := clk.NewTicker(time.Nanosecond, "task_send", "poll")
166168
defer ticker.Stop()
167169
for {
168170
select {
169171
case <-ctx.Done():
170172
return ctx.Err()
171173
case <-ticker.C:
174+
ticker.Reset(pollInterval, "task_send", "poll")
172175
task, err := client.TaskByID(ctx, task.ID)
173176
if err != nil {
174177
return xerrors.Errorf("get task by id: %w", err)

‎cli/task_send_test.go‎

Lines changed: 26 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ import (
2121
"github.com/coder/coder/v2/codersdk/agentsdk"
2222
"github.com/coder/coder/v2/pty/ptytest"
2323
"github.com/coder/coder/v2/testutil"
24+
"github.com/coder/quartz"
2425
)
2526

2627
func Test_TaskSend(t *testing.T) {
@@ -255,10 +256,10 @@ func Test_TaskSend(t *testing.T) {
255256
w := clitest.StartWithWaiter(t, inv)
256257

257258
// Wait for the command to enter the build-watching phase
258-
// of waitForTaskReady.
259-
pty.ExpectMatchContext(ctx, "Queued")
259+
// of waitForTaskIdle.
260+
pty.ExpectMatchContext(ctx, "Waiting for task to become idle")
260261

261-
// Pause the task while waitForTaskReady is polling. Since
262+
// Pause the task while waitForTaskIdle is polling. Since
262263
// no agent is connected, the task stays initializing until
263264
// we pause it, at which point the status becomes paused.
264265
pauseTask(ctx, t, setup.userClient, setup.task)
@@ -284,21 +285,42 @@ func Test_TaskSend(t *testing.T) {
284285
Message: "busy",
285286
}))
286287

288+
// Set up mock clock and traps before starting the command.
289+
mClock := quartz.NewMock(t)
290+
tickTrap := mClock.Trap().NewTicker("task_send", "poll")
291+
resetTrap := mClock.Trap().TickerReset("task_send", "poll")
292+
287293
// When: We send input while the app is working.
288-
inv, root := clitest.New(t, "task", "send", setup.task.Name, "some task input")
294+
inv, root := clitest.NewWithClock(t, mClock, "task", "send", setup.task.Name, "some task input")
289295
clitest.SetupConfig(t, setup.userClient, root)
290296

291297
ctx := testutil.Context(t, testutil.WaitLong)
292298
inv = inv.WithContext(ctx)
293299
w := clitest.StartWithWaiter(t, inv)
294300

301+
// Wait for ticker creation and release it.
302+
tickCall := tickTrap.MustWait(ctx)
303+
tickCall.MustRelease(ctx)
304+
tickTrap.Close()
305+
306+
// Fire the immediate first poll (time.Nanosecond initial interval).
307+
mClock.Advance(time.Nanosecond).MustWait(ctx)
308+
309+
// Wait for Reset (confirms first poll completed and saw "working").
310+
resetCall := resetTrap.MustWait(ctx)
311+
resetCall.MustRelease(ctx)
312+
resetTrap.Close()
313+
295314
// Transition the app back to idle so waitForTaskIdle proceeds.
296315
require.NoError(t, agentClient.PatchAppStatus(ctx, agentsdk.PatchAppStatus{
297316
AppSlug: "task-sidebar",
298317
State: codersdk.WorkspaceAppStatusStateIdle,
299318
Message: "ready",
300319
}))
301320

321+
// Fire second poll at the regular 5s interval.
322+
mClock.Advance(5 * time.Second).MustWait(ctx)
323+
302324
// Then: The command should complete successfully.
303325
require.NoError(t, w.Wait())
304326
})

0 commit comments

Comments
 (0)