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

cmd/go: when testing a package, also build the non-test variant #29527

Open
dvyukov opened this Issue Jan 3, 2019 · 5 comments

Comments

Projects
None yet
5 participants
@dvyukov
Copy link
Member

dvyukov commented Jan 3, 2019

go version devel +0175064e69 Thu Jan 3 05:07:58 2019 +0000 linux/amd64

If a package can be built only in test mode, go test succeeds while go build/install fails:

// broken.go
package broken
var X typ
// broken_test.go
package broken
import "testing"
type typ int
func TestX(t *testing.T) {
	var _ typ = X
}

If one develops just a package, go test is the main means of ensuring that everything works and lots of CIs just run go test, which means that it's possible to commit code that does not even build without noticing it.

It would be nice if go test would ensure that a package actually builds.

@katiehockman

This comment has been minimized.

Copy link
Contributor

katiehockman commented Jan 3, 2019

@katiehockman katiehockman added this to the Unplanned milestone Jan 3, 2019

@bcmills bcmills added the GoCommand label Jan 8, 2019

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 8, 2019

In-package tests are always at risk of unrealism. For example, what if broken_test.go provided an init function rather than a type declaration? Then the breakage would not be caught until run-time in some other test.

In contrast, a package-external test (package broken_test instead of package broken) would have caught this example right from the start.

Instead of adding a “clean package build” step where it isn't otherwise needed, I'd rather we encourage users to use external tests instead of in-package ones.

See also #29258.

@dvyukov

This comment has been minimized.

Copy link
Member

dvyukov commented Jan 9, 2019

But everybody has tons of tests that use unexported functions. That all will break and the export_test.go trick is quite inconvenient to use and in end it's again in-package _test.go file. For these reasons we always recommended the opposite. What am I missing/what has changed?

@bcmills

This comment has been minimized.

Copy link
Member

bcmills commented Jan 9, 2019

But everybody has tons of tests that use unexported functions.

I said “encourage”, not “require”. If you want to catch subtle variations between the test package and the real one, test against the real one: that's something you can do today, without any extra support from the go command.

we always recommended the opposite. What am I missing/what has changed?

internal packages make it possible to define (and test against) exported APIs without exposing them to the outside world. I don't know about official recommendations, but at least for me that nullifies the strongest argument for package-internal testing.

@bcmills bcmills changed the title cmd/go: test passes when build is broken cmd/go: when testing a package, also build the non-test variant Jan 17, 2019

@rsc

This comment has been minimized.

Copy link
Contributor

rsc commented Jan 17, 2019

This seems like a reasonable thing to try to help users notice.

I strongly disagree about suggesting use of external tests by default. Quite the opposite: I think external tests should be reserved for cases where an internal test would hit an import cycle.

I would like to understand how much it would cost in a typical go test cycle to build the package both with and without its internal tests. My guess is that it would not cost much - most of the cost of go test is in linking the binary, not the incremental build of the package itself.

@rsc rsc modified the milestones: Unplanned, Go1.13 Jan 17, 2019

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