Skip to content

Drive the circuit breaker's clock from tests, not sleep() #619

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
jeremy merged 1 commit into from
Aug 4, 2026
+73 −30
Merged

Drive the circuit breaker's clock from tests, not sleep() #619

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion internal/resilience/circuit_breaker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ import (
type CircuitBreaker struct {
config CircuitBreakerConfig
store *Store

// nowFn supplies the current time. Every time read in this type goes
// through now(), so tests can substitute a clock they advance by hand
// and drive the open/half-open timeouts without sleeping.
nowFn func() time.Time
}

// NewCircuitBreaker creates a new circuit breaker with the given config.
Expand All @@ -26,12 +31,13 @@ func NewCircuitBreaker(store *Store, config CircuitBreakerConfig) *CircuitBreake
return &CircuitBreaker{
config: config,
store: store,
nowFn: time.Now,
}
}

// now returns the current time.
func (cb *CircuitBreaker) now() time.Time {
return time.Now()
return cb.nowFn()
}

// Allow checks if a request should be allowed.
Expand Down
71 changes: 50 additions & 21 deletions internal/resilience/circuit_breaker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,38 @@ package resilience
import (
"os"
"path/filepath"
"sync"
"testing"
"time"

"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)

// fakeClock is a hand-advanced clock for driving circuit breaker timeouts.
// Tests assign its Now to CircuitBreaker.nowFn so open/half-open transitions
// happen because the test moved time, not because it slept long enough.
type fakeClock struct {
mu sync.Mutex
now time.Time
}

func newFakeClock() *fakeClock {
return &fakeClock{now: time.Date(2026, 1, 1, 0, 0, 0, 0, time.UTC)}
}

func (c *fakeClock) Now() time.Time {
c.mu.Lock()
defer c.mu.Unlock()
return c.now
}

func (c *fakeClock) Advance(d time.Duration) {
c.mu.Lock()
defer c.mu.Unlock()
c.now = c.now.Add(d)
}

func TestCircuitBreakerDefaultsClosed(t *testing.T) {
dir := t.TempDir()
store := NewStore(dir)
Expand Down Expand Up @@ -61,19 +86,21 @@ func TestCircuitBreakerClosesAfterSuccesses(t *testing.T) {
dir := t.TempDir()
store := NewStore(dir)

clock := newFakeClock()
cb := NewCircuitBreaker(store, CircuitBreakerConfig{
FailureThreshold: 3,
SuccessThreshold: 2,
OpenTimeout: 1 * time.Millisecond, // Very short timeout for testing
OpenTimeout: 30 * time.Second,
})
cb.nowFn = clock.Now

// Open the circuit
for range 3 {
cb.RecordFailure()
}

// Wait for timeout to allow transition to half-open
time.Sleep(10 * time.Millisecond)
// Move past the open timeout to allow transition to half-open
clock.Advance(31 * time.Second)

// This Allow() should trigger transition to half-open
allowed, _ := cb.Allow()
Expand Down Expand Up @@ -246,11 +273,13 @@ func TestCircuitBreakerStateTransitionsCorrectly(t *testing.T) {
dir := t.TempDir()
store := NewStore(dir)

clock := newFakeClock()
cb := NewCircuitBreaker(store, CircuitBreakerConfig{
FailureThreshold: 2,
SuccessThreshold: 1,
OpenTimeout: 500 * time.Millisecond,
OpenTimeout: 30 * time.Second,
})
cb.nowFn = clock.Now

// Start closed
state, _ := cb.State()
Expand All @@ -262,8 +291,8 @@ func TestCircuitBreakerStateTransitionsCorrectly(t *testing.T) {
state, _ = cb.State()
assert.Equal(t, CircuitOpen, state)

// Wait -> half-open
time.Sleep(600 * time.Millisecond)
// Past the open timeout -> half-open
clock.Advance(31 * time.Second)
state, _ = cb.State()
assert.Equal(t, CircuitHalfOpen, state)

Expand All @@ -278,24 +307,25 @@ func TestCircuitBreakerResetsStaleHalfOpenAttempts(t *testing.T) {
dir := t.TempDir()
store := NewStore(dir)

// Use longer timeouts for CI stability (50ms instead of 10ms)
openTimeout := 50 * time.Millisecond
staleTimeout := 100 * time.Millisecond
openTimeout := 30 * time.Second
staleTimeout := 2 * time.Minute

clock := newFakeClock()
cb := NewCircuitBreaker(store, CircuitBreakerConfig{
FailureThreshold: 2,
SuccessThreshold: 1,
OpenTimeout: openTimeout,
HalfOpenMaxRequests: 1,
StaleAttemptTimeout: staleTimeout,
})
cb.nowFn = clock.Now

// Open the circuit
cb.RecordFailure()
cb.RecordFailure()

// Wait for timeout to allow half-open
time.Sleep(openTimeout * 2)
// Move past the open timeout to allow half-open
clock.Advance(openTimeout * 2)

// First Allow() transitions to half-open and reserves a slot
allowed, err := cb.Allow()
Expand All @@ -310,8 +340,8 @@ func TestCircuitBreakerResetsStaleHalfOpenAttempts(t *testing.T) {
require.NoError(t, err)
assert.False(t, allowed, "expected second request to be rejected when half-open slots exhausted")

// Wait for stale timeout period
time.Sleep(staleTimeout + 50*time.Millisecond)
// Move past the stale timeout
clock.Advance(staleTimeout + time.Second)

// Now Allow() should reset stale attempts and allow
allowed, err = cb.Allow()
Expand All @@ -323,38 +353,37 @@ func TestCircuitBreakerSetsHalfOpenLastAttemptAt(t *testing.T) {
dir := t.TempDir()
store := NewStore(dir)

// Use longer timeouts for CI stability
openTimeout := 50 * time.Millisecond
openTimeout := 30 * time.Second

clock := newFakeClock()
cb := NewCircuitBreaker(store, CircuitBreakerConfig{
FailureThreshold: 2,
SuccessThreshold: 1,
OpenTimeout: openTimeout,
HalfOpenMaxRequests: 1,
})
cb.nowFn = clock.Now

// Open the circuit
cb.RecordFailure()
cb.RecordFailure()

// Wait for timeout to allow half-open
time.Sleep(openTimeout * 2)
// Move past the open timeout to allow half-open
clock.Advance(openTimeout * 2)

// Check that HalfOpenLastAttemptAt is zero before Allow()
state, _ := store.Load()
assert.True(t, state.CircuitBreaker.HalfOpenLastAttemptAt.IsZero(), "expected HalfOpenLastAttemptAt to be zero before Allow()")

// First Allow() transitions to half-open and reserves a slot
before := time.Now()
allowed, err := cb.Allow()
after := time.Now()
require.NoError(t, err)
assert.True(t, allowed, "expected first request to be allowed")

// HalfOpenLastAttemptAt should be set
// HalfOpenLastAttemptAt should be stamped with the reservation time
state, _ = store.Load()
assert.False(t, state.CircuitBreaker.HalfOpenLastAttemptAt.IsZero(), "expected HalfOpenLastAttemptAt to be set after Allow()")
assert.False(t, state.CircuitBreaker.HalfOpenLastAttemptAt.Before(before) || state.CircuitBreaker.HalfOpenLastAttemptAt.After(after), "HalfOpenLastAttemptAt should be between before and after Allow()")
assert.True(t, state.CircuitBreaker.HalfOpenLastAttemptAt.Equal(clock.Now()), "expected HalfOpenLastAttemptAt to be the time of the Allow() that reserved the slot")
}

func TestCircuitBreakerResetsStaleAttemptsWithZeroTimestamp(t *testing.T) {
Expand Down
24 changes: 16 additions & 8 deletions internal/resilience/hooks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,9 +269,8 @@ func TestGatingHooksResetsStaleHalfOpenAttemptsIntegration(t *testing.T) {
dir := t.TempDir()
store := NewStore(dir)

// Use longer timeouts for CI stability
openTimeout := 50 * time.Millisecond
staleTimeout := 100 * time.Millisecond
openTimeout := 30 * time.Second
staleTimeout := 2 * time.Minute

cfg := &Config{
CircuitBreaker: CircuitBreakerConfig{
Expand All @@ -291,7 +290,16 @@ func TestGatingHooksResetsStaleHalfOpenAttemptsIntegration(t *testing.T) {
},
}

hooks := NewGatingHooksFromConfig(store, cfg)
// Assemble the hooks by hand rather than via NewGatingHooksFromConfig so
// the circuit breaker runs on a clock this test advances.
clock := newFakeClock()
newHooks := func() *GatingHooks {
cb := NewCircuitBreaker(store, cfg.CircuitBreaker)
cb.nowFn = clock.Now
return NewGatingHooks(cb, NewRateLimiter(store, cfg.RateLimiter), NewBulkhead(store, cfg.Bulkhead))
}

hooks := newHooks()

op := basecamp.OperationInfo{
Service: "Todos",
Expand All @@ -305,8 +313,8 @@ func TestGatingHooksResetsStaleHalfOpenAttemptsIntegration(t *testing.T) {
ctx2, _ := hooks.OnOperationGate(context.Background(), op)
hooks.OnOperationEnd(ctx2, op, networkErr, time.Millisecond)

// Wait for timeout to allow half-open
time.Sleep(openTimeout * 2)
// Move past the open timeout to allow half-open
clock.Advance(openTimeout * 2)

// First request transitions to half-open and reserves the slot
ctx3, err := hooks.OnOperationGate(context.Background(), op)
Expand All @@ -325,12 +333,12 @@ func TestGatingHooksResetsStaleHalfOpenAttemptsIntegration(t *testing.T) {
store.Update(func(state *State) error {
state.CircuitBreaker.State = CircuitHalfOpen
state.CircuitBreaker.HalfOpenAttempts = 1
state.CircuitBreaker.HalfOpenLastAttemptAt = time.Now().Add(-staleTimeout * 2) // Beyond stale threshold
state.CircuitBreaker.HalfOpenLastAttemptAt = clock.Now().Add(-staleTimeout * 2) // Beyond stale threshold
return nil
})

// Create fresh hooks to simulate new process
hooks2 := NewGatingHooksFromConfig(store, cfg)
hooks2 := newHooks()

// This should detect stale attempts and allow the request
_, err = hooks2.OnOperationGate(context.Background(), op)
Expand Down
Loading