fix: address PR review comments on heartbeat tests

- Use registerActiveTurn/clearActiveTurn helpers instead of direct map access
- Update assertions to check HEARTBEAT_SKIPPED (matching new sentinel)
- Strengthen idle/proceeds tests to assert Mock response and err==nil
This commit is contained in:
Kristjan Kruus 2026-03-24 21:01:07 +02:00
parent 04411bd55d
commit 5efeb19c85

View file

@ -2137,22 +2137,22 @@ func TestProcessHeartbeat_SkipsWhenAgentBusy(t *testing.T) {
al, _, _, _, cleanup := newTestAgentLoop(t) al, _, _, _, cleanup := newTestAgentLoop(t)
defer cleanup() defer cleanup()
// Simulate an active turn by storing a turnState directly // Simulate an active turn by registering a turnState via the helper
busyTS := &turnState{ busyTS := &turnState{
sessionKey: "user-chat", sessionKey: "user-chat",
agentID: "main", agentID: "main",
phase: TurnPhaseRunning, phase: TurnPhaseRunning,
startedAt: time.Now(), startedAt: time.Now(),
} }
al.activeTurnStates.Store(busyTS.sessionKey, busyTS) al.registerActiveTurn(busyTS)
defer al.activeTurnStates.Delete(busyTS.sessionKey) defer al.clearActiveTurn(busyTS)
resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "telegram", "chat123") resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "telegram", "chat123")
if err != nil { if err != nil {
t.Fatalf("ProcessHeartbeat returned error: %v", err) t.Fatalf("ProcessHeartbeat returned error: %v", err)
} }
if resp != "HEARTBEAT_OK" { if resp != "HEARTBEAT_SKIPPED" {
t.Fatalf("expected HEARTBEAT_OK when busy, got %q", resp) t.Fatalf("expected HEARTBEAT_SKIPPED when busy, got %q", resp)
} }
} }
@ -2165,20 +2165,15 @@ func TestProcessHeartbeat_RunsWhenIdle(t *testing.T) {
t.Fatal("expected no active turns in fresh AgentLoop") t.Fatal("expected no active turns in fresh AgentLoop")
} }
// ProcessHeartbeat should proceed (not return early with HEARTBEAT_OK skip) // Given the mock provider, the heartbeat call should succeed with a fixed response
// and, with the mock provider used in newTestAgentLoop, return a deterministic // (which also implies it did not skip with HEARTBEAT_SKIPPED).
// mock response.
resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "cli", "direct") resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "cli", "direct")
// Must not skip due to busy check.
if resp == "HEARTBEAT_OK" {
t.Fatal("ProcessHeartbeat skipped despite no active turns")
}
// Given the mock provider, the heartbeat call should succeed with a fixed response.
if err != nil { if err != nil {
t.Fatalf("expected no error from ProcessHeartbeat when idle, got: %v", err) t.Fatalf("expected no error from ProcessHeartbeat when idle, got: %v", err)
} }
if resp == "HEARTBEAT_SKIPPED" {
t.Fatal("ProcessHeartbeat skipped despite no active turns")
}
if resp != "Mock response" { if resp != "Mock response" {
t.Fatalf("expected mock provider response %q, got %q", "Mock response", resp) t.Fatalf("expected mock provider response %q, got %q", "Mock response", resp)
} }
@ -2196,16 +2191,16 @@ func TestProcessHeartbeat_SkipsForAnyActiveSession(t *testing.T) {
phase: TurnPhaseRunning, phase: TurnPhaseRunning,
startedAt: time.Now(), startedAt: time.Now(),
} }
al.activeTurnStates.Store(key, ts) al.registerActiveTurn(ts)
defer al.activeTurnStates.Delete(key) defer al.clearActiveTurn(ts)
} }
resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "telegram", "chat123") resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "telegram", "chat123")
if err != nil { if err != nil {
t.Fatalf("ProcessHeartbeat returned error: %v", err) t.Fatalf("ProcessHeartbeat returned error: %v", err)
} }
if resp != "HEARTBEAT_OK" { if resp != "HEARTBEAT_SKIPPED" {
t.Fatalf("expected HEARTBEAT_OK when agent has active turns, got %q", resp) t.Fatalf("expected HEARTBEAT_SKIPPED when agent has active turns, got %q", resp)
} }
} }
@ -2220,25 +2215,28 @@ func TestProcessHeartbeat_ProceedsAfterTurnClears(t *testing.T) {
phase: TurnPhaseRunning, phase: TurnPhaseRunning,
startedAt: time.Now(), startedAt: time.Now(),
} }
al.activeTurnStates.Store(busyTS.sessionKey, busyTS) al.registerActiveTurn(busyTS)
// First call — should skip // First call — should skip
resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "cli", "direct") resp, err := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "cli", "direct")
if err != nil { if err != nil {
t.Fatalf("first ProcessHeartbeat returned error: %v", err) t.Fatalf("first ProcessHeartbeat returned error: %v", err)
} }
if resp != "HEARTBEAT_OK" { if resp != "HEARTBEAT_SKIPPED" {
t.Fatalf("expected skip on first call, got %q", resp) t.Fatalf("expected skip on first call, got %q", resp)
} }
// Clear the turn (simulating user conversation ending) // Clear the turn (simulating user conversation ending)
al.activeTurnStates.Delete(busyTS.sessionKey) al.clearActiveTurn(busyTS)
// Second call — should proceed (not skip) // Second call — should proceed (not skip)
resp2, err2 := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "cli", "direct") resp2, err2 := al.ProcessHeartbeat(context.Background(), "heartbeat prompt", "cli", "direct")
if err2 != nil { if err2 != nil {
t.Fatalf("second ProcessHeartbeat returned error: %v", err2) t.Fatalf("second ProcessHeartbeat returned error: %v", err2)
} }
if resp2 == "HEARTBEAT_SKIPPED" {
t.Fatal("ProcessHeartbeat still skipping after turn cleared")
}
if resp2 != "Mock response" { if resp2 != "Mock response" {
t.Fatalf("expected heartbeat to proceed and return %q, got %q", "Mock response", resp2) t.Fatalf("expected heartbeat to proceed and return %q, got %q", "Mock response", resp2)
} }