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

Coverage of go test #140

Closed
GinFungYJF opened this issue Oct 13, 2016 · 50 comments · Fixed by #3117 or #3118
Closed

Coverage of go test #140

GinFungYJF opened this issue Oct 13, 2016 · 50 comments · Fixed by #3117 or #3118

Comments

@GinFungYJF
Copy link
Contributor

Does the go rule still have no support for calculating the coverage of go test now? If so, is there any way to calculate it?

@yugui
Copy link
Member

yugui commented Oct 13, 2016

It does not support coverage right now.
Probably we need to wait for bazelbuild/bazel#1118.

@GinFungYJF
Copy link
Contributor Author

Any update on this? Or is there any usable workaround like
bazelbuild/bazel#1118 ?

@pmbethe09
Copy link
Member

pmbethe09 commented Dec 1, 2016 via email

@ghasemloo
Copy link
Contributor

I think we can fix this directly, go tool itself support coverage independent of bazel.
I think what needs to be done is adding something similar to the block regarding coverage from go test main (https://golang.org/src/cmd/go/test.go?h=cover+flag#L1522)
to generate test main template: (https://github.com/bazelbuild/rules_go/blob/master/go/tools/generate_test_main.go#L129).

I tried it but I don't fully understand how flags are parsed in the go test, they are defined in
https://golang.org/src/cmd/go/testflag.go

@GinFungYJF
Copy link
Contributor Author

@ghasemloo When I run go test -cover -x pkg, I see that it will do some preproccessing for the files that are not *_test.go files in the pkg before they are compiled.

go/pkg/tool/linux_amd64/cover -mode set -var GoCover_0 -o .A.go pkg/A.go
go/pkg/tool/linux_amd64/cover -mode set -var GoCover_0 -o .B.go pkg/B.go
...

Is it helpful to you?

@linuxerwang
Copy link
Contributor

It would be great if the future bazel-go-coverage also supports external-test besides standard unit tests. Here are a few references of this technique:

https://blog.cloudflare.com/go-coverage-with-external-tests/
https://husobee.github.io/golang/test/coverage/2015/11/17/external-test-coverage.html
https://www.elastic.co/blog/code-coverage-for-your-golang-system-tests

I hope I could find time to work on it.

@pmbethe09
Copy link
Member

pmbethe09 commented Jan 23, 2017 via email

@GinFungYJF
Copy link
Contributor Author

GinFungYJF commented Mar 16, 2017

Anybody has any idea about implementing it ? I don't know where to start :(

@jayconrod
Copy link
Contributor

@GinFungYJF We haven't given this a lot of thought. It seems clear this will be a lot of work though.

Bazel has some support for exposing coverage information to Skylark. ctx.coverage_instrumented() would be called inside go_library_impl to check whether coverage instrumentation is needed. An instrumented_files provider should be returned (I'm not sure how Bazel uses that information though).

The Go code itself needs to be modified to include coverage instrumentation. The cover story provides some information on how this works. It looks like go test inserts coverage instrumentation at the source level before the compiler sees it. I'm not familiar with the implementation of go test, but test.go is the starting point. A lot of that implementation is private, and if it looks like we would need to duplicate it, we should talk to the Go Libraries team about providing a stable API we could use.

If you decide to work on this, please post a design doc that we can discuss and comment on.

@jayconrod
Copy link
Contributor

@GinFungYJF I spoke with @alandonovan, and he filled in some gaps in my knowledge here.

go tool cover is the tool that adds coverage instrumentation to Go sources. That's already part of the standard distribution, so we wouldn't need to re-implement that. We'd just need to generate actions and files inside the rules to make sure that sources are processed with that before they are compiled when coverage is enabled.

Bazel expects coverage information to be in lcov format. We have some code in Blaze (Google-internal version of Bazel) that produces lcov data at the end of a Go program using the counters added by the instrumentation. This is linked into instrumented binaries. We should be able to open source this or re-implement it without too much work.

@linuxerwang
Copy link
Contributor

@jayconrod Thank you for providing these info, although I've figured them out by myself. What held me back was actually I could not find a way to elegantly enable it in rules_go:

Suppose we have pkg deps like: A (main) -> B -> C. If we want to enable test coverage for A, we have to add corresponding BUILD targets also for B and C.

go_binary(name = "A",deps = ["B", "C"]
go_binary(name = "A_cover", test_coverage = true, deps = ["B_cover", "C_cover"])
go_library(name = "B")
go_library(name = "B_cover", test_coverage = true)
go_library(name = "C")
go_library(name = "C_cover", test_coverage = true)

This is too cumbersome. The ideal way is as follows:

go_binary(name = "A",deps = ["B", "C"]
go_test_coverage(name = "A_cover", deps = ["A"])

But it's probably impossible in bazel, since if A has been built before A_cover, A will not be triggered by A_cover to rebuild.

@pmbethe09
Copy link
Member

This might be possible with aspects

https://bazel.build/versions/master/docs/skylark/aspects.html

@jayconrod
Copy link
Contributor

@linuxerwang Ideally, you shouldn't need to define separate targets with and without coverage; it should be a configuration option. Bazel already has a coverage command that runs tests in coverage mode. We just need to wire up the rules to instrument code when that option is enabled and generate output in the appropriate format.

@linuxerwang
Copy link
Contributor

@pmbethe09, @jayconrod Cool! I'll invest some time on it this weekend.

@linuxerwang
Copy link
Contributor

Alright, I made an early stage experiment, the instrumentation is working. But we need more work on enable the coverage hookup in the generated test main func.

linuxerwang@42f5056

You can run "bazel coverage examples/lib:lib_test". It simply passes, without coverage collected. And the test binary has not coverage enabled:

$ ./bazel-bin/examples/lib/lib_test -test.coverprofile=report.txt
testing: cannot use -test.coverprofile because test binary was not built with coverage enabled

Of course, we also need more work on lcov format mentioned by @jayconrod.

External test coverage might be feasible through aspects, but I didn't get a chance to try.

@jayconrod
Copy link
Contributor

@linuxerwang Thanks for working on this, it looks promising. A couple comments:

  • It would be better to run go tool cover through an action for each file. This will let Bazel parallelize the coverage transformation and cache the results. Basically, define a new Skylark function which takes a list of sources as an argument, creates new files for the transformed sources (ctx.new_file), runs go tool cover for each of them (ctx.action), then returns the transformed files. _emit_go_compile_action can call this function if coverage is enabled and use the result as a replacement for srcs, before calling symlink_tree_commands.
  • Any new Skylark functions in def.bzl should be private (name starts with '_'). This prevents other projects from depending on implementation details.

@ghasemloo
Copy link
Contributor

what I was suggesting was something similar to what I did for benchmark:
https://github.com/ghasemloo/rules_go/commit/fa2088cd8d3e367902d6d896d1deccfec5da76c2
Essentially modify the main file that is generated by bazel for go tests (https://github.com/bazelbuild/rules_go/blob/master/go/tools/generate_test_main.go) to be similar to what go cover does.

I looked at it a bit but it seemed complicated than I had to time to dig into. I still feels it should be possible to just modify https://github.com/bazelbuild/rules_go/blob/master/go/tools/generate_test_main.go
to make bazel test with something like -- --cover work but I am not completely sure it is really viable or even the right thing to do.

@jayconrod
Copy link
Contributor

In the long run, it's important for us to be compatible with bazel coverage. That means transforming code in response to ctx.coverage_instrumented() and generating data in lcov format. Bazel has some (rather sparse) documentation on controlling instrumentation in Skylark.

I'm not sure what the current status of this in Bazel is right now. It doesn't seem like it works yet. I'm watching bazelbuild/bazel#1118 for updates though.

@linuxerwang
Copy link
Contributor

linuxerwang commented May 14, 2017

Finished the change as @jayconrod suggested. Created pull request #455.

Will work on generate_test_main.go in a later pull request.

@linuxerwang
Copy link
Contributor

linuxerwang commented May 15, 2017

Actually, added the change to test main generator in the same pull request.

For an example run, execute:

$ bazel coverage examples/lib:lib_test

It generates the following files:

$ ls -l bazel-out/local-fastbuild/bin/examples/lib/
total 2016
-r-xr-xr-x 1 linuxerwang linuxerwang     471 May 14 20:57 asm_GoCover_0.go <====
drwxrwxr-x 3 linuxerwang linuxerwang    4096 Mar 18 17:49 bazel-out
drwxrwxr-x 4 linuxerwang linuxerwang    4096 May 14 12:42 deep
drwxrwxr-x 3 linuxerwang linuxerwang    4096 May 13 21:01 github.com
-r-xr-xr-x 1 linuxerwang linuxerwang     623 May 14 20:57 lib_GoCover_1.go <====
-r-xr-xr-x 1 linuxerwang linuxerwang 1931232 May 14 20:57 lib_test
drwxrwxr-x 2 linuxerwang linuxerwang    4096 May 13 21:01 lib_test.dir
-r-xr-xr-x 1 linuxerwang linuxerwang     630 May 14 20:57 lib_test.GoTestGenTest.params
-r-xr-xr-x 1 linuxerwang linuxerwang   22936 May 14 20:57 lib_test_main_test.a
-r-xr-xr-x 1 linuxerwang linuxerwang     876 May 14 20:57 lib_test_main_test.a.GoLinkFile.params
-r-xr-xr-x 1 linuxerwang linuxerwang    1940 May 14 20:57 lib_test_main_test.go
-r-xr-xr-x 1 linuxerwang linuxerwang   22637 May 14 20:57 lib_test_main_test.o
-r-xr-xr-x 1 linuxerwang linuxerwang   32892 May 14 20:57 lib_test.o
drwxrwxr-x 3 linuxerwang linuxerwang    4096 Mar 18 17:49 lib_test.runfiles
-r-xr-xr-x 1 linuxerwang linuxerwang     183 May 14 20:57 lib_test.runfiles_manifest

The instrumented files are named in such a way that the generated test main can extract cover vars easily. The code logic in generate_test_main.go is borrowed shamelessly from $GOROOT/src/cmd/go/test.go and related files.

$ ./bazel-out/local-fastbuild/bin/examples/lib/lib_test
2 + 3 = 5
PASS
coverage: 40.0% of statements

It has the following features unimplemented:

  1. make -test.coverprofile=report.txt work
  2. add lcov output support
  3. make cover mode configurable
  4. add aspect to make external coverage tests work

@linuxerwang
Copy link
Contributor

linuxerwang commented May 28, 2017

I took a short time trying the coverprofile output. The currently generated test binary has all the mechanism outputting everything:

$ ./bazel-bin/examples/lib/lib_test -test.coverprofile=a.out
2 + 3 = 5
PASS
coverage: 40.0% of statements

$ cat examples/lib/a.out 
mode: set
bazel-out/local-fastbuild/bin/examples/lib/asm_GoCover_0.cover.go:8.22,10.2 1 1
bazel-out/local-fastbuild/bin/examples/lib/asm_GoCover_0.cover.go:12.22,14.2 1 0
bazel-out/local-fastbuild/bin/examples/lib/lib_GoCover_1.cover.go:27.20,29.2 1 1
bazel-out/local-fastbuild/bin/examples/lib/lib_GoCover_1.cover.go:34.23,36.2 1 0
bazel-out/local-fastbuild/bin/examples/lib/lib_GoCover_1.cover.go:39.25,41.2 1 0

However, I am stuck at two problems:

  1. how to pass the argument "-test.coverprofile=a.out" to "bazel coverage" run.
    I can use flag.Value.Set(), but probably a bad idea.

  2. how to put "a.out" in bazel-testlogs/examples/lib/lib_test.

Can bazel team please shed some light on it?

Thanks.

@jayconrod
Copy link
Contributor

If bazel coverage is run with no other arguments, I think it should communicate with Bazel in whatever format it expects (lcov?). I don't think coverage is really well-supported in Bazel yet though: bazelbuild/bazel#1118 is still open, although there has been some activity in the last few months.

You can still explicitly set the -test.coverageprofile flag using --test_arg. I don't think this should be set automatically though.

@linuxerwang
Copy link
Contributor

@jayconrod Question is, after running bazel coverage --test_arg=test.coverprofile=cover.out examples/lib:lib_test, where is the file cover.out generated? I could not find it anywhere.

@Mistobaan
Copy link

Any progress on how to run coverage profiling using bazel? is it possible as of today? I can run bazel coverage on my tests but then is not clear how I inspect the results.

@jayconrod
Copy link
Contributor

No change on this recently. We've been focusing on toolchains, proto support, and dependency management lately. I'm hoping to get coverage into a better state by the end of the year.

@dkt1412
Copy link

dkt1412 commented Nov 10, 2017

So I ran this (quotes are significant!)
bazel coverage \<test-target\> --test_arg="-test.coverprofile=coverage.out"
and this generated the coverage file (woohoo) at
bazel-bin/\<test-target-path\>/\<target-test\>.runfiles/\<test-target-path\>/coverage.out
The problem I have now is how to turn this into a html report. I can't seem to use go tool cover --html=\<path-to-coverage.out\> b/c main.go doesn't actually live in my GOPATH (which I also can't set up b/c our bazel project doesn't have the go-required src/ setup).

Is there a way to get bazel to run go tool cover --html for us? I assume we'd have to add some sort of actions to link to go tool cover like @linuxerwang did, but it's unclear to me where that needs to go.

@jayconrod
Copy link
Contributor

Sorry, we still don't have a good story for this yet. I hope we'll be able to get to it soon, but cross-compilation and dependency management are bigger pain points right now.

You could try using the go_path rule, which builds a GOPATH-like source tree. Maybe cover could be run in there. See BUILD.bazel for an example of that.

@sluongng
Copy link
Contributor

I have some time today after taking a good break from bazel.
Made small progress:

# Generate test coverage data in Cobertura's XML format
#
# Running `bazel coverage //...` is required *before* building this target.
#
# This is the same as:
#   gocovmerge bazel-out/**/coverage.dat | <set env vars> gocover-cobertura >bazel-bin/coverage.dat
genrule(
    name = "cobertura-coverage-data",
    srcs = [":coverage-data"],
    outs = ["cobertura.xml"],
    cmd = " ".join([
        "$(locations @com_github_wadey_gocovmerge//:gocovmerge)",
        "$(locations //:coverage-data)",
        "|",
        "PATH=$$PATH:$$(dirname $(locations @go_sdk//:bin/go))",
        "GOPROXY=https://my-internal-go-proxy/",
        "GONOSUMDB=vcs.org.com",
        "GOCACHE=/tmp",
        "HOME=/tmp",
        "$(locations @com_github_boumenot_gocover_cobertura//:gocover-cobertura)",
        ">$@",
    ]),
    tags = [
        "local",
        "manual",
    ],
    tools = [
        "@go_sdk//:bin/go",
        "@com_github_wadey_gocovmerge//:gocovmerge",
        "@com_github_boumenot_gocover_cobertura//:gocover-cobertura",
    ],
)

The above worked, but setting those environment variables are hacky at best 🤔

The problem is mainly that gocover-cobertura https://github.com/boumenot/gocover-cobertura/blob/7bc1b925ff29267be04ee0693f39a099ea54f387/gocover-cobertura.go#L18 depends on golang.org/x/tools/go/packages which shell out to collect packages information using go list -json (?).

For that reason:

  • go binary needs to be available in $PATH
  • GOCACHE and HOME needed to be set
  • GOPROXY and GONOSUMDB is for the whole thing to work with my internal packages

After a very quick read, I think I may be able to decouple gocover-cobertura away from tools/go/packages by making some naive assumption about my setup 🤔 thus simplify how this is generated.

@AFMiziara
Copy link

@sluongng in my case I am trying to generate a unified coverage.out using bazel coverage //.... I tried your approach, but I am kind new to bazel. How did you import @com_github_wadey_gocovmerge as a dependency if this git repository does not have bazel configuration?

@fmeum
Copy link
Collaborator

fmeum commented Sep 13, 2021

Would a contribution adding configurable lcov output be welcome? This could be enabled via a setting such as @io_bazel_rules_go//go/config:lcov and would allow Bazel's --combined_report=lcov to work.

@achew22
Copy link
Member

achew22 commented Sep 13, 2021

It would definitely be welcome. What resources would you need to help you get a PR to handle this set up?

@fmeum
Copy link
Collaborator

fmeum commented Sep 13, 2021

It would definitely be welcome. What resources would you need to help you get a PR to handle this set up?

I think I have figured out the setup within rules_go. What is left is writing the actual Go tool that does the conversion and converts importpaths into workspace-relative paths. I will let you know when I hit any roadblocks.

Edit: It would be great if there were a way to depend on the value of --combined_report via a config_setting, but that gives an error. If you have an idea why, this could even be implemented without a separate flag.

@fmeum
Copy link
Collaborator

fmeum commented Sep 14, 2021

@achew22 Is it possible that emit_cover is entirely unused by rules_go? I wanted to try changing the srcname attribute to cover, but then noticed that this is never executed.

@sluongng
Copy link
Contributor

@achew22 Is it possible that emit_cover is entirely unused by rules_go? I wanted to try changing the srcname attribute to cover, but then noticed that this is never executed.

https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/bazelbuild/rules_go%24%405efc3c2+emit_cover&patternType=regexp this should help somewhat

It should be running as I am able to collect coverage via bazel coverage //... still and emit_cover is essentially using go tool cover to generate the modified source code.

@fmeum
Copy link
Collaborator

fmeum commented Sep 14, 2021

@achew22 Is it possible that emit_cover is entirely unused by rules_go? I wanted to try changing the srcname attribute to cover, but then noticed that this is never executed.

https://sourcegraph.com/search?q=context:global+repo:%5Egithub%5C.com/bazelbuild/rules_go%24%405efc3c2+emit_cover&patternType=regexp this should help somewhat

It should be running as I am able to collect coverage via bazel coverage //... still and emit_cover is essentially using go tool cover to generate the modified source code.

Yes, bazel coverage //... works, but I think that is due to this part of compilepkg.bzl. The extended logic in emit_coverage does not seem to be invoked though.

@togiles
Copy link

togiles commented Apr 13, 2022

@fmeum curious if you've made any further progress to get --combined_report=lcov working for go tests? I have a workaround for getting a combined coverage report in the go format that I'm using, but we'd really love to get our IDE coverage integration (e.g. intelliJ bazelbuild/intellij#159).

FWIW, here's my workaround for merging go coverage.dat reports and inspecting them manually ... but we really need --combined_report=lcov to work properly for first class support in our IDEs, and automated CI pipelines.

Generate all coverage reports across my repo

bazel coverage //...

Merge them all into a single report:

echo "mode: set" > /tmp/coverage.dat
cat `find ./bazel-out/ -name "coverage.dat"` | grep -v "mode: set" >> /tmp/coverage.dat

View the coverage report in your browser using go tool cover:

go tool cover --html=/tmp/coverage.dat

@fmeum
Copy link
Collaborator

fmeum commented Apr 14, 2022

@togiles Yes, I just submitted #3117. Could you give it a try and take a look at whether the reports it generates look reasonable?

@fmeum
Copy link
Collaborator

fmeum commented Apr 15, 2022

@linzhp Could you reopen? Sorry, I didn't know that a Fixes <comment link> reference would also end up closing the issue.

@linzhp linzhp reopened this Apr 15, 2022
phst added a commit to phst/rules_elisp that referenced this issue Apr 15, 2022
@togiles
Copy link

togiles commented Apr 27, 2022

@linzhp --- what is the typical time period for new releases? Also, is there an easy way to pull in a release directly after this fix w/out waiting for the new official release roll-up?

@linzhp
Copy link
Contributor

linzhp commented Apr 27, 2022

I try to have one release a month as long as there are some major features. Since we have something new this month, I can release a new version this week. You can depend on a specific commit of rules_go before an official release

@linzhp
Copy link
Contributor

linzhp commented Apr 27, 2022

@togiles Would you mind doing Step 1-5 in the minor release process and create a PR like #3049? It would be easier for me to do the rest of the release.

@togiles
Copy link

togiles commented Apr 28, 2022

Was (am) glad to do it ... however didn't have time to debug the first step - running into this issue as I'm attempting to do it on my mac - result of bazel run //go/tools/releaser --upgrade-dep all:

ERROR: /private/var/tmp/_bazel_tgiles/31564b2260a31ec0983c805b40d89c3a/external/org_golang_x_crypto/openpgp/elgamal/BUILD.bazel:3:11: GoCompilePkg external/org_golang_x_crypto/openpgp/elgamal/elgamal.a failed: I/O exception during sandboxed execution: xcrun failed with code 69.
This most likely indicates that SDK version [10.10] for platform [MacOSX] is unsupported for the target version of xcode.

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