Skip to content

Commit d8eac8d

Browse files
authored
Merge pull request kubernetes#94317 from kevindelgado/draft/fix-fakeclock-reset
Fix FakeClock::Reset to always succeed
2 parents 27df218 + 7e7ee90 commit d8eac8d

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)