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 4 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
14 changes: 13 additions & 1 deletion leaks.go
Expand Up @@ -21,6 +21,7 @@
package goleak

import (
"errors"
"fmt"

"go.uber.org/goleak/internal/stack"
Expand Down Expand Up @@ -55,6 +56,9 @@ func Find(options ...Option) error {
cur := stack.Current().ID()

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

if err := Find(options...); err != nil {
// Find does not appreciate teardowns
opts.teardown = nil
if err := Find(opts); err != nil {
t.Error(err)
}

if teardown != nil {
teardown(0)
}
}
55 changes: 38 additions & 17 deletions leaks_test.go
Expand Up @@ -40,17 +40,26 @@ func testOptions() Option {
}

func TestFind(t *testing.T) {
require.NoError(t, Find(), "Should find no leaks by default")
t.Run("Should find no leaks by default", func(t *testing.T) {
require.NoError(t, Find())
})

bg := startBlockedG()
err := Find(testOptions())
require.Error(t, err, "Should find leaks with leaked goroutine")
assert.Contains(t, err.Error(), "blockedG")
assert.Contains(t, err.Error(), "created by go.uber.org/goleak.startBlockedG")

// Once we unblock the goroutine, we shouldn't have leaks.
bg.unblock()
require.NoError(t, Find(), "Should find no leaks by default")
t.Run("Find leaks with leaked goroutine", func(t *testing.T) {
bg := startBlockedG()
err := Find(testOptions())
require.Error(t, err, "Should find leaks with leaked goroutine")
assert.Contains(t, err.Error(), "blockedG")
assert.Contains(t, err.Error(), "created by go.uber.org/goleak.startBlockedG")
Comment on lines +51 to +52
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
assert.Contains(t, err.Error(), "blockedG")
assert.Contains(t, err.Error(), "created by go.uber.org/goleak.startBlockedG")
assert.ErrorContains(t, err, "blockedG")
assert.ErrorContains(t, err, "created by go.uber.org/goleak.startBlockedG")

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ack. This doesn't look doable rn because we're using 1.7.0 testify, and ErrorContains was introduced in 1.7.1. I'll follow up with another PR to address this.


// Once we unblock the goroutine, we shouldn't have leaks.
bg.unblock()
require.NoError(t, Find(), "Should find no leaks by default")
})

t.Run("Find can't take in Cleanup option", func(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")
})
}

func TestFindRetry(t *testing.T) {
Expand All @@ -74,14 +83,26 @@ func (ft *fakeT) Error(args ...interface{}) {
}

func TestVerifyNone(t *testing.T) {
ft := &fakeT{}
VerifyNone(ft)
require.Empty(t, ft.errors, "Expect no errors from VerifyNone")
t.Run("VerifyNone finds leaks", func(t *testing.T) {
ft := &fakeT{}
VerifyNone(ft)
require.Empty(t, ft.errors, "Expect no errors from VerifyNone")

bg := startBlockedG()
VerifyNone(ft, testOptions())
require.NotEmpty(t, ft.errors, "Expect errors from VerifyNone on leaked goroutine")
bg.unblock()
})

bg := startBlockedG()
VerifyNone(ft, testOptions())
require.NotEmpty(t, ft.errors, "Expect errors from VerifyNone on leaked goroutine")
bg.unblock()
t.Run("cleanup registered callback should be called", func(t *testing.T) {
ft := &fakeT{}
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")
})
}

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 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
13 changes: 10 additions & 3 deletions testmain.go
Expand Up @@ -51,13 +51,20 @@ 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...)
teardown := opts.teardown

if exitCode == 0 {
if err := Find(options...); err != nil {
// Find does not appreciate teardowns
opts.teardown = nil
if err := Find(opts); err != nil {
fmt.Fprintf(_osStderr, "goleak: Errors on successful test run: %v\n", err)
exitCode = 1
}
}

_osExit(exitCode)
if teardown != nil {
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)
}