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

Revert "Make maxSleep and maxRetires configurable when building options (#94)" #116

Merged
merged 1 commit into from
Oct 24, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
3 changes: 0 additions & 3 deletions leaks.go
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@ func Find(options ...Option) error {
cur := stack.Current().ID()

opts := buildOpts(options...)
if err := opts.validate(); err != nil {
return err
}
if opts.cleanup != nil {
return errors.New("Cleanup can only be passed to VerifyNone or VerifyTestMain")
}
Expand Down
12 changes: 1 addition & 11 deletions leaks_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ var _ = TestingT(testing.TB(nil))
// testOptions passes a shorter max sleep time, used so tests don't wait
// ~1 second in cases where we expect Find to error out.
func testOptions() Option {
return MaxSleepInterval(time.Millisecond)
return maxSleep(time.Millisecond)
}

func TestFind(t *testing.T) {
Expand All @@ -60,16 +60,6 @@ func TestFind(t *testing.T) {
err := Find(Cleanup(func(int) { assert.Fail(t, "this should not be called") }))
require.Error(t, err, "Should exit with invalid option")
})

t.Run("Find should return error when maxRetries is less than 0", func(t *testing.T) {
err := Find(MaxRetryAttempts(-1))
require.Error(t, err, "maxRetries should be greater than 0")
})

t.Run("Find should return error when maxSleep is less than 0s", func(t *testing.T) {
err := Find(MaxSleepInterval(time.Duration(-1)))
require.Error(t, err, "maxSleep should be greater than 0s")
})
}

func TestFindRetry(t *testing.T) {
Expand Down
44 changes: 7 additions & 37 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
package goleak

import (
"errors"
"strings"
"time"

Expand All @@ -33,14 +32,10 @@ type Option interface {
apply(*opts)
}

const (
// We retry up to default 20 times if we can't find the goroutine that
// we are looking for.
_defaultRetryAttempts = 20
// In between each retry attempt, sleep for up to default 100 microseconds
// to let any running goroutine completes.
_defaultSleepInterval = 100 * time.Microsecond
)
// We retry up to 20 times if we can't find the goroutine that
// we are looking for. In between each attempt, we will sleep for
// a short while to let any running goroutines complete.
const _defaultRetries = 20

type opts struct {
filters []func(stack.Stack) bool
Expand All @@ -58,17 +53,6 @@ func (o *opts) apply(opts *opts) {
opts.cleanup = o.cleanup
}

// validate the options.
func (o *opts) validate() error {
if o.maxRetries < 0 {
return errors.New("maxRetryAttempts should be greater than 0")
}
if o.maxSleep <= 0 {
return errors.New("maxSleepInterval should be greater than 0s")
}
return nil
}

// optionFunc lets us easily write options without a custom type.
type optionFunc func(*opts)

Expand Down Expand Up @@ -123,25 +107,12 @@ func IgnoreCurrent() Option {
})
}

// MaxSleepInterval sets the maximum sleep time in-between each retry attempt.
// The sleep duration grows in an exponential backoff, to a maximum of the value specified here.
// If not configured, default to 100 microseconds.
func MaxSleepInterval(d time.Duration) Option {
func maxSleep(d time.Duration) Option {
return optionFunc(func(opts *opts) {
opts.maxSleep = d
})
}

// MaxRetryAttempts sets the retry upper limit.
// When finding extra goroutines, we'll retry until all goroutines complete
// or end up with the maximum retry attempts.
// If not configured, default to 20 times.
func MaxRetryAttempts(num int) Option {
return optionFunc(func(opts *opts) {
opts.maxRetries = num
})
}

func addFilter(f func(stack.Stack) bool) Option {
return optionFunc(func(opts *opts) {
opts.filters = append(opts.filters, f)
Expand All @@ -150,8 +121,8 @@ func addFilter(f func(stack.Stack) bool) Option {

func buildOpts(options ...Option) *opts {
opts := &opts{
maxRetries: _defaultRetryAttempts,
maxSleep: _defaultSleepInterval,
maxRetries: _defaultRetries,
maxSleep: 100 * time.Millisecond,
}
opts.filters = append(opts.filters,
isTestStack,
Expand All @@ -162,7 +133,6 @@ func buildOpts(options ...Option) *opts {
for _, option := range options {
option.apply(opts)
}

return opts
}

Expand Down
16 changes: 3 additions & 13 deletions options_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -88,20 +88,10 @@ func TestOptionsIgnoreAnyFunction(t *testing.T) {
}
}

func TestBuildOptions(t *testing.T) {
// With default options.
opts := buildOpts()
assert.Equal(t, _defaultSleepInterval, opts.maxSleep, "value of maxSleep not right")
assert.Equal(t, _defaultRetryAttempts, opts.maxRetries, "value of maxRetries not right")

// With customized options.
opts = buildOpts(MaxRetryAttempts(50), MaxSleepInterval(time.Microsecond))
assert.Equal(t, time.Microsecond, opts.maxSleep, "value of maxSleep not right")
assert.Equal(t, 50, opts.maxRetries, "value of maxRetries not right")
}

func TestOptionsRetry(t *testing.T) {
opts := buildOpts(MaxSleepInterval(time.Millisecond), MaxRetryAttempts(50)) // initial attempt + 50 retries = 51
opts := buildOpts()
opts.maxRetries = 50 // initial attempt + 50 retries = 11
opts.maxSleep = time.Millisecond

for i := 0; i < 50; i++ {
assert.True(t, opts.retry(i), "Attempt %v/51 should allow retrying", i)
Expand Down