Proposal: automatically set a "test" tag during "go test"

[tsuna]

This is a proposal for a change to go test. The goal is to make it easier to enable conditional compilation of test-only code, which is useful to stub out code during testing, by introducing a build tag that is automatically set during go test, similarly to how the race tag is automatically added for builds that use the -race flag.

For example, let's say we have some code that uses the unix.Setns() system call, which is a Linux-specific privileged system call (usually requires root or some elevated privileges like CAP_SYS_ADMIN). One way to make such code testable independently of the platform and privilege level is to do:

var setNs = func(h handle) error {
    return unix.Setns(h.fd(), unix.CLONE_NEWNET)
}

This way unit tests can override the call by changing the layer of indirection that the global variable creates:

func TestNetNs(t *testing.T) {
    // Mock setNs
    oldSetNs := setNs
    setNs = func(fd handle) error {
        return nil
    }
    defer func() {
        setNs = oldSetNs
    }()

However this is a bit tedious to do, it creates an unnecessary level of indirection in the production code and clutters the test code. Another solution that addresses this is to use a build tag instead:

// in setns_linux.go

// +build !test

func setNs(h handle) error {
    return unix.Setns(h.fd(), unix.CLONE_NEWNET)
}

// in setns_test.go
func setNs(h handle) error {
    return nil
}

The downside is that now go test ./... doesn't work anymore, one needs to know to pass -tags test. In a large codebase, people choose to use different build tags to implement this mechanism, which leads to inconsistencies, unless Makefiles (or equivalent) are used and go test is not to be run "manually".

Another place where a built-in test tag could be useful is when factoring out test libraries. Code in foo_test.go files is only visible in the current package and can't be imported from one package to another. Therefore it's common to create a test package to factor out test code re-used across packages. Doing this unfortunately creates room for production code importing test code. By flagging the test code with a build tag, this possibility is significantly reduced. But again, the build tag creates a usability issue.

There are a number of packages that already use a test tag (see for example the GitHub search for language:go "// +build test" filename:_test.go). Some of these packages would benefit from the usability improvement (for example golang.org/x/text does not use a Makefile but unicode/norm/forminfo_test.go assumes the test tag is used), other packages would start behaving differently (for example github.com/netrack/netrack-server would start running both the integration tests and the unit tests when make integration-test is run, instead of just the integration tests as the intent of the Makefile appears to be).

Alternatively the built-in tag could be gotest, which doesn't seem to be used on GitHub.

Looking forward to your feedback on this small proposal. Thanks in advance.

[robpike]

I don't understand this at all. If the file ends "_test.go" then as far as I can tell the meaning is exactly the same as if you have a "test" build tag as proposed here.

[jimmyfrasche]

Without the build tag you can't do // +build !test

[robpike]

But you don't need that. If you put the code that belongs only to the test in a file suffixed _test.go, it is as far as I can reason exactly the same behavior as adding a test build tag. Conversely, files not suffixed _test.go are equivalent to those tagged !test, assuming we're talking about the go test command, which we are.

[robpike]

Note that a "test" build tag in a separate package P imported by the package being tested, T, will have no bearing on the build of P if you run "go test" in T. You would have to first "go install" a version of P with the tag set, which means that the tag is never present when "go test" is running for T.

[jimmyfrasche]

I take it that it's primarily so that rather than

// non test file

package P

var realStubMeOutForTesting = func() {
   //test version, etc
}

func StubMeOutForTesting() {
 realStubMeOutForTesting()
}

//in p_test.go

package P

func init() {
   realStubMeOutForTesting = func() {
      //version for testing
   }
}

you could do

// in non-test build
// +build !test
package P

func StubMeOutForTesting() {
 //etc
}

//in p_test.go
package P

func StubMeOutForTesting() {
  //version for testing
}

the same way you would define different versions of the same function in, for example, // +build windows vs // +build !windows.

Secondarily. the proposal seems to be so that if package T imports P it also gets the // +build test version of StubMeOutForTesting. That seems like a big change to the go test command, which would have to rebuild all the dependencies with the test tag, whether they use it or not, but only run tests for the specified packages.

Having APIs of imported packages change semantics during testing seems like an invitation for fragile and poorly thought out tests.

[minux]

It seems you're suggesting to add per- package build tag called gotest. It will be very confusing to users. As the behavior is subtly different between go test and go build -tags gotest. The former only applies the build tag to the package under test whereas the later applies the build tag to all packages it builds.

[tsuna]

@robpike, @minux: I was naively thinking that the effect of the tag would propagate in the transitive closure of dependencies, but if that's not the case then that kills the idea of also relying on the built-in test tag for test-only libraries. I'm not going to suggest that build tags behave entirely different under go test.

@jimmyfrasche: In the second use case the idea wasn't to have APIs that change semantics during testing, but rather to make some APIs usable only during testing (to help prevent having production code depend on test libraries). But as mentioned above, it doesn't look like what I was suggesting would work anyway.

On Sun, Mar 6, 2016 at 4:49 PM, Rob Pike wrote:

files not suffixed _test.go are equivalent to those tagged !test, assuming we're talking about the go test command, which we are.

They're not equivalent since files not suffixed _test.go get built during go test, while files tagged !test would get ignored during go test, which is why this pattern makes it easy to swap out the implementation of certain functions, and makes for a nice stubbing mechanism.

[robpike]

I see the distinction now. It's very subtle, maybe too subtle. In any case, there are as you point out existing mechanisms to do this. While they are not particularly elegant, they do serve.

Still, the lack of tag propagation may be fatal to this proposal.

[tsuna]

The propagation isn't necessary for the first use case that motivated me to propose this change, namely making it easier to stub out certain functions when testing a given package.

You're right that this can be achieved today, albeit with usability issues. I wasn't trying to suggest something fundamentally new, merely an improvement to the user experience by creating a standard build tag used during testing so that people/tools don't have to know what tags to pass to go test for go test ./... (or go test ./mypkg) to work.

[adg]

To me, it seems like an anti-pattern for your tests to execute different code than the code that runs normally. Making this feasible globally seems like a dangerous path to take. I vote no.

[robpike]

I am too worried about the consequences of packages changing behavior when testing is enabled globally. Thanks for your proposal, but no.