youki icon indicating copy to clipboard operation
youki copied to clipboard

Discussion for addition of Code coverage in integration tests

Open YJDoc2 opened this issue 4 years ago • 6 comments

Currently we use cargo tests along with cargo-llvm-cov in CI to generate code coverage report. Even though it provides a good way to measure coverage, currently not all functions are tested, and in fact some are very hard to test.

Thus, should we think of using the OCI validation tests as well to generate the data? I asked the maintainer of cargo-llvm-cov if taking coverage report from an external command such as validation tests would be possible, and they stated that even though currently there is no direct way to do this, it should be still possible : https://github.com/taiki-e/cargo-llvm-cov/issues/93

Some things we will need to consider with this are :

  • to take coverage the code needs to be compiled on nightly , along with extra stuff that is needed to emit code coverage information
  • currently we use release version to validate, as that will be the final version to be used, so switching to instrumentation version needed for coverage may not be good.
  • running integration test once again for coverage separately could be a good idea, but currently on average the tests take about 5 minutes to run. This will introduce a separate job in CI of about 5 min duration.
  • How much difference will this actually make in code coverage report

What are your opinions @utam0k @Furisto @yihuaf ?

YJDoc2 avatar Sep 21 '21 06:09 YJDoc2

How about using debug builds for PR and other CI, and automatically running tests with release builds on the main branch once daily?

utam0k avatar Sep 21 '21 13:09 utam0k

How about using debug builds for PR and other CI, and automatically running tests with release builds on the main branch once daily?

I think this is reasonable. I think debug build for CI, validation, and coverage. I don't think we need to run coverage again for release build.

yihuaf avatar Sep 22 '21 03:09 yihuaf

Rust 1.60 introduced a new feature for test coverage. Can we use it?

utam0k avatar May 06 '22 00:05 utam0k

I think we can incorporate that, we might need an additional component to be installed in the CI grcov which converts the data generated to lcov format needed for coverage report. I'm looking into if we can directly generate lcov from cargo. I think the CI action that we use also have updated to the new feature, updating the action version in CI might be able to give us this as well.

YJDoc2 avatar May 06 '22 13:05 YJDoc2

Rust 1.60 introduced a new feature for test coverage. Can we use it?

cargo-llvm-cov (v0.2+) uses -C instrument-coverage that stabilized in 1.60. (v0.1 uses -Z instrument-coverage that is old version of -C instrument-coverage.

By the way, the feature mentioned in this issue has already been implemented as llvm show-env subcommand, It should be possible to use cargo-llvm-cov for integration testing. like:

source <(cargo llvm-cov show-env --export-prefix) # Set the environment variables needed to get coverage.
cargo llvm-cov clean --workspace # remove artifacts that may affect the coverage results
cargo build # build rust binaries
# commands using binaries in target/debug/*, including `cargo test`
# ...
cargo llvm-cov --no-run --lcov # generate report without tests

taiki-e avatar May 06 '22 18:05 taiki-e

Hey, thanks a lot for this! I'll try to update according to this and adding integration tests into code coverage, although we might need to build them separately to the youki, as we don't want to consider them into code coverage. Thanks :)

YJDoc2 avatar May 07 '22 07:05 YJDoc2