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

proposal: cmd/go: breakout testmain generation functionality into its own tool #30421

Closed
nemith opened this issue Feb 26, 2019 · 8 comments
Closed
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal
Milestone

Comments

@nemith
Copy link
Contributor

nemith commented Feb 26, 2019

When go test is ran there is logic in go/src/cmd/go/internal/load/test.go that will scan a package for it's given tests and generate the code for the entry point a test binary. The go tool then compiles and executes that binary.

External build tools like Bazel (https://bazel.build/) and Buck (https://buckbuild.com/) bypass the go tool and execute the compiler and linkers directly to get control over the output archives, etc to use for distributed caching and to unify building across multiple languages. To deal with tests both Buck[1] and Bazel[2] have done a copy and paste of the testmain generation code with some modifications and embedded them.

Being an internal package, obviously it is not a stable API. Every Go version there can be minor changes to how tests are discovered or how the testmain code generation is code. These have to be painstakingly backported into Bazel and Buck to keep working while also maintaining comparability with older Go versions.

Since the generated testmain code is linked with the testing library found in the stdlib it would make sense that there is a clear way to generate testmain code that is tied to the version of Go toolchain.

I am proposing abstracting out the test discovery and testmain generation code into it's own external tool (e.g go tool testmaingen) that can be called from external tools. The go tool could call either a shared library or the testmaingen tool directly.

This would be similar to how cgo code generation is done.

[1] https://github.com/facebook/buck/blob/master/src/com/facebook/buck/features/go/testmaingen.go.in
[2] https://github.com/bazelbuild/rules_go/blob/master/go/tools/builders/generate_test_main.go

@gopherbot gopherbot added this to the Proposal milestone Feb 26, 2019
@nemith nemith changed the title proposal: breakout testmain generation functionality into it's own tool proposal: cmd/go: breakout testmain generation functionality into it's own tool Feb 27, 2019
@nemith
Copy link
Contributor Author

nemith commented Feb 27, 2019

cc: @bcmills @rsc @ianlancetaylor

@bcmills
Copy link
Contributor

bcmills commented Feb 27, 2019

CC @jayconrod @matloob @ianthehat

@josharian
Copy link
Contributor

Somewhat related: #29430

@jayconrod
Copy link
Contributor

You can kind of already do this. If you run go list -test -f '{{if eq .Name "main"}}{{.GoFiles}}{{end}}' *.go, go list writes the testmain source to the build cache, and you can read it out from there. Kludgy though.

Something that complicates this change is that we generate the testmain source when loading packages, not during execution as you'd expect. The generated testmain is listed in .GoFiles, not .CompiledGoFiles because of this. If we were going to generate the testmain in a subprocess, we should move the code generation into a proper action first so that we can benefit from caching. Actually, we should do this anyway so go list can do less work.

Even if we did this, I'd be very surprised if the generated file could be used as-is. Bazel needs to do some other stuff in main before running tests, like changing directories, sharding, filtering, and coverage post-processing. Every build system is going to need some custom code, and I'm not confident we can factor all that out into a separate file.

@ALTree ALTree changed the title proposal: cmd/go: breakout testmain generation functionality into it's own tool proposal: cmd/go: breakout testmain generation functionality into its own tool Feb 27, 2019
@nemith
Copy link
Contributor Author

nemith commented Feb 27, 2019

That is pretty interesting. I didn't know it was stored in the cache. Digging through the cache seems like it could be more fragile than what we are dealing with here.

I'd be very surprised if the generated file could be used as-is. Bazel needs to do some other stuff in main before running tests, like changing directories, sharding, filtering, and coverage **post-processing.

In Buck we change directories , do test discovery, do sharding through a "test runner" which is just a wrapper around the compiled testmain which seems like a much cleaner solution with better abstractions, but that isn't really the topic here.

Does Bazel/Blaze do this for other languages as well like Python or C++?

Every build system is going to need some custom code, and I'm not confident we can factor all that out into a separate file.

There is very little that Buck does here intentionally as it's already too hard to maintain the testmain generation code with just changes to the testing package

The core problem we have had w/ Buck is that the testing package and the generated code that uses has changed with nearly every major Go release. We have been able to hack around this by adding reflection, implementing an interface, etc Even if some of the testmaingen code was available as an external library that could be imported it would help, but it would require a stable API between it and the testing package which may be less than ideal.

Here are some of the changes that has been required.

In fact when we upgrade to new Go versions this is really the only consistent blocker we have had over the years.

@bcmills bcmills added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Feb 28, 2019
@rsc
Copy link
Contributor

rsc commented Mar 7, 2019

I think @jayconrod's answer here is the right one. There shouldn't be a separate API for generating testmain.go by itself. Even knowing that testmain.go is a thing is too much detailed knowledge about tests. What we've exposed is how to ask 'tell me the exact way the test binary should get built' instead. That's a much more general query.

@bcmills bcmills added the NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. label Mar 28, 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 Mar 28, 2019
@andybons
Copy link
Member

Per discussion with @golang/proposal-review, looks like this is resolved. Please comment if that’s not the case.

@nemith
Copy link
Contributor Author

nemith commented Apr 17, 2019

I guess then the problem is there is no spec on how other build tools should interact with the testing package. It would be beneficial to have a spec on how that interaction would look so that other building tools could just build their own test main generation (or whatever).

The original problem is that the interaction between the the testing package and the tool is very much intertwined leading to a lot of breakage when you step slightly outside of it.

@golang golang locked and limited conversation to collaborators Apr 16, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
FrozenDueToAge NeedsDecision Feedback is required from experts, contributors, and/or the community before a change can be made. Proposal
Projects
None yet
Development

No branches or pull requests

7 participants