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

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

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

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

dvyukov opened this issue Jan 3, 2019 · 5 comments
Labels
FeatureRequest GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@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 katiehockman added FeatureRequest NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jan 3, 2019
@katiehockman
Copy link
Contributor

/cc @bcmills @rsc

@katiehockman katiehockman added this to the Unplanned milestone Jan 3, 2019
@bcmills bcmills added the GoCommand cmd/go label Jan 8, 2019
@bcmills
Copy link
Contributor

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
Copy link
Member Author

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
Copy link
Contributor

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 added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 15, 2019
@gopherbot gopherbot removed the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 15, 2019
@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
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
@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jan 17, 2019
@gopherbot gopherbot removed the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Jan 17, 2019
@andybons andybons modified the milestones: Go1.13, Go1.14 Jul 8, 2019
@rsc rsc modified the milestones: Go1.14, Backlog Oct 9, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FeatureRequest GoCommand cmd/go NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

6 participants