Skip to content

Conversation

@machine424
Copy link

adjust RunOnFailure() doc.

follow up to #129

…ailure() option is used.

adjust RunOnFailure() doc.

follow up to uber-go#129
@CLAassistant
Copy link

CLAassistant commented Oct 10, 2024

CLA assistant check
All committers have signed the CLA.

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.

@machine424
Copy link
Author

cc @sywhang

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.

@machine424
Copy link
Author

cc @sywhang I'd like to get this fix merged so we can make use of the feature.
(when you have some time)

@machine424
Copy link
Author

cc @sywhang @abhinav @r-hang
It'd be great to have a release with this fix, thanks.

@machine424
Copy link
Author

Should I understand that the repo is no longer maintained and that we should fork?
cc @sywhang @abhinav @r-hang

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants