-
Notifications
You must be signed in to change notification settings - Fork 278
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
code coverage support #184
Comments
Has anyone started working on this? This is going to become a priority from my team soon. We're working on replacing our reliance on sbt in our jenkins pipeline with bazel. One part of that is figuring out the test report and codecov ( scoverage ) stories. |
I don't know (and think) anyone started working on it but I'll add that
there's been a lot of discussion on various coverage issues on bazel itself
so I'd recommend anyone tackling this to read a bit beforehand
…On Fri, 5 May 2017 at 20:40 doug tangren ***@***.***> wrote:
Has anyone started working on this? This is going to become a priority
from my team soon. We're working on replacing our reliance on sbt in our
jenkins pipeline with bazel. One part of that is figuring out the test
report and codecov ( scoverage ) stories.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub
<#184 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF4kLM62g_5DrI10vlWYAwmLVb7Dfks5r217wgaJpZM4NG3kM>
.
|
👍 I was looking through the machinery that ties scoverage into sbt, and it looks like it may be possible to to just opt into code coverage for a regular scala_test by threading through a few additional arguments flags: namely: |
opt in meaning maybe this is possible today without any additional changes and when the coverage story in bazel settles something like this could then just become a conveniences of being instrumented by default with running |
The scala side should be easy. What I don't know is how to plug in to
bazel's expectations (if they are even standard). Like where should we
write the coverage files to? What should the format be?
…On Fri, May 5, 2017 at 08:41 doug tangren ***@***.***> wrote:
opt in meaning maybe this is possible today without any additional changes
and when the coverage story in bazel settles something like this could then
just become a conveniences of being instrumented by default with running bazel
cover
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#184 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEJdgmLCVqU5WVE95c7FE6dhkMXP38Tks5r221OgaJpZM4NG3kM>
.
|
My guess is it'll be the same as the test_filter flag story. When x is present in the env ( or args ) do y. My guess about where the conventional place to write reports would be customizable with rule dependent defaults, like junit test reports |
I really suggest searching the Bazel github issues with the term coverage.
There have been several discussions about it in the issues.
…On Fri, 5 May 2017 at 21:56 doug tangren ***@***.***> wrote:
My guess is it'll be the same as the test_filter flag story. When x is
present in the env ( or args ) do y. My guess about where the conventional
place to write reports would be customizable with rule dependent defaults,
like junit test reports
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIFzVFzcvA0v4oPL3Z6UF9Srf6YWNZks5r23DSgaJpZM4NG3kM>
.
|
@johnynek do you know ( specifically for scalac ) if there needs to be anything special done for scalac plugins to set up a special kind of classpath separate form the classpath of the files being compiled? I've now optimistically convinced myself this should be more straightforward that I originally thought so I couldn't help trying out a POC to close out my friday on a happy note. my WORKSPACE # other stuff ...
filegroup(
name = "scoverage_plugin",
srcs = ["sbt-plugins/scalac-scoverage-plugin_2.11-1.2.0.jar"],
visibility = ["//visibility:public"],
)
java_import(
name = "sbt-plugins",
jars=glob([
"sbt-plugins/scalac-scoverage*.jar"],
exclude = [
"sbt-plugins/scala-library*jar",
"sbt-plugins/scala-xml*jar"
]),
visibility = ["//visibility:public"],
) my BUILD scala_test(
name = "foo",
size = "small",
srcs = glob(
[
"src/test/scala/**/*.scala",
"src/test/java/**/*.java",
],
),
jvm_flags = [
"-Dfile.encoding=UTF-8",
],
plugins = ["@meetuplib//:scoverage_plugin"], # the scoverage scalac plugin jar
scalacopts = [
"-P:scoverage:dataDir:/path/to/target/scovtest/scoverage-data",
"-Yrangepos"
],
deps = [
# other stuff....
"@meetuplib//:sbt-plugins" # puts scoverage deps on my classpath
],
) when I run
The only thing that turned up in a lazy google search was this issue that popped on up the intellij mailing list. I didn't quite get the solution in that case or how it may be part of the problem in mine. |
I was actually speaking with ahirreddy@ about this the other day. It might be easier just to build the plugin as a deploy_jar for now though. |
Now I'm thinking there may be something to do with the bootclasspath which may actually be related to an open bug with scala. One thing I failed to mention above when setting up my test was that initially I didn't have the scalac-scoverage-runtime jar on my classpath which caused a similar issue. After adding that to my scala_tests' deps, that problem was resolved. The next issue I ran into was the I tried the change mentioned in the issue by adding the -nobootcp to my I'm a little confused because the identity of this class technically didn't change between scala 2.10 and 2.11 so either way I expected it to be on the classpath one way or another. |
I have a little bit more mental energy to this but I'm still stumped on the |
we should be able to use a target in the current repo as a plugin. If we can't now, that shouldn't be too much work. Then, we could just make a deploy jar ourselves by linking in the full path. |
Sorry, I never really described what deploy_jar is although seems like you found out but just in case: The deploy_jar is a fat-jar that contains the current compiled target and all its transitive dependencies (both runtime and compile time). It is analogous to the deploy_jar you can get from a java_binary (although not executable if from scala_library). I realized over the weekend that just directly using a deploy_jar from one of the rules_scala rules probably won't work because I don't think it will put the plugin.xml in the right place. So, would need to take the deploy_jar from the rules and have another action/genrule put the plugin.xml inside that fat jar. I evntually wanted to demonstrate this by replacing the linter plugin in the tests with a custom rolled one (both to demonstrate how to make plugins and drop a rules_scala dependency). Unfortunately, I am really reluctant to say I have the time to do this :( . Had spent my 'free-work' time over weekend pushing through the bazel changes outlined in: #57 (comment) PS: Do you think it is a problem that using two plugins might have redundant classes in their fat-jars? (Assuming that the classes are identical duplicates then it seems like it would just be like a 'stylistic' issue versus a functional one.) |
How do you make a deploy_jar without a main class, do scalac plugins run with a java main? Also, I can kind of half confirm the deploy_jar thing works. I cloned the scoverage repo and used sbt-assembly to bake me a fat jar. I referenced that in a Next issue is related to sandboxing which is probably just something Im doing wrong. Given the scalacopt for the plugin's data dir (
This kind of makes sense because sandboxing shouldn't let me write outside. To continue the POC I added
that allows the scalac plugin to generate two files
So thats the recording process in a nutshell. Report generation... that's a whole other beast but I just need to confirm that the coverage recordings are all that my jenkins plugin requires. Reports may not be a need to have for me atm. |
I'm happy to help but will probably need more direction, I have a decent sense for bazel's architectural model still have some blindspots. I'm very much in a POC phase right now where Im testing to see if it can work in a timeboxed period of time. The next to think about is probably more relevant to the discussion on bazel core's issue tracker. How should the library rules detect the conditional case for appending the scalac plugin. You typically will only want to do that in some testing mode. What you ship shouldn't have the scoverage instrumentation. |
circling back to this. I had a few questions that I think could be answered faster here but perhaps should be asked on the bazel-discuss list.
I think I found some examples of where I can create the scoverage binary jar here. I think that was suggested above. I'm just double check to make sure that's the right pattern to follow.
So the way to know the coverage should be enabled or not is documented here. The problem I'm running into is that I need access to the scoverage jar to conditionally append it to this list. What I haven't seem examples of yet in bazel rules is how to dynamically access a dependency from within an implementation. My guess is that there's something fundamentally wrong with me asking how to do that because bazel needs to run though the analysis phase first before I have the context. That said, pointers are welcome!
This is related to the above. I'll need to provide the scoverage enabled compile run |
The answer came to me on the train ride home. I can just declare these as private ( |
I put together an initial prototype that was intended as a pt 1 of the process to have scoverage record starting measurements here One part I'm getting stuck on is the directory that scoverage should write data to I'm currently using a subdirectory of testlogs scoverage_data_dir = "bazel-testlogs/{pkg}/{name}/scoverage-data".format(
pkg=ctx.label.package,
name=ctx.label.name
) This doesn't quite work and I'll allude to why
note: dispite the fact that scoverage says it wrote the initial coverage.xml file it doesn't actually exist
An the reason is test logs are attributed to test packages not library packages. Looking for pointers on an alternative approach for picking this directory. I did notice that the JUnit xml reporters using and env var to know where to record data. I'm not sure if there's a similar feature for coverage writers. I don't think that skylark rules can access env vars if there was. |
status update. I updated my branch with a change that partially addresses the coverage dir problem. I realized that the action that writes the coverage data needs to own the process of creating the directory so I ended up threading the coverage dir as a new
But I am still unsure how to provide that directory to the test runner which I think I need to write the measurement data to. Let me know what you folks think of my current direction. I'd like to pivot early in the process of I'm going down the wrong direction.
|
I assume there is some env-var similar to test where we are supposed to write the file, and in a particular format, but I really haven't looked. I would google the bazel google group and see the story there. |
Following up but still blocked. I posted the bazel discuss list to ask for some direction/input but haven't gotten as much feed back as I've gotten here. Consider this a repost of my current state. I'm trying to follow along with changes that were brought into 5.0 that may help me get code coverage working for scala. My current blocker is knowing where to write coverage data to. If I'm reading this right, this should be the block of code that exposes set of If anyone here has time to take a quick look at what I have so far, below is our diff of rules_scala |
Not that familiar with coverage but from my experience the Bazel people
monitor the Bazel tag in SO more than the discuss group. Maybe posting
there will yield faster results.
…On Tue, 30 May 2017 at 21:20 doug tangren ***@***.***> wrote:
Following up but still blocked. I posted the bazel discuss list
<https://groups.google.com/d/msg/bazel-discuss/pJQBD-vWaVo/1K5txXhWCAAJ>
to ask for some direction/input but haven't gotten as much feed back as
I've gotten here. Consider this a repost of my current state.
I'm trying to follow along with changes that were brought into 5.0 that
may help me get code coverage working for scala.
My current blocker is knowing where to write coverage data to.
If I'm reading this right, this
<https://github.com/bazelbuild/bazel/blob/1fd27bc4d97533031401c54aa05eb75b13c6874b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java#L427-L435>
should be the block of code that exposes set of COVERAGE_* environment
variables to test runs. I can see that coverage is enabled by inspecting
ctx.configuration.coverage_enabled when I run bazel coverage and I
believe that I'm setting the right instrumented_files field in the struct
scala_library returns, but it's not immediately clear why coverage data
may be null
<https://github.com/bazelbuild/bazel/blob/1fd27bc4d97533031401c54aa05eb75b13c6874b/src/main/java/com/google/devtools/build/lib/rules/test/TestRunnerAction.java#L559-L561>,
which may be why I'm not seeing those env variables with I run bazel
coverage -s
If anyone here has time to take a quick look at what I have so far, below
is our diff of rules_scala
***@***.***
<meetuparchive@master...scoverage-support>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#184 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ABUIF-iz-48pNg58JGjWg7abgHH2I_ubks5r_F4IgaJpZM4NG3kM>
.
|
FYI, here's how pants integrates with jacoco for code coverage: |
http://www.jacoco.org/jacoco/trunk/doc/agent.html documentation on using the java agent. Seems like it might not be that much work. |
Does jacoco work for scala as well? At Meetup I know some engineers use jacoco specifically for java and scoverage for everything else. Any new news on this enabling this would be great! I kind of dropped my initial effort of getting scoverage to work with bazel in favor of letting a jenkins job using sbt to generated reports be our answer for coverage as we moved our primary building over to bazel. My original scoverage branch for this scala_rules is likely horrible unmergable with this master branch by now but I still have it here for reference. |
It looks like Jacoco works on byte code unless I’m wrong. So it may work for both java and scala. |
I think maybe the only open issue is converting to lcov format, which I think is what bazel is standardizing on. |
@iirina Any advice on how to proceed with Scala coverage now that |
For Scala the |
@greggdonovan I put together this document with the instructions. I want to write a blog post, but to speed things up a doc should work for now. |
Is anyone actively working on bazel scala coverage right now? It seems like @softprops had a partially working solution that would instrument the classes using scoverage, but there are a few blockers regarding collecting the coverage data and the output format for the coverage data. I'd be interested in picking things up, but it looks like there are a couple options moving forward. We can implement the coverage using Jacoco and potentially plug into Bazel's JacocoTestRunner or we can continue with the Scoverage changes which may require some custom tooling to handle coverage formats. |
@prebeta Which one provides more accurate coverage reports for Scala code? |
@dsilvasc from my initial investigations, the branch/class coverage numbers are pretty much the same between the two. The issue with mixins skewing coverage results in Jacoco seems has been resolved in 8.0+. However, scoverage is able to give more precise statement coverage in the case of single line conditionals, but I'm not sure if this is even translatable into lcov format. Here's an example of scoverage vs jacoco reports. Scoverage report: https://i.imgur.com/ShYX29f.png Jacoco report: https://i.imgur.com/0YnDnxF.png |
@andyscott added preliminary code coverage support in #692 It's 'preliminary' because it produces lcov coverage traces, but iiuc there were some bazel bugs blocking bazel's built in support for generating coverage reports. You have to run I did however have some success uploading the coverage traces to codecov.io for a small test project. |
@beala-stripe Thanks for the pointer! I was doing some testing and it seems to work well. :) However, I'm hitting an issue when generating coverage using genhtml, the sources listed in coverage.dat files are relative to a scala root directory instead of the project root. For example, I have this project structure:
But when I run
instead of:
and it makes The issue looks very similar to: bazelbuild/bazel#2528, and the solution was to pass in I'm currently on bazel version 0.23.1, and the
It looks like the flag has been enabled by default in 0.24.1: bazelbuild/bazel@e64066d
|
@prebeta-- We have the same issue internally at Stripe! To work around this I've pieced together a Python script that rebuilds |
@andyscott thanks for clarifying :) I've worked around the pathing issues as well with a similar script, I just wanted to bring attention to the potential issues with newer versions of bazel and having |
What's the current story on this and are they examples to get started, perhaps for folks coming from the land of sbt? |
Let me summarize the state (as I see it) for December 2019 (Bazel 1.2.0 and rules_scala 26cf9b7). In the latest versions of Bazel --experimental_java_coverage is removed. To run code coverage with Jacoco for Scala code you can run (you need the extra toolchain as the scala support wasn't enabled by default - please see the comments in #692): bazel coverage --extra_toolchains="@io_bazel_rules_scala//test/coverage:enable_code_coverage_aspect" //... It will produce several .dat files for your modules. Then you've got two issues to solve:
Both can be solved by using an approach taken e.g. by Gerrit project. Using a modified version of this script solved the html report problem for me. |
I believe this issue could be closed as Jacoco code coverage for Scala works. |
Thanks! from your previous post it sounds like it works but in awkward way. Is this bazel's fault, rules_scala's fault or just how coverage business works? (We don't do coverage internally so I don't really know) |
If we have a working example can we document it. Some times as passed so things may have changed but this is where we're currently blocked scala_rules@5261499b0485f33799a1b210796fcdfa720a5344
|
|
Your rules_scala version seems to be a bit old (August 2019). I tried with 26cf9b7, that is from November 2019. Also, I used rules_jvm_external 3.1 for this test to add the JVM dependencies. |
Here is a working example: https://github.com/gergelyfabian/bazel-scala-example Run: bazel coverage --extra_toolchains="@io_bazel_rules_scala//test/coverage:enable_code_coverage_aspect" //... EDIT: example was updated with multiple targets generating code coverage and a demonstration how to use genhtml (with a script inspired by Gerrit project). |
Maybe this could be a documentation PR?
On Thu, 2 Jan 2020 at 18:13 Gergely Fábián ***@***.***> wrote:
Here is a working example:
https://github.com/gergelyfabian/bazel-scala-example
Run:
bazel coverage
***@***.***_bazel_rules_scala//test/coverage:enable_code_coverage_aspect"
//...
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#184?email_source=notifications&email_token=AAKQQF3R7JU6FMP5NRSQWVLQ3YG4BA5CNFSM4DI3PEGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEH6WPQQ#issuecomment-570255298>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKQQF5NLTKMQC4RHIFEYPTQ3YG4BANCNFSM4DI3PEGA>
.
--
*Ittai Zeidman*
40 Hanamal street, Tel Aviv, Israel
<http://www.wix.com>
|
Sure :) However, please let me know what you mean by that, as I'm a bazel beginner :) |
I mean send a PR to rules_scala which adds a detailed explanation on how to
use coverage.
It sounds like this can be lengthy so maybe have a separate markdown file
and link to it from the main readme?
On Tue, 7 Jan 2020 at 13:25 Gergely Fábián ***@***.***> wrote:
Maybe this could be a documentation PR?
Sure :) However, please let me know what you mean by that, as I'm a bazel
beginner :)
How should I create a documentation PR, and for which component?
rules_scala I guess?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#184?email_source=notifications&email_token=AAKQQF2W3IM224GLIQPMKSLQ4RRBXA5CNFSM4DI3PEGKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIIR7JY#issuecomment-571547559>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAKQQF6GB5RMABE3E2OUMELQ4RRBXANCNFSM4DI3PEGA>
.
--
*Ittai Zeidman*
40 Hanamal street, Tel Aviv, Israel
<http://www.wix.com>
|
Just a quick update for anyone still paying attention: #1006 This should completely remove the need to run any scripts to fix file paths. You should just be able to run genhtml from your directory and passing in the coverage.dat (although it will add noise to your code, but that's a different conversations 😂) |
After merging #1006 to master indeed code coverage support in rules_scala become a lot better. I confirm that for my usecase indeed simply running genhtml works without any path fixes. |
@gergelyfabian great! Wdyt about sending a PR to the readme of what one should do to get coverage? |
Here is the PR for documentation on coverage: #1017 I only tested with scalatest. |
Regarding scoverage integration: FYI, as commented in
the scala side isn't hard, we can create an extra instrumented version of build target, then it writes to the location we specified. like this https://github.com/tanishiking/bazel-playground/blob/main/18-scala-scoverage The thing is "how to plug in to bazel's expectations (if they are even standard)", and maybe now we have good references
|
Hey, I've been trying to get https://github.com/tanishiking/bazel-playground/tree/main/18-scala-scoverage How scoverage-scalac-plugin works?While JaCoCo instrument the compiled bytecode, Scoverage instrument against the source code (Scala AST) in the very early stages of the compilation phases. To instrument the given scala code, we'll provde the following scalacopts -Xplugin:/path/to/plugin/scalac-scoverage-plugin_1.4.11.jar # for scoverage >2, we have to add some more deps
-P:scoverage:dataDir:/path/to/project/output
-P:scoverage:sourceRoot:/path-to/project/root
Watch @ckipp01's video for more details 👍 Naive integrationSo, I tried to make PoC scoverage + rules_scala works here https://github.com/tanishiking/bazel-playground/tree/main/18-scala-scoverage
Even though it doesn't follow "bazel way" (it generates coverage report by However, there're plenty of problems with this implementation, of course. (1)
|
+1 |
This would look like using:
https://github.com/scoverage/scalac-scoverage-plugin
to get the scala test rules to emit code coverage reports, then aggregating them across the repo.
It is related to bazelbuild/bazel#1118 but I don't think it is actually a blocker. We may just be able to produce coverage output of each test rule, then an aspect to walk all the rules and aggregate a total.
The text was updated successfully, but these errors were encountered: