diff --git a/clock/testing/fake_clock.go b/clock/testing/fake_clock.go index 79e11deb..462c40c2 100644 --- a/clock/testing/fake_clock.go +++ b/clock/testing/fake_clock.go @@ -48,7 +48,6 @@ type fakeClockWaiter struct { stepInterval time.Duration skipIfBlocked bool destChan chan time.Time - fired bool afterFunc func() } @@ -198,12 +197,10 @@ func (f *FakeClock) setTimeLocked(t time.Time) { if w.skipIfBlocked { select { case w.destChan <- t: - w.fired = true default: } } else { w.destChan <- t - w.fired = true } if w.afterFunc != nil { @@ -305,44 +302,48 @@ func (f *fakeTimer) C() <-chan time.Time { return f.waiter.destChan } -// Stop stops the timer and returns true if the timer has not yet fired, or false otherwise. +// Stop prevents the Timer from firing. It returns true if the call stops the +// timer, false if the timer has already expired or been stopped. func (f *fakeTimer) Stop() bool { f.fakeClock.lock.Lock() defer f.fakeClock.lock.Unlock() + active := false newWaiters := make([]*fakeClockWaiter, 0, len(f.fakeClock.waiters)) for i := range f.fakeClock.waiters { w := f.fakeClock.waiters[i] if w != &f.waiter { newWaiters = append(newWaiters, w) + continue } + // If timer is found, it has not been fired yet. + active = true } f.fakeClock.waiters = newWaiters - return !f.waiter.fired + return active } -// Reset resets the timer to the fake clock's "now" + d. It returns true if the timer has not yet -// fired, or false otherwise. +// Reset changes the timer to expire after duration d. It returns true if the +// timer had been active, false if the timer had expired or been stopped. func (f *fakeTimer) Reset(d time.Duration) bool { f.fakeClock.lock.Lock() defer f.fakeClock.lock.Unlock() - active := !f.waiter.fired + active := false - f.waiter.fired = false f.waiter.targetTime = f.fakeClock.time.Add(d) - var isWaiting bool for i := range f.fakeClock.waiters { w := f.fakeClock.waiters[i] if w == &f.waiter { - isWaiting = true + // If timer is found, it has not been fired yet. + active = true break } } - if !isWaiting { + if !active { f.fakeClock.waiters = append(f.fakeClock.waiters, &f.waiter) } diff --git a/clock/testing/fake_clock_test.go b/clock/testing/fake_clock_test.go index db7ddbb5..94c2c8df 100644 --- a/clock/testing/fake_clock_test.go +++ b/clock/testing/fake_clock_test.go @@ -281,14 +281,19 @@ func TestFakeStop(t *testing.T) { if !tc.HasWaiters() { t.Errorf("expected a waiter to be present, but it is not") } - timer.Stop() + if !timer.Stop() { + t.Errorf("stop should return true as we are stopping an unexpired timer") + } if tc.HasWaiters() { t.Errorf("expected existing waiter to be cleaned up, but it is still present") } + if timer.Stop() { + t.Errorf("stop should return false as the timer has already been stopped") + } } // This tests the pattern documented in the go docs here: https://golang.org/pkg/time/#Timer.Stop -// This pattern is required to safely reset a timer, so should be common. +// This pattern is required to safely reset a timer prior to Go 1.23, so should be common. // This also tests resetting the timer func TestFakeStopDrain(t *testing.T) { start := time.Time{} @@ -303,7 +308,9 @@ func TestFakeStopDrain(t *testing.T) { t.Errorf("timer should have ticked after 1 second, got %v", readTime) } - timer.Reset(time.Second) + if timer.Reset(time.Second) { + t.Errorf("reset should return false as the timer had expired") + } if !tc.HasWaiters() { t.Errorf("expected a waiter to be present, but it is not") } @@ -318,6 +325,79 @@ func TestFakeStopDrain(t *testing.T) { } } +func TestFakeReset(t *testing.T) { + start := time.Now() + t.Run("reset active timer", func(t *testing.T) { + tc := NewFakeClock(start) + timer := tc.NewTimer(time.Second) + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + tc.Step(999 * time.Millisecond) + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + if !timer.Reset(time.Second) { + t.Errorf("reset should return true as the timer is active") + } + tc.Step(time.Millisecond) + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + tc.Step(999 * time.Millisecond) + if tc.HasWaiters() { + t.Errorf("expected existing waiter to be cleaned up, but it is still present") + } + if readTime := assertReadTime(t, timer.C()); !readTime.Equal(start.Add(1999 * time.Millisecond)) { + t.Errorf("timer should have ticked after reset + 1 second, got %v", readTime) + } + }) + + t.Run("reset expired timer", func(t *testing.T) { + tc := NewFakeClock(start) + timer := tc.NewTimer(time.Second) + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + tc.Step(time.Second) + if tc.HasWaiters() { + t.Errorf("expected existing waiter to be cleaned up, but it is still present") + } + if readTime := assertReadTime(t, timer.C()); !readTime.Equal(start.Add(time.Second)) { + t.Errorf("timer should have ticked after 1 second, got %v", readTime) + } + if timer.Reset(time.Second) { + t.Errorf("reset should return false as the timer had expired") + } + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + tc.Step(time.Second) + if readTime := assertReadTime(t, timer.C()); !readTime.Equal(start.Add(2 * time.Second)) { + t.Errorf("timer should have ticked again after reset + 1 more second, got %v", readTime) + } + }) + + t.Run("reset stopped timer", func(t *testing.T) { + tc := NewFakeClock(start) + timer := tc.NewTimer(time.Second) + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + timer.Stop() + if timer.Reset(time.Second) { + t.Errorf("reset should return false as the timer had been stopped") + } + if !tc.HasWaiters() { + t.Errorf("expected a waiter to be present, but it is not") + } + tc.Step(time.Second) + if readTime := assertReadTime(t, timer.C()); !readTime.Equal(start.Add(time.Second)) { + t.Errorf("timer should have ticked after reset + 1 second, got %v", readTime) + } + }) +} + func TestTimerNegative(t *testing.T) { tc := NewFakeClock(time.Now()) timer := tc.NewTimer(-1 * time.Second)