[Fake clock] Make Stop / Reset return false if Timer stopped

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)
 	}
 

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)