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: add a build tag "test" #21360

Closed
robpike opened this Issue Aug 9, 2017 · 13 comments

Comments

Projects
None yet
10 participants
@robpike
Contributor

robpike commented Aug 9, 2017

To have code be compiled only during tests, it's easy: just put the code in a file suffixed _test.go. However, there is no mechanism for the complement: to have code that is excluded when testing. I propose we set the tag "test" when the build is being run under "go test".

The desire for this came up for me recently when I wanted to test some code's behavior given a value I wanted to be constant. The only way to do this now is to make it a variable that the test modifies, but a variable is not a constant. There are things you can do with a constant, such as declare array types, that are illegal with a variable.

It was necessary to change the semantics of the code to make it testable. That bothers me.

This is a small problem, I admit, but the proposed solution is also small.

@robpike robpike added the Proposal label Aug 9, 2017

@gopherbot gopherbot added this to the Proposal milestone Aug 9, 2017

@cespare

This comment has been minimized.

Show comment
Hide comment
@cespare

cespare Aug 9, 2017

Contributor

Isn't this the same as #14668, which you didn't like?

Contributor

cespare commented Aug 9, 2017

Isn't this the same as #14668, which you didn't like?

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Aug 9, 2017

Member

This also looks like a duplicate of/related to #12120 (/cc @slimsag), which is even older.

I'll paste a point I made there, since it's relevant:

However, it is not possible to exclude some code from being built into tests ran by go test [via a build tag]

I want to share an opinion related to that. I consider that a feature. I think it's great you (or anyone) can't exclude some .go files from tests. That makes test code purely additive and easier to reason about.

Member

dmitshur commented Aug 9, 2017

This also looks like a duplicate of/related to #12120 (/cc @slimsag), which is even older.

I'll paste a point I made there, since it's relevant:

However, it is not possible to exclude some code from being built into tests ran by go test [via a build tag]

I want to share an opinion related to that. I consider that a feature. I think it's great you (or anyone) can't exclude some .go files from tests. That makes test code purely additive and easier to reason about.

@robpike

This comment has been minimized.

Show comment
Hide comment
@robpike

robpike Aug 9, 2017

Contributor

Yes, I didn't like it then, but today I encountered an actual instance where I wanted it. My arguments before still stand, but I did find a new argument in favor.

It's not quite a duplicate of #14668, and that's closed anyway, so I'll leave this for now.

Contributor

robpike commented Aug 9, 2017

Yes, I didn't like it then, but today I encountered an actual instance where I wanted it. My arguments before still stand, but I did find a new argument in favor.

It's not quite a duplicate of #14668, and that's closed anyway, so I'll leave this for now.

@slimsag

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Aug 9, 2017

... sentiment about my proposal being closed as working-as-intended and later reopened two years later aside..

I've had a desire for this functionality multiple times. Most recently, I've had to exclude certain code from a test invocation inside a package-level init, i.e.:

func init() {
	if !isTest {
		...
	}
}

This led me to use an atrocious concoction like this, which relies on initialization order to function correctly:

mypackage.go:

var isTest bool

mypackage_test.go:

var _ = func() bool {
	isTest = true
	return true
}()

If a test build tag existed today, I would use it.

slimsag commented Aug 9, 2017

... sentiment about my proposal being closed as working-as-intended and later reopened two years later aside..

I've had a desire for this functionality multiple times. Most recently, I've had to exclude certain code from a test invocation inside a package-level init, i.e.:

func init() {
	if !isTest {
		...
	}
}

This led me to use an atrocious concoction like this, which relies on initialization order to function correctly:

mypackage.go:

var isTest bool

mypackage_test.go:

var _ = func() bool {
	isTest = true
	return true
}()

If a test build tag existed today, I would use it.

@griesemer

This comment has been minimized.

Show comment
Hide comment
@griesemer

griesemer Aug 9, 2017

Contributor

I haven't made up my mind quite yet, just musing aloud:

Assuming a test built tag, arguably the _test.go file name extension mechanism is not needed anymore; in some sense it would remove a mechanism. I'm not suggesting we do, having test files marked by name seems like a nice thing to have, but a test build tag will lead to overlap where currently there's none. Imagine also the confusion that can be created by excluding a _test.go file from a test by using a !test build tag...

This suggests an alternative, such as a _notest.go filename extension.

