State of code coverage?

[keith]

I've been looking through bazel/rules_apple/apple_support/rules_swift and it appears that there's a decent amount of integration for supporting code coverage that could work for iOS. But it seems like some critical pieces are not hooked up, like empty filegroups, stub binaries and variables that wouldn't be passed through to iOS tests.

What's the plan with supporting code coverage for iOS? Is this something that's already supported inside of Google but not in the public for some reason?

[thomasvl]

@allevato likely knows the most about this.

[allevato]

Indeed, the coverage support in the rules is currently for 🤫 internal use 🤫 at this time.

I'm not too up-to-date on the current state of Bazel coverage. Bazel parses lcov tracefiles to generate its coverage reports, and I recently added support to llvm-cov to export lcov formatted files (llvm/llvm-project@b2091c9) so that we could use that within Google, but externally you'd need to wait for an Xcode release that includes that patch or build your own copy for it to be useful.

[keith]

Is that the only thing missing from making this work externally? (It seems like the rules and test runner might also need some changes?)

[allevato]

It looks like the version of llvm-cov distributed with Xcode 11 beta includes the --format=lcov flag that I added, so once that's in place, it would be up to the coverage collection scripts in Bazel to invoke it and transform the raw coverage data into the format it uses.

[tinder-maxwellelliott]

Any update on this ticket? Looking to start trying to surface some coverage on my Bazel project. I would believe this would be supported if the last ticket was from the xcode 11 beta

[keith]

We have this working on our project internally, but we have a custom test runner to do so. But I can outline the gist of how it works here:

1. Set --experimental_use_llvm_covmap globally
2. Set --test_env=LCOV_MERGER=/usr/bin/true because of Add _lcov_merger attribute to test rules #691 (or apply that patch if you really want to use bazel's merging functionality)
3. Pass LLVM_PROFILE_FILE to your test runner's environment. Our internal test runner uses xctestrun files and sets that key in the TestingEnvironmentVariables args
4. The path you set in that var produces a profraw file
5. Run xcrun llvm-profdata merge "$profraw" -output "$profdata"
6. Process the profdata file however you'd like in the test runner with access to the binary.

A few gotchas:

• If you're using remote caching you'll probably need to run coverage jobs with a single stable --output_base because when doing path remapping with llvm-cov you can only remap a single path
• llvm-cov expects local relative files, so when you're parsing the coverage data we first pushd "$ROOT" ($ROOT is from bazel) to make sure the tested file paths are actually relative
• llvm-cov doesn't always error when you'd expect it to, to make sure we don't let it produce invalid data because of silent errors we run llvm-cov show, and check if it produces any output to stderr, and if it does we error since this means something is misconfigured
• Transitive source files are included in the COVERAGE_MANIFEST (from bazel) which you may not want depending on how you think about it. We remove them from the file to make sure we only count coverage of the target we're directly testing

[tinder-maxwellelliott]

Thank you @keith for these tips, I used them alongside this blog post to put together a sample app that uses Coveralls to show test coverage with Bazel https://github.com/tinder-maxwellelliott/ios-bazel-test-with-coverage

This approach works fine when you only have 1 unit test suite. In the real world you will have several, to make sure to get coverage for all of them you would need to:

1. Run the bazel coverage command (seen in the repo above) on each target, you will create a profraw file for each
2. Merge all the profraw files into a single profdata file via xcrun llvm-profdata merge
3. For each unit test you will run xcrun llvm-cov export --format=lcov using the path to the xctest bundle binary for each test target (you can see an example of this in the repo above)
4. Update paths in lcov files to not be relative to $(bazel info execution_root) but instead to your repo's root. There is an example of this in the repo above
5. Upload your multiple lcov files to your code coverage provider. If they do not support lcov merging support then you may need to instead parse these by hand and do your own line coverage merging each targets files.

[keith]

Nice example. Note in general you should prefer bazel coverage over bazel test, which will also set --collect_code_coverage for you (and potentially flip some other things).

FWIW I think we should update the test runner in this repo to at least have some minimal amount of coverage support, so if you'd like to submit a PR for that I'd be happy to review it!

