Skip to content

Commit

Permalink
Revert "Make maxSleep and maxRetires configurable when building optio…
Browse files Browse the repository at this point in the history
…ns (#94)" (#116)

This reverts commit 751da59.

Per discussion in #93, we don't want to release this change as-is.
  • Loading branch information
sywhang committed Oct 24, 2023
1 parent 7b4998f commit 5643445
Show file tree
Hide file tree
Showing 4 changed files with 11 additions and 64 deletions.
3 changes: 0 additions & 3 deletions leaks.go
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
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
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
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

0 comments on commit 5643445

Please sign in to comment.