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 Cleanup option #78

Merged
merged 8 commits into from Sep 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 9 additions & 1 deletion leaks.go
Expand Up @@ -55,6 +55,9 @@ func Find(options ...Option) error {
cur := stack.Current().ID()

opts := buildOpts(options...)
if opts.teardown != nil {
return fmt.Errorf("Cleanup can only be passed to VerifyNone or VerifyTestMain")
sywhang marked this conversation as resolved.
Show resolved Hide resolved
}
var stacks []stack.Stack
retry := true
for i := 0; retry; i++ {
Expand All @@ -79,12 +82,17 @@ type testHelper interface {
//
// defer VerifyNone(t)
func VerifyNone(t TestingT, options ...Option) {
opts := buildOpts(options...)
if h, ok := t.(testHelper); ok {
// Mark this function as a test helper, if available.
h.Helper()
}

if err := Find(options...); err != nil {
if err := Find(opts); err != nil {
t.Error(err)
}

if opts.teardown != nil {
opts.teardown(0)
}
}
11 changes: 11 additions & 0 deletions leaks_test.go
Expand Up @@ -51,6 +51,10 @@ func TestFind(t *testing.T) {
// Once we unblock the goroutine, we shouldn't have leaks.
bg.unblock()
require.NoError(t, Find(), "Should find no leaks by default")

// Find can't take in Cleanup option
err = Find(Cleanup(func(int) { assert.Fail(t, "this should not be called") }))
require.Error(t, err, "Should exit with invalid option")
sywhang marked this conversation as resolved.
Show resolved Hide resolved
}

func TestFindRetry(t *testing.T) {
Expand Down Expand Up @@ -82,6 +86,13 @@ func TestVerifyNone(t *testing.T) {
VerifyNone(ft, testOptions())
require.NotEmpty(t, ft.errors, "Expect errors from VerifyNone on leaked goroutine")
bg.unblock()

cleanupCalled := false
VerifyNone(ft, Cleanup(func(c int) {
assert.Equal(t, 0, c)
cleanupCalled = true
}))
require.True(t, cleanupCalled, "expect cleanup registered callback to be called")
sywhang marked this conversation as resolved.
Show resolved Hide resolved
}

func TestIgnoreCurrent(t *testing.T) {
Expand Down
34 changes: 28 additions & 6 deletions options.go
Expand Up @@ -41,6 +41,16 @@ type opts struct {
filters []func(stack.Stack) bool
maxRetries int
maxSleep time.Duration
teardown func(int)
}

// implement apply so that opts struct itself can be used as
// an Option.
func (o opts) apply(opts *opts) {
opts.filters = o.filters
opts.maxRetries = o.maxRetries
opts.maxSleep = o.maxSleep
opts.teardown = o.teardown
}

// optionFunc lets us easily write options without a custom type.
Expand All @@ -57,6 +67,18 @@ func IgnoreTopFunction(f string) Option {
})
}

// Cleanup sets up a cleanup function that will be executed at the
// end of the leak.
Copy link
Collaborator

Choose a reason for hiding this comment

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

end of the leak check?

// When passed to [VerifyTestMain], the exit code passed to cleanupFunc
// will be set to the exit code of TestMain.
// When passed to [VerifyNone], the exit code will be set to 0.
// This cannot be passed to [Find].
func Cleanup(cleanupFunc func(exitCode int)) Option {
return optionFunc(func(opts *opts) {
opts.teardown = cleanupFunc
sywhang marked this conversation as resolved.
Show resolved Hide resolved
})
}

// IgnoreCurrent records all current goroutines when the option is created, and ignores
// them in any future Find/Verify calls.
func IgnoreCurrent() Option {
Expand Down Expand Up @@ -98,23 +120,23 @@ func buildOpts(options ...Option) *opts {
return opts
}

func (vo *opts) filter(s stack.Stack) bool {
for _, filter := range vo.filters {
func (o *opts) filter(s stack.Stack) bool {
for _, filter := range o.filters {
if filter(s) {
return true
}
}
return false
}

func (vo *opts) retry(i int) bool {
if i >= vo.maxRetries {
func (o *opts) retry(i int) bool {
if i >= o.maxRetries {
return false
}

d := time.Duration(int(time.Microsecond) << uint(i))
if d > vo.maxSleep {
d = vo.maxSleep
if d > o.maxSleep {
d = o.maxSleep
}
time.Sleep(d)
return true
Expand Down
10 changes: 7 additions & 3 deletions testmain.go
Expand Up @@ -51,13 +51,17 @@ type TestingM interface {
// for any goroutine leaks and fail the tests if any leaks were found.
func VerifyTestMain(m TestingM, options ...Option) {
exitCode := m.Run()
opts := buildOpts(options...)

if exitCode == 0 {
if err := Find(options...); err != nil {
if err := Find(opts); err != nil {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Won't this end up passing a opts to Find that has teardown set to non-nil? Same for VerifyNone.

fmt.Fprintf(_osStderr, "goleak: Errors on successful test run: %v\n", err)
exitCode = 1
}
}

_osExit(exitCode)
if opts.teardown != nil {
opts.teardown(exitCode)
} else {
_osExit(exitCode)
}
}
9 changes: 9 additions & 0 deletions testmain_test.go
Expand Up @@ -75,4 +75,13 @@ func TestVerifyTestMain(t *testing.T) {
VerifyTestMain(dummyTestMain(0))
assert.Equal(t, 0, <-exitCode, "Expect no errors without leaks")
assert.NotContains(t, <-stderr, "goleak: Errors", "No errors on successful run without leaks")

cleanupCalled := false
cleanupExitcode := 0
VerifyTestMain(dummyTestMain(3), Cleanup(func(ec int) {
cleanupCalled = true
cleanupExitcode = ec
}))
assert.True(t, cleanupCalled)
assert.Equal(t, 3, cleanupExitcode)
}