(And pushing this a bit further: Instead of build tags, one could encode the file build properties in the file name: imagine x+test.go, x-test.go, or syscall+linux+mipsx.go... In fact, we already do essentially this in file names because otherwise we can't keep the files apart in a meaningful way. Would also allow the build system to determine whether a file is required or not by simply looking at the name rather than the contents. But I digress. Maybe something to think about for Go 2.)

Contributor

griesemer commented Aug 9, 2017

I haven't made up my mind quite yet, just musing aloud:

Assuming a test built tag, arguably the _test.go file name extension mechanism is not needed anymore; in some sense it would remove a mechanism. I'm not suggesting we do, having test files marked by name seems like a nice thing to have, but a test build tag will lead to overlap where currently there's none. Imagine also the confusion that can be created by excluding a _test.go file from a test by using a !test build tag...

This suggests an alternative, such as a _notest.go filename extension.

(And pushing this a bit further: Instead of build tags, one could encode the file build properties in the file name: imagine x+test.go, x-test.go, or syscall+linux+mipsx.go... In fact, we already do essentially this in file names because otherwise we can't keep the files apart in a meaningful way. Would also allow the build system to determine whether a file is required or not by simply looking at the name rather than the contents. But I digress. Maybe something to think about for Go 2.)

@OneOfOne

This comment has been minimized.

Show comment
Hide comment
@OneOfOne

OneOfOne Aug 15, 2017

Contributor

I'll be that guy, what's wrong with go test -tags test?

You are in control of the tests, you can easily add that without changing the testing logic.

Contributor

OneOfOne commented Aug 15, 2017

I'll be that guy, what's wrong with go test -tags test?

You are in control of the tests, you can easily add that without changing the testing logic.

@slimsag

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Aug 15, 2017

@OneOfOne That is something extra I have to explain to every contributor who wishes to run the tests of my project, and it's the kind of command that makes people go Wait, what?!. Not having that build tag would mean compilation would fail, and they'd report issues because of it.

slimsag commented Aug 15, 2017

@OneOfOne That is something extra I have to explain to every contributor who wishes to run the tests of my project, and it's the kind of command that makes people go Wait, what?!. Not having that build tag would mean compilation would fail, and they'd report issues because of it.

@OneOfOne

This comment has been minimized.

Show comment
Hide comment
@OneOfOne

OneOfOne Aug 15, 2017

Contributor

@slimsag while that's a valid point, it's easily worked around, for example just add an extra test file to notify the users:

checktags_test.go

// +build !test

package pkg

func init() {
    log.Println("please rerun go test with -tags test")
}
Contributor

OneOfOne commented Aug 15, 2017

@slimsag while that's a valid point, it's easily worked around, for example just add an extra test file to notify the users:

checktags_test.go

// +build !test

package pkg

func init() {
    log.Println("please rerun go test with -tags test")
}
@slimsag

This comment has been minimized.

Show comment
Hide comment
@slimsag

slimsag Aug 15, 2017

I admit, that is a pretty decent workaround I had not thought of. Thanks @OneOfOne I think I'll start using that for now!

It'd still be nice to have real support for this, though.

slimsag commented Aug 15, 2017

I admit, that is a pretty decent workaround I had not thought of. Thanks @OneOfOne I think I'll start using that for now!

It'd still be nice to have real support for this, though.

@ianlancetaylor

This comment has been minimized.

Show comment
Hide comment
@ianlancetaylor

ianlancetaylor Aug 15, 2017

Contributor

I think it's worth noting that this would require us to potentially rebuild every dependency of the package being tested. This is somewhat problematic when using a build system other than go build, such as Bazel. It essentially implies that every package must be treated as having two possible outputs, one to use for a normal build and one to use for tests. This doesn't prevent us from adopting this plan, but it's worth considering.

Contributor

ianlancetaylor commented Aug 15, 2017

I think it's worth noting that this would require us to potentially rebuild every dependency of the package being tested. This is somewhat problematic when using a build system other than go build, such as Bazel. It essentially implies that every package must be treated as having two possible outputs, one to use for a normal build and one to use for tests. This doesn't prevent us from adopting this plan, but it's worth considering.

@dmitshur

This comment has been minimized.

Show comment
Hide comment
@dmitshur

dmitshur Aug 15, 2017

Member

I'll expand on @ianlancetaylor's point to say that it'd affect not only the build system, but the experience of Go users. If this is adopted, it could mean that a dependency of a dependency of a dependency (any one of them) can in theory change its behavior when you do go test your/package compared to when you do go build your/package.

Imagine running into an issue in production, then trying to reproduce it by writing a test and running it with go test, only to eventually find out it's not possible to reproduce via go test because the issue happens only when test build tag is not specified.

Right now, that is not possible, so a Go user is safe knowing their dependencies will behave identically for go build your/package and go test your/package.

However, I'll note that the original proposal doesn't necessarily mean the test build tag would have to apply to all dependencies. One way of implementing this is to have that build tag only apply to the current package whose test are being executed, but not to its dependencies. However, this would come with its own disadvantage in that it would make the test build tag special, unlike all others. This would be a rule all Go users would have to learn and understand and always keep in mind when writing and reviewing Go code.

Member

dmitshur commented Aug 15, 2017

I'll expand on @ianlancetaylor's point to say that it'd affect not only the build system, but the experience of Go users. If this is adopted, it could mean that a dependency of a dependency of a dependency (any one of them) can in theory change its behavior when you do go test your/package compared to when you do go build your/package.

Imagine running into an issue in production, then trying to reproduce it by writing a test and running it with go test, only to eventually find out it's not possible to reproduce via go test because the issue happens only when test build tag is not specified.

Right now, that is not possible, so a Go user is safe knowing their dependencies will behave identically for go build your/package and go test your/package.

However, I'll note that the original proposal doesn't necessarily mean the test build tag would have to apply to all dependencies. One way of implementing this is to have that build tag only apply to the current package whose test are being executed, but not to its dependencies. However, this would come with its own disadvantage in that it would make the test build tag special, unlike all others. This would be a rule all Go users would have to learn and understand and always keep in mind when writing and reviewing Go code.

@mibk

This comment has been minimized.

Show comment
Hide comment
@mibk

mibk Nov 2, 2017

Contributor

Is there any other real-world example where we could benefit from having this build tag, or is it really just about constants? If it's just about constants, maybe we can find a better solution.

Contributor

mibk commented Nov 2, 2017

Is there any other real-world example where we could benefit from having this build tag, or is it really just about constants? If it's just about constants, maybe we can find a better solution.

@rsc

This comment has been minimized.

Show comment
Hide comment
@rsc

rsc Nov 6, 2017

Contributor

This could be useful, but the build system complications probably outweigh the utility.

Contributor

rsc commented Nov 6, 2017

This could be useful, but the build system complications probably outweigh the utility.

@rsc rsc closed this Nov 6, 2017

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