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

testing: clarify when TestMain is appropriate and not #44200

Open
matttproud opened this issue Feb 10, 2021 · 6 comments
Open

testing: clarify when TestMain is appropriate and not #44200

matttproud opened this issue Feb 10, 2021 · 6 comments

Comments

@matttproud
Copy link
Contributor

@matttproud matttproud commented Feb 10, 2021

We had an open discussion in a longer design review thread about when user-authored custom TestMain (debuted in release 1.4) entrypoints are necessary or not compared to func init. We were able to offer at best fuzzy criteria for this. It would be beneficial for the newcomer's perspective if some basic litmus tests, advice, or criteria were provided. I have no opinion on where but rather that they are central and findable.

I don't want to bike shed the reasoning, but I will enumerate some of the tradeoffs I heard:

  1. More individuals are familiar with func init semantics, so that speaks in favor of func init for setup. One counterpoint is that a setup routine that requires all package initializers to have been run by the time it is executed could become flaky or error-prone.
  2. Not all tests strictly need what is commonly setup, making the test setup process more expensive than it needs to be (e.g., when a test execution filter -test.run=pattern is used). This also speaks against func init above.
  3. From a code health standpoint, central setup in func init or func TestMain, could imply mutated global state. Unless the tests use whatever is setup in a hermetic way individually, it could accidentally encode test ordering assumptions between test functions. Might be better to encourage tests to locally set up what they need versus not and to enforce cleanup, especially now that we have (*testing.T).Cleanup.

One final ancillary point I heard in an earlier discussion related to user-authored TestMain that a documentation treatment should also cover:

  1. what is the preferred way to handle errors in TestMain? panic, os.Exit(1), log.Fatal?
  2. where should text output be written: stdout or stderr?

Before stating that the answers to the above is obvious, it may be obvious to a seasoned practitioner but not a newcomer.

@seankhliao seankhliao changed the title Clarify When TestMain is Appropriate and Not testing: clarify when TestMain is appropriate and not Feb 10, 2021
@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 10, 2021

The main argument for TestMain, per #8202, is the ability to run something on shut down, after all the tests have been run. A secondary argument is that TestMain, unlike init functions, can examine flags passed to the program. If all you need is something to run at program startup that doesn't need to look at flags, then an init function is fine.

Personally I would say that errors in TestMain should be reported on os.Stderr and that TestMain should call either os.Exit or log.Fatal depending on personal preference.

I'm not sure any of this needs to go into the docs, but a blog post would be fine.

@matttproud
Copy link
Contributor Author

@matttproud matttproud commented Feb 10, 2021

Would an example code body or similar suffice? We're going to take care of this, I expect, in our internal documentation, but I'd hate to leave folks wondering. I've seen a zoo of practices around use over the years, some of which have been wrong. There are nice soft ways that counsel above could be phrased without requiring folks to look at previous issue IDs. What do you think?

@ianlancetaylor
Copy link
Contributor

@ianlancetaylor ianlancetaylor commented Feb 10, 2021

I'm not sure how to write an example for TestMain.

Do you not like the idea of a blog post? It might be similar to the one for contexts (https://blog.golang.org/context), in the sense of a blog post describing the right way to use a library feature.

@matttproud
Copy link
Contributor Author

@matttproud matttproud commented Feb 10, 2021

@bcmills
Copy link
Member

@bcmills bcmills commented Feb 11, 2021

IMO an init function is usually not the right choice no matter what: if tests expect some shared setup, a sync.Once is usually a better fit. If every test necessarily requires some kind of setup (like stubbing out functions or injecting dependencies), that's usually a symptom of a deeper problem with the package (like too much global state, or an API that can test itself but can't be integration-tested by downstream users), and fixing that deeper problem brings the package back to where a sync.Once will suffice.

@gopherbot
Copy link

@gopherbot gopherbot commented Jul 14, 2021

Change https://golang.org/cl/334649 mentions this issue: testing: clarify in docs that TestMain is advanced

gopherbot pushed a commit that referenced this issue Jul 15, 2021
Beginner and intermediate Go users periodically use TestMain when
requirements do not necessitate TestMain (exceeding least-mechanism
design). This commit expands package testing's documentation to convey
that the TestMain feature itself is somewhat low-level and potentially
unsuitable for casual testing where ordinary test functions would
suffice.

Fixes #42161
Updates #44200

Change-Id: I91ba0b048c3d6f79110fe8f0fbb58d896edca366
Reviewed-on: https://go-review.googlesource.com/c/go/+/334649
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
steeve added a commit to znly/go that referenced this issue Aug 19, 2021
Beginner and intermediate Go users periodically use TestMain when
requirements do not necessitate TestMain (exceeding least-mechanism
design). This commit expands package testing's documentation to convey
that the TestMain feature itself is somewhat low-level and potentially
unsuitable for casual testing where ordinary test functions would
suffice.

Fixes golang#42161
Updates golang#44200

Change-Id: I91ba0b048c3d6f79110fe8f0fbb58d896edca366
Reviewed-on: https://go-review.googlesource.com/c/go/+/334649
Trust: Ian Lance Taylor <iant@golang.org>
Run-TryBot: Ian Lance Taylor <iant@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Rob Pike <r@golang.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
5 participants