Merge pull request #39 from euank/​-t

 Fix 'timer.Reset' for the fake clock

Author: apelisse

clock/testing/fake_clock.go

@@ -45,6 +45,7 @@ type fakeClockWaiter struct {
 	fired         bool
 }
 
+// NewFakeClock constructs a fake clock set to the provided time.
 func NewFakeClock(t time.Time) *FakeClock {
 	return &FakeClock{
 		time: t,
@@ -65,7 +66,7 @@ func (f *FakeClock) Since(ts time.Time) time.Duration {
 	return f.time.Sub(ts)
 }
 
-// Fake version of time.After(d).
+// After is the fake version of time.After(d).
 func (f *FakeClock) After(d time.Duration) <-chan time.Time {
 	f.lock.Lock()
 	defer f.lock.Unlock()
@@ -78,7 +79,7 @@ func (f *FakeClock) After(d time.Duration) <-chan time.Time {
 	return ch
 }
 
-// Fake version of time.NewTimer(d).
+// NewTimer constructs a fake timer, akin to time.NewTimer(d).
 func (f *FakeClock) NewTimer(d time.Duration) clock.Timer {
 	f.lock.Lock()
 	defer f.lock.Unlock()
@@ -95,7 +96,11 @@ func (f *FakeClock) NewTimer(d time.Duration) clock.Timer {
 	return timer
 }
 
+// Tick constructs a fake ticker, akin to time.Tick
 func (f *FakeClock) Tick(d time.Duration) <-chan time.Time {
+	if d <= 0 {
+		return nil
+	}
 	f.lock.Lock()
 	defer f.lock.Unlock()
 	tickTime := f.time.Add(d)
@@ -110,14 +115,15 @@ func (f *FakeClock) Tick(d time.Duration) <-chan time.Time {
 	return ch
 }
 
-// Move clock by Duration, notify anyone that's called After, Tick, or NewTimer
+// Step moves the clock by Duration and notifies anyone that's called After,
+// Tick, or NewTimer.
 func (f *FakeClock) Step(d time.Duration) {
 	f.lock.Lock()
 	defer f.lock.Unlock()
 	f.setTimeLocked(f.time.Add(d))
 }
 
-// Sets the time.
+// SetTime sets the time.
 func (f *FakeClock) SetTime(t time.Time) {
 	f.lock.Lock()
 	defer f.lock.Unlock()
@@ -157,14 +163,15 @@ func (f *FakeClock) setTimeLocked(t time.Time) {
 	f.waiters = newWaiters
 }
 
-// Returns true if After has been called on f but not yet satisfied (so you can
+// HasWaiters returns true if After has been called on f but not yet satisfied (so you can
 // write race-free tests).
 func (f *FakeClock) HasWaiters() bool {
 	f.lock.RLock()
 	defer f.lock.RUnlock()
 	return len(f.waiters) > 0
 }
 
+// Sleep is akin to time.Sleep
 func (f *FakeClock) Sleep(d time.Duration) {
 	f.Step(d)
 }
@@ -186,24 +193,25 @@ func (i *IntervalClock) Since(ts time.Time) time.Duration {
 	return i.Time.Sub(ts)
 }
 
-// Unimplemented, will panic.
+// After is unimplemented, will panic.
 // TODO: make interval clock use FakeClock so this can be implemented.
 func (*IntervalClock) After(d time.Duration) <-chan time.Time {
 	panic("IntervalClock doesn't implement After")
 }
 
-// Unimplemented, will panic.
+// NewTimer is unimplemented, will panic.
 // TODO: make interval clock use FakeClock so this can be implemented.
 func (*IntervalClock) NewTimer(d time.Duration) clock.Timer {
 	panic("IntervalClock doesn't implement NewTimer")
 }
 
-// Unimplemented, will panic.
+// Tick is unimplemented, will panic.
 // TODO: make interval clock use FakeClock so this can be implemented.
 func (*IntervalClock) Tick(d time.Duration) <-chan time.Time {
 	panic("IntervalClock doesn't implement Tick")
 }
 
+// Sleep is unimplemented, will panic.
 func (*IntervalClock) Sleep(d time.Duration) {
 	panic("IntervalClock doesn't implement Sleep")
 }
@@ -250,5 +258,17 @@ func (f *fakeTimer) Reset(d time.Duration) bool {
 	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
+			break
+		}
+	}
+	if !isWaiting {
+		f.fakeClock.waiters = append(f.fakeClock.waiters, &f.waiter)
+	}
+
 	return active
 }

clock/testing/fake_clock_test.go

@@ -194,3 +194,81 @@ func TestFakeStop(t *testing.T) {
 		t.Errorf("expected existing waiter to be cleaned up, but it is still present")
 	}
 }
+
+// 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 also tests resetting the timer
+func TestFakeStopDrain(t *testing.T) {
+	start := time.Time{}
+	tc := NewFakeClock(start)
+	timer := tc.NewTimer(time.Second)
+	tc.Step(1 * time.Second)
+	// Effectively `if !timer.Stop { <-t.C }` but with more asserts
+	if timer.Stop() {
+		t.Errorf("stop should report the timer had triggered")
+	}
+	if readTime := assertReadTime(t, timer.C()); !readTime.Equal(start.Add(1 * time.Second)) {
+		t.Errorf("timer should have ticked after 1 second, got %v", readTime)
+	}
+
+	timer.Reset(time.Second)
+	if !tc.HasWaiters() {
+		t.Errorf("expected a waiter to be present, but it is not")
+	}
+	select {
+	case <-timer.C():
+		t.Fatal("got time early on clock; haven't stepped yet")
+	default:
+	}
+	tc.Step(1 * 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)
+	}
+}
+
+func TestTimerNegative(t *testing.T) {
+	tc := NewFakeClock(time.Now())
+	timer := tc.NewTimer(-1 * time.Second)
+	if !tc.HasWaiters() {
+		t.Errorf("expected a waiter to be present, but it is not")
+	}
+	// force waiters to be called
+	tc.Step(0)
+	tick := assertReadTime(t, timer.C())
+	if tick != tc.Now() {
+		t.Errorf("expected -1s to turn into now: %v != %v", tick, tc.Now())
+	}
+}
+
+func TestTickNegative(t *testing.T) {
+	// The stdlib 'Tick' returns nil for negative and zero values, so our fake
+	// should too.
+	tc := NewFakeClock(time.Now())
+	if tick := tc.Tick(-1 * time.Second); tick != nil {
+		t.Errorf("expected negative tick to be nil: %v", tick)
+	}
+	if tick := tc.Tick(0); tick != nil {
+		t.Errorf("expected negative tick to be nil: %v", tick)
+	}
+}
+
+// assertReadTime asserts that the channel can be read and returns the time it
+// reads from the channel.
+func assertReadTime(t testing.TB, c <-chan time.Time) time.Time {
+	type helper interface {
+		Helper()
+	}
+	if h, ok := t.(helper); ok {
+		h.Helper()
+	}
+	select {
+	case ti, ok := <-c:
+		if !ok {
+			t.Fatalf("expected to read time from channel, but it was closed")
+		}
+		return ti
+	default:
+		t.Fatalf("expected to read time from channel, but couldn't")
+	}
+	panic("unreachable")
+}