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

proposal: testing: allow TestMain to return int #23404

Closed
rogpeppe opened this Issue Jan 10, 2018 · 4 comments

Comments

Projects
None yet
5 participants
@rogpeppe
Contributor

rogpeppe commented Jan 10, 2018

Currently TestMain is expected to call os.Exit itself, but that means it's
not possible to use defer to do package-wide test cleanup there,
because the defer will never be called.

I propose that TestMain be optionally allowed to return an integer,
in which case os.Exit will be called with its result.

Then we can write:

 func TestMain(m *testing.M) int {
     defer testhelper.Cleanup()
     return m.Run()
 }

which is arguably more natural and harder to get wrong than calling os.Exit.

@gopherbot gopherbot added this to the Proposal milestone Jan 10, 2018

@gopherbot gopherbot added the Proposal label Jan 10, 2018

@mvdan

This comment has been minimized.

Show comment
Hide comment
@mvdan

mvdan Jan 10, 2018

Member

Playing the devil's advocate - if one wants to be sure that os.Exit can't interfere with defers, what about something like:

func TestMain(m *testing.M) { os.Exit(testMain(m)) }

func testMain(m *testing.M) int {
     defer testhelper.Cleanup()
     return m.Run()
}

I understand that this is more verbose, but I'm wondering if that offsets adding extra code and documentation to the testing package. I would definitely agree with you if the testing package was designed from the ground up, however. Unless there is a specific reason for os.Exit that we are both missing.

Member

mvdan commented Jan 10, 2018

Playing the devil's advocate - if one wants to be sure that os.Exit can't interfere with defers, what about something like:

func TestMain(m *testing.M) { os.Exit(testMain(m)) }

func testMain(m *testing.M) int {
     defer testhelper.Cleanup()
     return m.Run()
}

I understand that this is more verbose, but I'm wondering if that offsets adding extra code and documentation to the testing package. I would definitely agree with you if the testing package was designed from the ground up, however. Unless there is a specific reason for os.Exit that we are both missing.

@OneOfOne

This comment has been minimized.

Show comment
Hide comment
@OneOfOne

OneOfOne Jan 10, 2018

Contributor

@mvdan os.Exit is required to indicate tests success or failure iirc.

Contributor

OneOfOne commented Jan 10, 2018

@mvdan os.Exit is required to indicate tests success or failure iirc.

@mvdan

This comment has been minimized.

Show comment
Hide comment
@mvdan

mvdan Jan 10, 2018

Member

@OneOfOne what is your point? The integer that you can use in an os.Exit call is the same integer that @rogpeppe suggests to have as a return parameter. See my silly workaround above as an example.

Member

mvdan commented Jan 10, 2018

@OneOfOne what is your point? The integer that you can use in an os.Exit call is the same integer that @rogpeppe suggests to have as a return parameter. See my silly workaround above as an example.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Jan 29, 2018

Contributor

TestMain is already a rare need. It doesn't make much sense to stack another one on top, especially when you can do what @mvdan suggests.

Contributor

rsc commented Jan 29, 2018

TestMain is already a rare need. It doesn't make much sense to stack another one on top, especially when you can do what @mvdan suggests.

@rsc rsc closed this Jan 29, 2018

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