Skip to content

Commit 8ca7856

Browse files
committed
screentracker test: extract driveClockUntil helper
Extract a shared driveClockUntil helper that advances the mock clock one event at a time until a condition is met. Use it in both sendWithClockDrive and the initial prompt lifecycle test, replacing the ad-hoc 500-iteration loop. Also change sendWithClockDrive to return nothing (instead of error) since all call sites were wrapping it with require.NoError. The error check now happens inside via require.NoError.
1 parent 9a1b238 commit 8ca7856

1 file changed

Lines changed: 44 additions & 33 deletions

File tree

lib/screentracker/pty_conversation_test.go

Lines changed: 44 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -75,26 +75,52 @@ func fillToStable(ctx context.Context, t *testing.T, agent *testAgent, mClock *q
7575
}
7676
}
7777

78+
// driveClockUntil advances the mock clock one event at a time until done
79+
// returns true or the context is cancelled.
80+
func driveClockUntil(ctx context.Context, t *testing.T, mClock *quartz.Mock, done func() bool) {
81+
t.Helper()
82+
for i := 0; ; i++ {
83+
if done() {
84+
return
85+
}
86+
select {
87+
case <-ctx.Done():
88+
t.Fatal("context cancelled waiting for condition")
89+
default:
90+
}
91+
_, ok := mClock.Peek()
92+
if !ok {
93+
time.Sleep(time.Millisecond)
94+
continue
95+
}
96+
_, w := mClock.AdvanceNext()
97+
w.MustWait(ctx)
98+
// Periodically yield so conversation goroutines can process
99+
// clock events between advances.
100+
if i%10 == 0 {
101+
time.Sleep(time.Millisecond)
102+
}
103+
}
104+
}
105+
78106
// sendWithClockDrive calls Send() in a goroutine and advances the mock clock
79107
// until Send completes. This drives the snapshot loop (which signals
80108
// stableSignal) and writeStabilize (which creates mock timers).
81-
func sendWithClockDrive(ctx context.Context, t *testing.T, c *st.PTYConversation, mClock *quartz.Mock, parts ...st.MessagePart) error {
109+
func sendWithClockDrive(ctx context.Context, t *testing.T, c *st.PTYConversation, mClock *quartz.Mock, parts ...st.MessagePart) {
82110
t.Helper()
83111
errCh := make(chan error, 1)
84112
go func() {
85113
errCh <- c.Send(parts...)
86114
}()
87-
for {
115+
driveClockUntil(ctx, t, mClock, func() bool {
88116
select {
89117
case err := <-errCh:
90-
return err
91-
case <-ctx.Done():
92-
return ctx.Err()
118+
require.NoError(t, err)
119+
return true
93120
default:
121+
return false
94122
}
95-
_, w := mClock.AdvanceNext()
96-
w.MustWait(ctx)
97-
}
123+
})
98124
}
99125

100126
func assertMessages(t *testing.T, c *st.PTYConversation, expected []st.ConversationMessage) {
@@ -309,7 +335,7 @@ func TestMessages(t *testing.T) {
309335
fillToStable(ctx, t, agent, mClock, "2", interval, threshold)
310336

311337
// User message is recorded.
312-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "3"}))
338+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "3"})
313339

314340
// After send, screen is dirty from writeStabilize. Set to "4" and stabilize.
315341
fillToStable(ctx, t, agent, mClock, "4", interval, threshold)
@@ -321,7 +347,7 @@ func TestMessages(t *testing.T) {
321347

322348
// Agent message is updated when the screen changes before a user message.
323349
fillToStable(ctx, t, agent, mClock, "5", interval, threshold)
324-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "6"}))
350+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "6"})
325351

326352
fillToStable(ctx, t, agent, mClock, "7", interval, threshold)
327353
assertMessages(t, c, []st.ConversationMessage{
@@ -334,7 +360,7 @@ func TestMessages(t *testing.T) {
334360
assert.Equal(t, st.ConversationStatusStable, c.Status())
335361

336362
// Send another message.
337-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "8"}))
363+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "8"})
338364

339365
// After filling to stable, messages and status are correct.
340366
fillToStable(ctx, t, agent, mClock, "7", interval, threshold)
@@ -348,7 +374,7 @@ func TestMessages(t *testing.T) {
348374

349375
// Common overlap between screens is removed after a user message.
350376
fillToStable(ctx, t, agent, mClock, "1", interval, threshold)
351-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "2"}))
377+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "2"})
352378
fillToStable(ctx, t, agent, mClock, "1\n3", interval, threshold)
353379
assertMessages(t, c, []st.ConversationMessage{
354380
{Id: 0, Message: "1", Role: st.ConversationRoleAgent},
@@ -357,7 +383,7 @@ func TestMessages(t *testing.T) {
357383
})
358384

359385
fillToStable(ctx, t, agent, mClock, "1\n3x", interval, threshold)
360-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "4"}))
386+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "4"})
361387
fillToStable(ctx, t, agent, mClock, "1\n3x\n5", interval, threshold)
362388
assertMessages(t, c, []st.ConversationMessage{
363389
{Id: 0, Message: "1", Role: st.ConversationRoleAgent},
@@ -379,7 +405,7 @@ func TestMessages(t *testing.T) {
379405

380406
// Fill to stable with screen "1", then send.
381407
fillToStable(ctx, t, agent, mClock, "1", interval, threshold)
382-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "2"}))
408+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "2"})
383409

384410
// After send, set screen to "x" and take snapshots for new agent message.
385411
fillToStable(ctx, t, agent, mClock, "x", interval, threshold)
@@ -418,7 +444,7 @@ func TestMessages(t *testing.T) {
418444
assert.Equal(t, st.ConversationStatusStable, c.Status())
419445

420446
// Now send should succeed.
421-
require.NoError(t, sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "4"}))
447+
sendWithClockDrive(ctx, t, c, mClock, st.MessagePartText{Content: "4"})
422448

423449
// After send, screen is dirty. Set to "2" (different from "1") so status is changing.
424450
agent.setScreen("2")
@@ -529,24 +555,9 @@ func TestInitialPromptReadiness(t *testing.T) {
529555
// writeStabilize runs with onWrite changing the screen, so it completes.
530556
agent.setScreen("ready")
531557
// Drive clock until the initial prompt is sent (queue drains).
532-
for i := 0; i < 500; i++ {
533-
_, ok := mClock.Peek()
534-
if !ok {
535-
time.Sleep(1 * time.Millisecond)
536-
continue
537-
}
538-
_, w := mClock.AdvanceNext()
539-
w.MustWait(ctx)
540-
// Check if the queue has been drained by checking status.
541-
// After the initial prompt is sent, last message is user, so status
542-
// is "changing". Then after more snapshots, it becomes stable.
543-
// We just need to advance until the send loop has processed the message.
544-
// A simple heuristic: check if Messages() shows a user message.
545-
msgs := c.Messages()
546-
if len(msgs) >= 2 {
547-
break
548-
}
549-
}
558+
driveClockUntil(ctx, t, mClock, func() bool {
559+
return len(c.Messages()) >= 2
560+
})
550561

551562
// The initial prompt should have been sent. Set a clean screen and
552563
// advance enough ticks for the snapshot loop to record it as an

0 commit comments

Comments
 (0)