-
-
Notifications
You must be signed in to change notification settings - Fork 673
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
Comments
It does not support coverage right now. |
Any update on this? Or is there any usable workaround like |
AFAIK no one is working on this. If it is something you need, you may have
to be the one to implement it.
…On Thu, Dec 1, 2016 at 7:18 AM, GinFungYJF ***@***.***> wrote:
Any update on this? Or is there any usable workaround like
bazelbuild/bazel#1118 <bazelbuild/bazel#1118> ?
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#140 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALF-0gypcrlX564I8vd_Om9fLXppZ3mAks5rDrskgaJpZM4KVagd>
.
|
I think we can fix this directly, go tool itself support coverage independent of bazel. I tried it but I don't fully understand how flags are parsed in the go test, they are defined in |
@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.
Is it helpful to you? |
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/ I hope I could find time to work on it. |
@linuxerwang -- sounds great. Feel free to send some ideas here and then
of course send a PR .
…On Sat, Jan 21, 2017 at 7:41 AM, linuxerwang ***@***.***> wrote:
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.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#140 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ALF-0kVhZC4CZ0nFDvf92v2h8TbnemMdks5rUfz1gaJpZM4KVagd>
.
|
Anybody has any idea about implementing it ? I don't know where to start :( |
@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. 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 If you decide to work on this, please post a design doc that we can discuss and comment on. |
@GinFungYJF I spoke with @alandonovan, and he filled in some gaps in my knowledge here.
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. |
@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"] This is too cumbersome. The ideal way is as follows: go_binary(name = "A",deps = ["B", "C"] 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. |
This might be possible with aspects https://bazel.build/versions/master/docs/skylark/aspects.html |
@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. |
@pmbethe09, @jayconrod Cool! I'll invest some time on it this weekend. |
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. 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 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. |
@linuxerwang Thanks for working on this, it looks promising. A couple comments:
|
what I was suggesting was something similar to what I did for benchmark: 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 |
In the long run, it's important for us to be compatible with 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. |
Finished the change as @jayconrod suggested. Created pull request #455. Will work on generate_test_main.go in a later pull request. |
Actually, added the change to test main generator in the same pull request. For an example run, execute:
It generates the following files:
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.
It has the following features unimplemented:
|
I took a short time trying the coverprofile output. The currently generated test binary has all the mechanism outputting everything:
However, I am stuck at two problems:
Can bazel team please shed some light on it? Thanks. |
If You can still explicitly set the |
@jayconrod Question is, after running |
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. |
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. |
So I ran this (quotes are significant!) Is there a way to get bazel to run |
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 |
I have some time today after taking a good break from bazel.
The above worked, but setting those environment variables are hacky at best 🤔 The problem is mainly that For that reason:
After a very quick read, I think I may be able to decouple |
@sluongng in my case I am trying to generate a unified |
Would a contribution adding configurable lcov output be welcome? This could be enabled via a setting such as |
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 Edit: It would be great if there were a way to depend on the value of |
@achew22 Is it possible that emit_cover is entirely unused by |
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 |
Yes, |
@fmeum curious if you've made any further progress to get FWIW, here's my workaround for merging go coverage.dat reports and inspecting them manually ... but we really need Generate all coverage reports across my repo
Merge them all into a single report:
View the coverage report in your browser using
|
@linzhp Could you reopen? Sorry, I didn't know that a |
@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? |
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 |
@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. |
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
|
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?
The text was updated successfully, but these errors were encountered: