cmd/cover: should support -coverprofile unifying tests from multiple packages
wadey opened this issue · 57 comments
What steps will reproduce the problem? If possible, include a link to a program on play.golang.org. 1. run `go test -coverprofile=coverage.out ./...` What is the expected output? coverage.out contains coverage all of the specified packages What do you see instead? `cannot use test profile flag with multiple packages` Which compiler are you using (5g, 6g, 8g, gccgo)? 6g Which operating system are you using? OS X 10.9 Which version are you using? (run 'go version') go version go1.2 darwin/amd64 Please provide any additional information below. If I manually construct a coverage.out file from multiple packages, it seems to work fine: 1. go test -coverprofile a.part ./a 2. go test -coverprofile b.part ./b 3. echo "mode: set" >coverage.out 4. grep -h -v "mode: set" *.part >>coverage.out 5. go tool cover -html=coverage.out Why can't `go test -coverprofile` do this automatically?
It is not an urgent issue. The workaround is simple, please check the shell script: https://gist.github.com/hailiang/0f22736320abe6be71ce
CL https://golang.org/cl/159910044 mentions this issue.
Can this be included in 1.5. I don't see any technical reason for it to not be. Thank you.
I am interested in getting total test coverage across a DAG of packages. All the solutions online do naive merges of cover profiles via concatenation (including comment 6 here), but this does not take into account lines covered across package boundaries. +1 for anyone who can work on this.
Edited My original phrasing was bad.
I wouldn't say there's "no interest" but rather nobody with time to work on it.
With regard to the solution I can see two approaches:
- Instrumenting every package in
go list ./...for each run and then merging cover profiles. - Generating a single instrumented test binary with all tests combined. This probably means go test -c ./... produces one combined test binary.
I personally want go test -c to support producing a single binary. In particular, I want go test -c std to work, so that I can easily benchmark compiler changes. Right now I have to play games to generate and manage a slew of test binaries.
However, this would require a bit of thought. Right now, all tests and benchmarks are implicitly namespaced by the package. Take for example go test -c encoding/.... Several packages have a TestEncode function. If you did ./encoding.test -test.run=Encode, should it run all of them? How do you specify just one of them? There's a similar question about output. Run e.g. go test -v -run=^TestEncode$ encoding/... and observe that the go tool is responsible for providing the output that differentiates the different packages. Same thing for benchmarks.
This is not to say it can't/shouldn't be done, just that there are some careful decisions to be made.
@josharian Tests also can define their own command-line flag parsing. I don't see how you could make that work right.
I would think that you would deal with that the same way you do now when combining packages that define their own command-line flag parsing in a non-test program—by manually fixing those collisions when they occur.
@josharian wrt name spacing, The full package path could be part of the test name, or there could be a seperate flag -runpkg that works in conjunction with -run.
Another challenge (just writing them down here as I think of them) -- tests are supposed to be executed from the directory in which they are written. This lets them find fixtures easily. However, if multiple tests are bundled into a single executable, there is no single correct working directory. Again, not a showstopper, just something to be thought through.
@josharian Please don't rely on that. go test -c github.com/jdoe/foo is a thing, and so is cross-compiling & rsyncing the test binary.
@josharian Or maybe I should say, don't rely on that in general. It's fine for a lone project to decide it has more onerous requirements; that doesn't make those requirements of go test itself.
@tv42 that is how the go tool works right now, there are many tests in the standard library that rely on that (try e.g. go test -c net && mv net.test /tmp && /tmp/net.test), and as far as I know, it is the recommended way to make fixtures available for tests. For the record, I have cross-compiled and scp'd tests more times than I would like. The point is just that this is useful behavior that was intentionally designed; I would not give it up lightly.
I just tried
go test --coverprofile foo --coverpkg ./...
go tool cover -html foo
and pulled up a web page with coverage data from all the packages in the tree. However, it was created only by running tests in the current directory, not the dependent ones. That is, it contains coverage information for all the packages that were used in running the tests for this package.
So, it's not clear but I believe the request that one should be able to do
go test --coverprofile foo --coverpkg ./... ./...
(note the extra ./...). The issue title is therefore incorrect, and I will address that.
As for fixing the issue, it seems to me that an external tool that merges a set of profiles is easy to do. I don't see much need for doing. this within go test itself.
You should try https://github.com/pierrre/gotestcover
I've written a tool to properly merge coverprofiles that doesn't just blindly cat them together. It uses golang/x/tools/cover to parse the files and outputs a merged version back to stdout:
https://github.com/wadey/gocovmerge
It's still a work in progress, but it is working great for our repos.
We combine with a generic Makefile that runs go test -coverprofile … -coverpkg (all same repo dependencies) … for each folder in a repo. If anyone is interested in that Makefile I can clean it up and post it as well.
I create a library that may help, https://github.com/bluesuncorp/overalls
all it does is recursively go through each directory ( aka each package ), run go test and produce coverprofiles, then merges all profiles into a single one at the root of the project directory called overalls.coverprofile
then you can use a tool like https://github.com/mattn/goveralls to send it to coveralls.io
@wadey @joeybloggs Did you used my tool before creating your own tool?
https://github.com/pierrre/gotestcover
No @pierrre, I did not even know it existed it, looks good; I was looking way before you commented here and couldn't find anything that was an all in solution, just got around to creating my own
I will try yours as well, I think that I may encounter one of the same issues I did while still using a bash script for this. When using Semaphore CI they symlink your project directory under the GOPATH to somewhere outside the GOPATH, so when I ran tests everything went fine except the paths in the coverprofile were for the symlinked directory and not the proper GOPATH, because of that when submitting to coveralls.io the repo and coverprofile paths did not match up resulting in 0% test coverage being reported.
Yours may work fine, but might want to check that out; that's why in my package I ask for the package for relative to the GOPATH/SRC/ directory so I can navigate there abstracting that issue away from the end user.
Workaround on this issue:
https://github.com/h12w/gosweep
@h12w that does a tad more than running -coverprofile tests for multiple packages :) these days I also use https://github.com/wadey/gocovmerge although it would be best if this was available directly from the go tool and possibly run in parallel too.
Just in case this helps anyone, per @robpike's comment above, I wanted to do something like this:
go test --coverprofile foo --coverpkg ./... ./...
I was able to do so by combining 'go list' and @wadey's tool:
# For each package with test files, run with full coverage (including other packages)
go list -f '{{if gt (len .TestGoFiles) 0}}"go test -covermode count -coverprofile {{.Name}}.coverprofile -coverpkg ./... {{.ImportPath}}"{{end}}' ./... | xargs -I {} bash -c {}
# Merge the generated cover profiles into a single file
gocovmerge `ls *.coverprofile` > coverage.txt
# run 'go tool cover -html cover.txt' or upload to travis, codecov, coveralls, etc.
Note that if you have multiple leaf packages with the same name (I have listener/tcp and cmd/send/tcp), it will be necessary to add additional qualifiers to the generated .coverprofile file name to prevent one package from overwriting the other. For example, I used -coverprofile {{.Name}}_{{len .Imports}}_{{len .Deps}}.coverprofile, which generates something like tcp_6_64.coverprofile and tcp_15_70.coverprofile.
The problem here is not about merging the coverage profiles from the tests for each subpackage.
The problem is that there is no way to produce a coverage profile that spans multiples subpackages from the tests of a single package.
i.e. in a package like:
project
project/subpkg1
project/subpkg2
project imports subpkg1 and subpkg2 and the tests from project are end-to-end. So project tests are also covering the subpackages.
I think the output of -cover with -coverpkg provides the right figure for this sort of coverage. The problem is that it cannot output a profile along with it too.
Merging independent coverage profiles is not addressing the real issue (but it's nice to have).
The solutions from @pierrre @wadey and @joeybloggs merge multiple independent coverage profiles into one but only @mmindenhall's solution produces coverage of files that cross package boundaries. Works great!
None of the solutions work well if the project uses the vendor directory, even using the workround
$(go list ./... | grep -v /vendor/) insted of ./...
When I wrote my comment above, my project was using wgo (wrapper around go cmd) for dependencies. We have since switched to vendoring with govendor. After a lot of trial and error, I got this to work. The key is to replace the ./... wildcards with a list of your project packages (excluding vendored). One occurrence must be replaced with a list, and the other with a comma-separated list.
Here's how I did it using govendor:
# create list of project packages, excluding vendored (with govendor)
export PKGS=$(govendor list -no-status +local)
# make comma-separated
export PKGS_DELIM=$(echo "$PKGS" | paste -sd "," -)
# run with full coverage (including other packages) with govendor
go list -f '{{if or (len .TestGoFiles) (len .XTestGoFiles)}}govendor test -covermode count -coverprofile {{.Name}}_{{len .Imports}}_{{len .Deps}}.coverprofile -coverpkg $PKGS_DELIM {{.ImportPath}}{{end}}' $PKGS | xargs -I {} bash -c {}
If not using govendor, this should also work:
# as in @rodrigocorsi2 comment above (using full path to grep due to 'grep -n' alias)
export PKGS=$(go list ./... | /usr/bin/grep -v /vendor/)
# make comma-separated
export PKGS_DELIM=$(echo "$PKGS" | paste -sd "," -)
# should work as before replacing './...' with vars
go list -f '{{if or (len .TestGoFiles) (len .XTestGoFiles)}}go test -covermode count -coverprofile {{.Name}}_{{len .Imports}}_{{len .Deps}}.coverprofile -coverpkg $PKGS_DELIM {{.ImportPath}}{{end}}' $PKGS | xargs -I {} bash -c {}My Makefile after @mmindenhall help:
test:
@echo Running tests
$(eval PKGS := $(shell go list ./... | grep -v /vendor/))
$(eval PKGS_DELIM := $(shell echo $(PKGS) | sed -e 's/ /,/g'))
go list -f '{{if or (len .TestGoFiles) (len .XTestGoFiles)}}go test -test.v -test.timeout=120s -covermode=count -coverprofile={{.Name}}_{{len .Imports}}_{{len .Deps}}.coverprofile -coverpkg $(PKGS_DELIM) {{.ImportPath}}{{end}}' $(PKGS) | xargs -I {} bash -c {}
gocovmerge `ls *.coverprofile` > cover.out
rm *.coverprofileIs it possible to add total coverage percentage to an HTML report?
I belive it's golang issue, because HTML is generated by go tool.
As a workaround this idea comes into my mind:
- run go tool cover -func=cover.out
- get total statements value
- add it to the top of generated HTML
A slightly modified version of @rodrigocorsi2's Makefile command that dosn't need gocovmerge:
PKGS ?= $(shell go list ./... | /usr/bin/grep -v /vendor/)
PKGS_DELIM ?= $(shell echo $(PKGS) | sed -e 's/ /,/g')
test:
$(shell [ -e cover.out ] && rm cover.out)
go list -f '{{if or (len .TestGoFiles) (len .XTestGoFiles)}}go test -test.v -test.timeout=120s -covermode=count -coverprofile={{.Name}}_{{len .Imports}}_{{len .Deps}}.coverprofile -coverpkg $(PKGS_DELIM) {{.ImportPath}}{{end}}' $(PKGS) | xargs -I {} bash -c {}
echo "mode: count" > cover.out
grep -h -v "^mode:" *.coverprofile >> "cover.out"
rm *.coverprofile
go tool cover -html=cover.out -o=cover.html@krak3n if the tests have any overlap in lines that are covered that method won't work, that is why I wrote gocovmerge, to correctly merge coverage lines that are duplicated between the profiles.
@wadey I've been having some trouble getting @rodrigocorsi2's Makefile working. It would seem my bash-fu is not up to it.
What do you think of gocovmerge (or a similar tool) wrapping go test with the arguments passed through? So you get a lovely cover.out file in one step. That'd make it easy for anyone wanting to do this (not just me).
Of course it would be even better if there was a good way to address @hsanjuan's comment too -- and incorporate the changes into the cover tool directly via a CL.
What do you think of gocovmerge (or a similar tool) wrapping go test with the arguments passed through?
@nathany you can check out https://github.com/haya14busa/goverage which I believe does exactly this (although I haven't tested it myself).
@wadey @nathany check out https://github.com/go-playground/overalls it's been in existence for quite some time and is tried and true.
I agree it would be nice for it to be built right in.
@joeybloggs unless I'm missing something it looks like go-playground/overalls does not do smart merging of the profiles (like gocovmerge) and just dumps all of the output into one file (see here https://github.com/go-playground/overalls/blob/dab02cad1edafb5aa1d38729ba4bfd5618b836ce/overalls.go#L295-L297 ) This means it won't handle overlaps in coverage correctly.
@wadey I've never had a problem before, thanks I'll look into it; it seems pretty trivial to implement.
Thanks @wadey. The goverage tool from @haya14busa is looking good and was easy to set up.
The big problem with all the workarounds is that on larger repositories the time to run coverage is prohibitively slow. I'm working on a repo where unit tests run in 12 seconds using go test <list of packages>. Running each package separately (even without coverage) increases the test time to 200 seconds (the runtime is about the same with coverage enabled).
I'd like to contribute a fix for this issue. Looking at the patch from a few years ago and some experimenting I've done, it doesn't seem to be a difficult thing to fix.
This single 1 line change allows go test --coverprofile ... to run with multiple packages:
diff --git a/src/cmd/go/internal/test/testflag.go b/src/cmd/go/internal/test/testflag.go
@@ -164,7 +164,7 @@ func testFlags(args []string) (packageNames, passToTest []string) {
}
case "coverprofile":
testCover = true
- testProfile = true
case "covermode":
switch value {
case "set", "count", "atomic":The profiles are written to a file in the same directory as the package. It seems like what might be missing is support for multiple files in go tool cover, including merging of multiple profiles when there are overlaps.
Anything else that would need to change?
I'm working on a repo where unit tests run in 12 seconds using go test . Running each package separately (even without coverage) increases the test time to 200 seconds (the runtime is about the same with coverage enabled).
If this is true then the problem here is that when you run go test it is rebuilding the test packages' dependencies over and over. You can fix that by first doing 'go test -i ', which installs all the prerequisite packages, so that the separate command reuse them. My plan is that in Go 1.10 the go command will cache build results more aggressively so that the 'go test -i' step will not be necessary for the two ways to run in the same amount of time. But we're not there today.
Thanks! -i cuts the run time down to 34 seconds. Still slower than running all packages at once, but much better overall.
@dnephin I like the approach, any thoughts on whether or not this would be considered?
@rsc I would be happy to work on this for 1.10, provided anything required by this is in place by then. Any guidance from you or anyone else would be good. Also this can have the IDE label as well.
In the mean time, I made another tool: https://github.com/AlekSi/gocoverutil. Unlike some bash scripts mentioned there, it sets exit status correctly if tests fail, it sets -coverpkg to include all packages, etc.
But I'm really looking forward to retiring it when this issue and #21126 are closed.
@rsc now that the build cache is in, are we closer to fix this bug? I think people are waiting here for some guidance on how to fix it. I can attempt something myself, but it would be great if you could provide some guidance. Thanks!
Maybe a different way to see the problem is that we should have a different covermode for which we can apply this feature.
For example, running go test -coverprofile=coverage.out ./... should detect that the user wants to run in a recursive mode thus set the covermode to a recursive (or better named mode) in which, regardless of how many times a line is executed, it counts only as a simple boolean value (yes / no)?
Of course, this could also be done in a more complex way, by storing which test hit which line and then, on a per-line basis, and then offer for each line of code touched by tests a slice of test names. It would then be up to the tools to read that information and display it to the users, in the most simplistic way doing this as described above with a simple yes / no mode.
Sorry for the double message.
The first suggestions is a great way to get this started. But the more powerful and resourceful go test -converprofile is , the better.
Change https://golang.org/cl/76875 mentions this issue: cmd/go: allow -coverprofile with multiple packages being tested