Fix test approach for detecting issues

Makefile

@@ -53,4 +53,4 @@ staticcheck: bin/staticcheck
 
 .PHONY: test
 test:
-	go test -race ./...
+	go test -v -race ./...

go.mod

@@ -3,7 +3,6 @@ module go.uber.org/ratelimit
 go 1.18
 
 require (
-	github.com/benbjohnson/clock v1.3.0
 	github.com/stretchr/testify v1.6.1
 	go.uber.org/atomic v1.7.0
 )

go.sum

@@ -1,5 +1,3 @@
-github.com/benbjohnson/clock v1.3.0 h1:ip6w0uFQkncKQ979AypyG0ER7mqUSBdKLOgAle/AT8A=
-github.com/benbjohnson/clock v1.3.0/go.mod h1:J11/hYXuz8f4ySSvYwY0FKfm+ezbsZBKZxNJlLklBHA=
 github.com/davecgh/go-spew v1.1.0/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=
 github.com/davecgh/go-spew v1.1.1 h1:vj9j/u1bqnvCEfJOwUhtlOARqs3+rkHYY13jYWTU97c=
 github.com/davecgh/go-spew v1.1.1/go.mod h1:J7Y8YcW2NihsgmVo/mv3lAwl/skON4iLHjSsI+c5H38=

ratelimit.go

@@ -22,8 +22,6 @@ package ratelimit // import "go.uber.org/ratelimit"
 
 import (
 	"time"
-
-	"github.com/benbjohnson/clock"
 )
 
 // Note: This file is inspired by:
@@ -45,6 +43,16 @@ type Clock interface {
 	Sleep(time.Duration)
 }
 