[tinder-maxwellelliott]

Nice example. Note in general you should prefer bazel coverage over bazel test, which will also set --collect_code_coverage for you (and potentially flip some other things).

FWIW I think we should update the test runner in this repo to at least have some minimal amount of coverage support, so if you'd like to submit a PR for that I'd be happy to review it!

Does bazel coverage run the tests as well? Looking at the docs this looks the case

I will have to familiarize myself with the test runner. I would love to assist

[keith]

Yes it does, here are the docs https://docs.bazel.build/versions/master/command-line-reference.html#coverage

Here's a small PR with that update tinder-maxwellelliott/ios-bazel-test-with-coverage#5

But unrelated to that, by doing coverage correctly in the test runner instead you could avoid grabbing the binary from the bazel-bin as you have to there, also you wouldn't have to disable sandboxing, since the test runner would have access to everything it needed and can write files to $TEST_UNDECLARED_OUTPUTS_DIR that you can grab later.

Also depending on your use case you might actually want a LCOV_MERGER, your script is probably close to that.

[tinder-maxwellelliott]

Yes it does, here are the docs https://docs.bazel.build/versions/master/command-line-reference.html#coverage

Here's a small PR with that update tinder-maxwellelliott/ios-bazel-test-with-coverage#5

But unrelated to that, by doing coverage correctly in the test runner instead you could avoid grabbing the binary from the bazel-bin as you have to there, also you wouldn't have to disable sandboxing, since the test runner would have access to everything it needed and can write files to $TEST_UNDECLARED_OUTPUTS_DIR that you can grab later.

Also depending on your use case you might actually want a LCOV_MERGER, your script is probably close to that.

This is definitely the best option possible. Having everyone do this manually when we have everything we need already in the test runner is wasteful. I will read through it and see what I can accomplish

[tinder-maxwellelliott]

But unrelated to that, by doing coverage correctly in the test runner instead you could avoid grabbing the binary from the bazel-bin as you have to there, also you wouldn't have to disable sandboxing, since the test runner would have access to everything it needed and can write files to $TEST_UNDECLARED_OUTPUTS_DIR that you can grab later.

Where is the best place to start looking into this kind of an addition?

[keith]

I think the changes could be entirely localized to this shell script https://github.com/bazelbuild/rules_apple/blob/master/apple/testing/default_runner/ios_test_runner.template.sh

[tinder-maxwellelliott]

I think the changes could be entirely localized to this shell script https://github.com/bazelbuild/rules_apple/blob/master/apple/testing/default_runner/ios_test_runner.template.sh

Looking into this more. I can see that COVERAGE is set when running ios_test_runner.template.sh, I would guess if that is set then we should set $LLVM_PROFILE_FILE and $LCOV_MERGER variables to either defaults or passed in values. Then inside of XCTestRunner we need to perform our xcrun llvm-cov export commands on the built binaries

[keith]

LCOV_MERGER will have to be managed by users unfortunately. We currently use --test_env= LCOV_MERGER=/usr/bin/true to just ignore this all together.

LLVM_PROFILE_FILE may or may not have to be passed explicitly based on how this is done, since xcodebuild will pass it itself depending on the contents of your xctestrun file, but if you don't care about the format of xcresult bundles this probably is the easiest way (also since xctestrunner is harder to change)

[acecilia]

Just to link here all coverage related information, some work has been merged in Bazel here. Maybe we could see better coverage support with bazel 4

[keith]

Can folks who are interested test out #1223

You'll still need to pass --test_env=LCOV_MERGER=/usr/bin/true

But this generates a lcov file for consumption

[keith]

Note you also have to pass --experimental_use_llvm_covmap always (at least for now bazelbuild/bazel#13884)

[sgammon]

with bazelbuild/bazel#14724 it seems like progress is being made on this and may no longer require LCOV_MERGER=true (after some rules updates to adopt, presumably)

[wendyliga]

@keith could we use this improvement #1411 ? so we still are able to pass desired LLVM_PROFILE_FILE, so if we choose to handle the coverage manually