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

fix: ensure the appropriate error message is produced when the RunOnFailure() option is used. #132

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
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 @@ -59,9 +59,6 @@ func Find(options ...Option) error {
if opts.cleanup != nil {
return errors.New("Cleanup can only be passed to VerifyNone or VerifyTestMain")
}
if opts.runOnFailure {
Copy link
Author

Choose a reason for hiding this comment

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

This will also block its use in VerifyTestMain
I adjusted the doc to mention this has no effect on VerifyNone

keeping/adding a check in the code would require more effort/changes.

return errors.New("RunOnFailure can only be passed to VerifyTestMain")
}
var stacks []stack.Stack
retry := true
for i := 0; retry; i++ {
Expand Down
5 changes: 3 additions & 2 deletions options.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,9 @@ func IgnoreCurrent() Option {
})
}

// RunOnFailure makes goleak look for leaking goroutines upon test failures.
// By default goleak only looks for leaking goroutines when tests succeed.
// RunOnFailure makes [VerifyTestMain] look for leaking goroutines upon test failures as well.
// By default [VerifyTestMain] only looks for leaking goroutines when tests succeed.
// This has no effect on [VerifyNone], which always checks for leaking goroutines.
func RunOnFailure() Option {
return optionFunc(func(opts *opts) {
opts.runOnFailure = true
Expand Down
2 changes: 1 addition & 1 deletion testmain.go
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ func VerifyTestMain(m TestingM, options ...Option) {
errorMsg string
)

if !opts.runOnFailure && exitCode == 0 {
if exitCode == 0 {
errorMsg = "goleak: Errors on successful test run:%v\n"
run = true
} else if opts.runOnFailure {
Expand Down
8 changes: 6 additions & 2 deletions testmain_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,15 @@ func TestVerifyTestMain(t *testing.T) {

VerifyTestMain(dummyTestMain(7), RunOnFailure())
assert.Equal(t, 7, <-exitCode, "Exit code should not be modified")
assert.Contains(t, <-stderr, "goleak: Errors", "Find leaks on unsuccessful runs with RunOnFailure specified")
assert.Contains(t, <-stderr, "goleak: Errors on unsuccessful test run", "Find leaks on unsuccessful runs with RunOnFailure specified")
Copy link
Author

Choose a reason for hiding this comment

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

goleak: Errors wasn't enough as it was hiding RunOnFailure can only be passed to VerifyTestMain in this case.


VerifyTestMain(dummyTestMain(0))
assert.Equal(t, 1, <-exitCode, "Expect error due to leaks on successful runs")
assert.Contains(t, <-stderr, "goleak: Errors", "Find leaks on successful runs")
assert.Contains(t, <-stderr, "goleak: Errors on successful test run", "Find leaks on successful runs")

VerifyTestMain(dummyTestMain(0), RunOnFailure())
assert.Equal(t, 1, <-exitCode, "Expect error due to leaks on successful runs")
assert.Contains(t, <-stderr, "goleak: Errors on successful test run", "Find leaks on successful runs")

blocked.unblock()
VerifyTestMain(dummyTestMain(0))
Expand Down