+type internalClock struct{}
+
+func (i *internalClock) Now() time.Time {
+	return time.Now()
+}
+
+func (i *internalClock) Sleep(duration time.Duration) {
+	time.Sleep(duration)
+}
+
 // config configures a limiter.
 type config struct {
 	clock Clock
@@ -60,7 +68,7 @@ func New(rate int, opts ...Option) Limiter {
 // buildConfig combines defaults with options.
 func buildConfig(opts []Option) config {
 	c := config{
-		clock: clock.New(),
+		clock: &internalClock{},
 		slack: 10,
 		per:   time.Second,
 	}

ratelimit_mock_time_test.go

@@ -0,0 +1,111 @@
+package ratelimit
+
+/**
+This fake time implementation is a modification of time mocking
+the mechanism used by Ian Lance Taylor in https://github.com/golang/time project
+https://github.com/golang/time/commit/579cf78fd858857c0d766e0d63eb2b0ccf29f436
+
+Modified parts:
+ - advanceUnlocked method uses in-place filtering of timers
+   instead of a full copy on every remove.
+   Since we have 100s of timers in our tests current linear
+   the complexity of this operation is OK
+   If going to have 1000s in the future, we can use heap to store timers.
+ - advanceUnlocked method yields the processor, after every timer triggering,
+   allowing other goroutines to run
+*/
+
+import (
+	"runtime"
+	"sync"
+	"time"
+)
+
+// testTime is a fake time used for testing.
+type testTime struct {
+	mu     sync.Mutex
+	cur    time.Time   // current fake time
+	timers []testTimer // fake timers
+}
+
+// makeTestTime hooks the testTimer into the package.
+func makeTestTime() *testTime {
+	return &testTime{
+		cur: time.Now(),
+	}
+}
+
+// testTimer is a fake timer.
+type testTimer struct {
+	when time.Time
+	ch   chan<- time.Time
+}
+
+// now returns the current fake time.
+func (tt *testTime) now() time.Time {
+	tt.mu.Lock()
+	defer tt.mu.Unlock()
+	return tt.cur
+}
+
+// newTimer creates a fake timer. It returns the channel,
+// a function to stop the timer (which we don't care about),
+// and a function to advance to the next timer.
+func (tt *testTime) newTimer(dur time.Duration) (<-chan time.Time, func() bool, func()) {
+	tt.mu.Lock()
+	defer tt.mu.Unlock()
+	ch := make(chan time.Time, 1)
+	timer := testTimer{
+		when: tt.cur.Add(dur),
+		ch:   ch,
+	}
+	tt.timers = append(tt.timers, timer)
+	return ch, func() bool { return true }, tt.advanceToTimer
+}
+
+// advance advances the fake time.
+func (tt *testTime) advance(dur time.Duration) {
+	tt.mu.Lock()
+	defer tt.mu.Unlock()
+	tt.advanceUnlocked(dur)
+}
+
+// advanceUnlock advances the fake time, assuming it is already locked.
+func (tt *testTime) advanceUnlocked(dur time.Duration) {
+	tt.cur = tt.cur.Add(dur)
+
+	i := 0
+	j := 0
+	for i < len(tt.timers) {
+		if tt.timers[i].when.After(tt.cur) {
+			if i != j {
+				tt.timers[j] = tt.timers[i]
+			}
+			i++
+			j++
+		} else {
+			tt.timers[i].ch <- tt.cur
+			for i := 0; i < 16; i++ {
+				runtime.Gosched()
+			}
+			i++
+		}
+	}
+	tt.timers = tt.timers[0:j]
+}
+
+// advanceToTimer advances the time to the next timer.
+func (tt *testTime) advanceToTimer() {
+	tt.mu.Lock()
+	defer tt.mu.Unlock()
+	if len(tt.timers) == 0 {
+		panic("no timer")
+	}
+	when := tt.timers[0].when
+	for _, timer := range tt.timers[1:] {
+		if timer.when.Before(when) {
+			when = timer.when
+		}
+	}
+	tt.advanceUnlocked(when.Sub(tt.cur))
+}

ratelimit_test.go

@@ -7,10 +7,18 @@ import (
 
 	"go.uber.org/atomic"
 
-	"github.com/benbjohnson/clock"
 	"github.com/stretchr/testify/assert"
 )
 
+func (tt *testTime) Now() time.Time {
+	return tt.now()
+}
+
+func (tt *testTime) Sleep(duration time.Duration) {
+	timer, _, _ := tt.newTimer(duration)
+	<-timer
+}
+
 type testRunner interface {
 	// createLimiter builds a limiter with given options.
 	createLimiter(int, ...Option) Limiter
@@ -27,7 +35,7 @@ type testRunner interface {
 type runnerImpl struct {
 	t *testing.T
 
-	clock       *clock.Mock
+	clock       *testTime
 	constructor func(int, ...Option) Limiter
 	count       atomic.Int32
 	// maxDuration is the time we need to move into the future for a test.
@@ -64,21 +72,28 @@ func runTest(t *testing.T, fn func(testRunner)) {
 
 	for _, tt := range impls {
 		t.Run(tt.name, func(t *testing.T) {
-			// Set a non-default time.Time since some limiters (int64 in particular) use
-			// the default value as "non-initialized" state.
-			clockMock := clock.NewMock()
-			clockMock.Set(time.Now())
 			r := runnerImpl{
 				t:           t,
-				clock:       clockMock,
+				clock:       makeTestTime(),
 				constructor: tt.constructor,
 				doneCh:      make(chan struct{}),
 			}
 			defer close(r.doneCh)
 			defer r.wg.Wait()
 
 			fn(&r)
-			r.clock.Add(r.maxDuration)
+			go func() {
+				move := func() {
+					defer func() {
+						_ = recover()
+						time.Sleep(10 * time.Millisecond)
+					}()
+					r.clock.advanceToTimer()
+				}
+				for {
+					move()
+				}
+			}()
 		})
 	}
 }
@@ -96,6 +111,7 @@ func (r *runnerImpl) startTaking(rls ...Limiter) {
 			for _, rl := range rls {
 				rl.Take()
 			}
+			r.clock.advance(time.Nanosecond)
 			r.count.Inc()
 			select {
 			case <-r.doneCh:
@@ -120,14 +136,14 @@ func (r *runnerImpl) afterFunc(d time.Duration, fn func()) {
 	if d > r.maxDuration {
 		r.maxDuration = d
 	}
-
+	timer, _, _ := r.clock.newTimer(d)
 	r.goWait(func() {
 		select {
 		case <-r.doneCh:
 			return
-		case <-r.clock.After(d):
+		case <-timer:
+			fn()
 		}
-		fn()
 	})
 }
 
@@ -143,6 +159,7 @@ func (r *runnerImpl) goWait(fn func()) {
 }
 
 func TestUnlimited(t *testing.T) {
+	t.Parallel()
 	now := time.Now()
 	rl := NewUnlimited()
 	for i := 0; i < 1000; i++ {
@@ -152,6 +169,7 @@ func TestUnlimited(t *testing.T) {
 }
 
 func TestRateLimiter(t *testing.T) {
+	t.Parallel()
 	runTest(t, func(r testRunner) {
 		rl := r.createLimiter(100, WithoutSlack)
 
@@ -168,6 +186,7 @@ func TestRateLimiter(t *testing.T) {
 }
 
 func TestDelayedRateLimiter(t *testing.T) {
+	t.Parallel()
 	runTest(t, func(r testRunner) {
 		slow := r.createLimiter(10, WithoutSlack)
 		fast := r.createLimiter(100, WithoutSlack)
@@ -186,6 +205,7 @@ func TestDelayedRateLimiter(t *testing.T) {
 }
 
 func TestPer(t *testing.T) {
+	t.Parallel()
 	runTest(t, func(r testRunner) {
 		rl := r.createLimiter(7, WithoutSlack, Per(time.Minute))
 
@@ -199,6 +219,7 @@ func TestPer(t *testing.T) {
 }
 
 func TestSlack(t *testing.T) {
+	t.Parallel()
 	// To simulate slack, we combine two limiters.
 	// - First, we start a single goroutine with both of them,
 	//   during this time the slow limiter will dominate,