Skip to content

Commit 7e7ee90

Browse files
committed
Fix FakeClock::Reset to always succeed
Current FakeClock::Reset only successfully resets the timer and returns true when the timer has neither fired nore been stopped. This is incorrect behavior as real clocks allow for reseting in both of those situations (as long has the channel has been drained). This is useful in tests that use a fake clock to test wait.BackofManager, as the timer resets after each iteration of Backoff() after the timer has already fired.
1 parent d159ae3 commit 7e7ee90

File tree

2 files changed

+20
-5
lines changed

2 files changed

+20
-5
lines changed

staging/src/k8s.io/apimachinery/pkg/util/clock/clock.go

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -348,7 +348,13 @@ func (f *fakeTimer) Stop() bool {
348348
// Reset conditionally updates the firing time of the timer. If the
349349
// timer has neither fired nor been stopped then this call resets the
350350
// timer to the fake clock's "now" + d and returns true, otherwise
351-
// this call returns false. This is like time.Timer::Reset.
351+
// it creates a new waiter, adds it to the clock, and returns true.
352+
//
353+
// It is not possible to return false, because a fake timer can be reset
354+
// from any state (waiting to fire, already fired, and stopped).
355+
//
356+
// See the GoDoc for time.Timer::Reset for more context on why
357+
// the return value of Reset() is not useful.
352358
func (f *fakeTimer) Reset(d time.Duration) bool {
353359
f.fakeClock.lock.Lock()
354360
defer f.fakeClock.lock.Unlock()
@@ -360,7 +366,15 @@ func (f *fakeTimer) Reset(d time.Duration) bool {
360366
return true
361367
}
362368
}
363-
return false
369+
// No existing waiter, timer has already fired or been reset.
370+
// We should still enable Reset() to succeed by creating a
371+
// new waiter and adding it to the clock's waiters.
372+
newWaiter := fakeClockWaiter{
373+
targetTime: f.fakeClock.time.Add(d),
374+
destChan: seekChan,
375+
}
376+
f.fakeClock.waiters = append(f.fakeClock.waiters, newWaiter)
377+
return true
364378
}
365379

366380
// Ticker defines the Ticker interface

staging/src/k8s.io/apimachinery/pkg/util/clock/clock_test.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -217,8 +217,8 @@ func TestFakeTimer(t *testing.T) {
217217
t.Errorf("unexpected channel read")
218218
default:
219219
}
220-
if twoSec.Reset(time.Second) {
221-
t.Errorf("Expected twoSec.Reset() to return false")
220+
if !twoSec.Reset(time.Second) {
221+
t.Errorf("Expected twoSec.Reset() to return true")
222222
}
223223
if !treSec.Reset(time.Second) {
224224
t.Errorf("Expected treSec.Reset() to return true")
@@ -238,8 +238,9 @@ func TestFakeTimer(t *testing.T) {
238238
case <-oneSec.C():
239239
t.Errorf("unexpected channel read")
240240
case <-twoSec.C():
241-
t.Errorf("unexpected channel read")
241+
// Expected!
242242
default:
243+
t.Errorf("unexpected channel non-read")
243244
}
244245
select {
245246
case <-treSec.C():

0 commit comments

Comments
 (0)