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: delay registration of flags until MainStart #21051

Open
rsc opened this Issue Jul 17, 2017 · 7 comments

Comments

Projects
None yet
6 participants
@rsc
Contributor

rsc commented Jul 17, 2017

Mitchell Hashimoto pointed out at Gophercon that if you write
packages that provides extra test hooks mentioning *testing.T,
the import of "testing" forces the testing flags into any resulting
binary, even non-test binaries. At Hashicorp they define a separate
package that defines a copied testing.T interface only to avoid
these flag definitions.

I sent CL 49251 to delay the registration of the flags so that
those packages can import "testing" directly instead of a fake
package that must be kept in sync.

For discussion for Go 1.10.

@rsc rsc added this to the Go1.10 milestone Jul 17, 2017

@bradfitz

This comment has been minimized.

Member

bradfitz commented Jul 17, 2017

This happened so enough times in Camlistore that we have a test for it.

I suppose this CL would "break" this code:

https://github.com/camlistore/camlistore/blob/77ed42e/pkg/buildinfo/buildinfo.go#L37-L40

But it wouldn't really matter, since it was only detecting that to avoid the testing flags showing up.

@rsc

This comment has been minimized.

Contributor

rsc commented Oct 23, 2017

No one has objected, so I suggest we at least try this.

@rsc rsc added the NeedsFix label Oct 23, 2017

@rasky

This comment has been minimized.

Member

rasky commented Oct 23, 2017

#19533 is a tracking bug for the generic issue of dropping init-time work that is unused. I understand that we're pretty far though.

@rsc

This comment has been minimized.

Contributor

rsc commented Nov 29, 2017

CL 49251 is approved but needs to be rebased.

@odeke-em

This comment has been minimized.

Member

odeke-em commented Dec 6, 2017

For the link happy folks, @rsc is referring to https://golang.org/cl/49251

@gopherbot

This comment has been minimized.

gopherbot commented Dec 14, 2017

Change https://golang.org/cl/49251 mentions this issue: testing: avoid registering flags except in test binaries

@rsc rsc modified the milestones: Go1.10, Go1.11 Dec 14, 2017

@rsc

This comment has been minimized.

Contributor

rsc commented Dec 14, 2017

Unexpected problem in CL; need to introduce some new API like testing.Init to force registration of flags. Leaving for Go 1.11.

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