Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a WithSynchronousAfterFunc option #68

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 25 additions & 6 deletions clockwork.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,20 +41,36 @@ func NewRealClock() Clock {
return &realClock{}
}

// FakeClockOption is an option for configuring a FakeClock.
type FakeClockOption func(*fakeClock)

// WithSynchronousAfterFunc is a FakeClockOption which ensures that the callback to AfterFunc is called synchronously
// instead of in a separate goroutine. If true, this option will cause any Advance calls to block until the callback
// from all fired timers has completed.
func WithSynchronousAfterFunc(b bool) FakeClockOption {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On a first-pass, I don't think the argument is needed. The name of the option communicates the intent: if present, the AfterFunc is sync, if absent, AfterFunc is async (the default).

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed, thanks

return func(c *fakeClock) {
c.synchronousAfterFunc = b
}
}

// NewFakeClock returns a FakeClock implementation which can be
// manually advanced through time for testing. The initial time of the
// FakeClock will be the current system time.
//
// Tests that require a deterministic time must use NewFakeClockAt.
func NewFakeClock() FakeClock {
return NewFakeClockAt(time.Now())
func NewFakeClock(opts ...FakeClockOption) FakeClock {
return NewFakeClockAt(time.Now(), opts...)
}

// NewFakeClockAt returns a FakeClock initialised at the given time.Time.
func NewFakeClockAt(t time.Time) FakeClock {
return &fakeClock{
func NewFakeClockAt(t time.Time, opts ...FakeClockOption) FakeClock {
clock := &fakeClock{
time: t,
}
for _, opt := range opts {
opt(clock)
}
return clock
}

type realClock struct{}
Expand Down Expand Up @@ -94,6 +110,8 @@ type fakeClock struct {
waiters []expirer
blockers []*blocker
time time.Time
// synchronousAfterFunc determines whether AfterFunc calls should be executed synchronously or in a goroutine.
synchronousAfterFunc bool
}

// blocker is a caller of BlockUntil.
Expand Down Expand Up @@ -181,7 +199,8 @@ func (fc *fakeClock) newTimer(d time.Duration, afterfunc func()) *fakeTimer {
},
stop: func() bool { return fc.stop(ft) },

afterFunc: afterfunc,
afterFunc: afterfunc,
synchronousAfterFunc: fc.synchronousAfterFunc,
}
fc.set(ft, d)
return ft
Expand Down Expand Up @@ -307,7 +326,7 @@ func (fc *fakeClock) setExpirer(e expirer, d time.Duration) {
return fc.waiters[i].expiry().Before(fc.waiters[j].expiry())
})

// Notify blockers of our new waiter.
// Notify blockers of our new waiter.
var blocked []*blocker
count := len(fc.waiters)
for _, b := range fc.blockers {
Expand Down
32 changes: 32 additions & 0 deletions clockwork_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ package clockwork
import (
"context"
"errors"
"fmt"
"reflect"
"sync/atomic"
"testing"
"time"
)
Expand Down Expand Up @@ -199,3 +201,33 @@ func TestFakeClockRace(t *testing.T) {
go func() { fc.NewTimer(d) }()
go func() { fc.Sleep(d) }()
}

func TestFakeClockSynchronousAfterFunc(t *testing.T) {
t.Parallel()
fc := NewFakeClock(WithSynchronousAfterFunc(true))
var called atomic.Bool
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's avoid atomic unless as a last resort. Here you can pretty readily use a channel:

var called atomic.Bool - > called := make(chan bool)

called.Store(true) -> close(called)

if called.Load() { t.Falal(...) } ->

select {
case <-called:
  t.Falal(...)
default:
}

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I actually realized that since this is called synchronously, there's no need for further synchronization, so I just used a bool. I ran the same test with the -race flag on, and I verified that the test passed; and then I also verified that the race detector test fails without the WithSynchronousAfterFunc option.

fc.AfterFunc(time.Second, func() {
called.Store(true)
})
if called.Load() {
t.Fatal("AfterFunc called before Advance")
}
fc.Advance(time.Second)
if !called.Load() {
t.Fatal("AfterFunc not called after Advance")
}
}

func ExampleWithSynchronousAfterFunc() {
fc := NewFakeClock(WithSynchronousAfterFunc(true))
fc.AfterFunc(time.Second, func() {
fmt.Println("AfterFunc called")
})
fmt.Println("Calling Advance")
fc.Advance(time.Second)
fmt.Println("Advance returned")
// Output:
// Calling Advance
// AfterFunc called
// Advance returned
}
26 changes: 26 additions & 0 deletions example_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,3 +46,29 @@ func TestMyFunc(t *testing.T) {
// Assert the final state.
assertState(t, i, 1)
}

// myFunc2 is an example of a time-dependent function which uses AfterFunc.
func myFunc2(clock Clock, i *int) {
clock.AfterFunc(3*time.Second, func() {
*i += 1
})
}

func TestMyFunc2(t *testing.T) {
var i int

// Use WithSynchronousAfterFunc to ensure that the AfterFunc callback is called by the time Advance returns.
c := NewFakeClock(WithSynchronousAfterFunc(true))

// Call myFunc2, which will schedule a callback to increment i.
myFunc2(c, &i)

// Assert the initial state.
assertState(t, i, 0)

// Now advance the clock forward in time to trigger the callback.
c.Advance(3 * time.Second)

// Assert the final state.
assertState(t, i, 1)
}
8 changes: 7 additions & 1 deletion timer.go
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@ type fakeTimer struct {
// If present when the timer fires, the timer calls afterFunc in its own
// goroutine rather than sending the time on Chan().
afterFunc func()
// synchronousAfterFunc indicates that the afterFunc should be called in the same goroutine as the timer.
synchronousAfterFunc bool
}

func (f *fakeTimer) Reset(d time.Duration) bool {
Expand All @@ -40,7 +42,11 @@ func (f *fakeTimer) Stop() bool {

func (f *fakeTimer) expire(now time.Time) *time.Duration {
if f.afterFunc != nil {
go f.afterFunc()
if f.synchronousAfterFunc {
f.afterFunc()
} else {
go f.afterFunc()
}
return nil
}

Expand Down