cmake: Add `Coverage` script #191

hebasto · 2024-05-06T23:26:37Z

Using separated scripts provides additional flexibility as there is no need to specify coverage report specific options during the build configuration stage.

Examples of usage on Ubuntu 24.04 with LCOV v2.0:

total coverage:

$ cmake -B build -DCMAKE_BUILD_TYPE=Coverage -DCMAKE_CXX_FLAGS="-fprofile-update=atomic"
$ cmake --build build
$ cmake -DLCOV_OPTS="--ignore-errors mismatch" -P build/Coverage.cmake
# OR
$ cmake \
    -DJOBS=$(nproc) \
    -DEXTENDED_FUNCTIONAL_TESTS=ON \
    -DLCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch" \
    -P build/Coverage.cmake

fuzz coverage:

$ cmake -B build -DCMAKE_BUILD_TYPE=Coverage -DCMAKE_CXX_FLAGS="-fprofile-update=atomic" -DBUILD_FUZZ_BINARY=ON
$ cmake --build build
$ cmake \
    -DFUZZ_SEED_CORPUS_DIR=<path/to/seed/corpus> \
    -DLCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch" \
    -P build/CoverageFuzz.cmake

This PR supports both LCOV v1 and LCOV v2. Hence, it is possible to test it on Ubuntu 24.04.

hebasto · 2024-05-08T13:40:20Z

Included changes from bitcoin#30063.

hebasto · 2024-05-09T11:01:38Z

@maflcko Want to tested this PR in your https://github.com/maflcko/b-c-cov?

vasild · 2024-05-13T12:04:06Z

cmake/script/Coverage.cmake

+execute_process(
+  COMMAND ${LCOV_FILTER_COMMAND} test_bitcoin.info test_bitcoin_filtered.info
+  WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
+)


This file consists mostly of such execute_process() calls. The most natural way to execute a bunch of commands is from a shell script. I would suggest to use a shell script for this. This 77 lines cmake file would be equivalent roughly to the following:

functional_test_runner="test/functional/test_runner.py" if [ -n "$EXTENDED_FUNCTIONAL_TESTS" ] ; then functional_test_runner="$functional_test_runner --extended" fi if [ -n "$JOBS" ] ; then CMAKE_CTEST_COMMAND="$CMAKE_CTEST_COMMAND -j $JOBS" functional_test_runner="$functional_test_runner -j $JOBS" fi cd $CMAKE_CURRENT_LIST_DIR $CMAKE_CTEST_COMMAND --build-config Coverage $LCOV_COMMAND --capture --directory src --test-name test_bitcoin --output-file test_bitcoin.info $LCOV_COMMAND --zerocounters --directory src $LCOV_FILTER_COMMAND test_bitcoin.info test_bitcoin_filtered.info $LCOV_COMMAND --add-tracefile test_bitcoin_filtered.info --output-file test_bitcoin_filtered.info $LCOV_COMMAND --add-tracefile baseline_filtered.info --add-tracefile test_bitcoin_filtered.info --output-file test_bitcoin_coverage.info $GENHTML_COMMAND test_bitcoin_coverage.info --output-directory test_bitcoin.coverage $functional_test_runner $LCOV_COMMAND --capture --directory src --test-name functional-tests --output-file functional_test.info $LCOV_COMMAND --zerocounters --directory src $LCOV_FILTER_COMMAND functional_test.info functional_test_filtered.info $LCOV_COMMAND --add-tracefile functional_test_filtered.info --output-file functional_test_filtered.info $LCOV_COMMAND --add-tracefile baseline_filtered.info --add-tracefile test_bitcoin_filtered.info --add-tracefile functional_test_filtered.info --output-file total_coverage.info | $GREP_EXECUTABLE "%" | $AWK_EXECUTABLE "{ print substr($3,2,50) \"/\" $5 }" > coverage_percent.txt $GENHTML_COMMAND total_coverage.info --output-directory total.coverage

The most natural way to execute a bunch of commands is from a shell script. I would suggest to use a shell script for this.

From CMake's point of view, using a platform independent approach is more natural :)

However, I have to admit that it is not expected that the user will run coverage scripts on Windows.

vasild · 2024-05-13T12:09:43Z

cmake/script/CoverageInclude.cmake.in

+if("@CMAKE_CXX_COMPILER_ID@" STREQUAL "Clang")
+  find_program(LLVM_COV_EXECUTABLE llvm-cov REQUIRED)
+  set(COV_TOOL "${LLVM_COV_EXECUTABLE} gcov")


Clang has a different native workflow: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html which I guess produces better results compared to its gcc compatibility layer.

Right. But this PR aims to mirror the master branch behaviour. Further improvements can be done later, no?

vasild · 2024-05-13T12:17:45Z

doc/developer-notes.md

-./configure --enable-lcov
-make
-make cov
+cmake -B build -DCMAKE_BUILD_TYPE=Coverage


I think with/without coverage should be independent option from debug/release. For example one may want to gather coverage on debug builds or release builds? I do not think we need a dedicated build type "Coverage" in order to pass a few extra flags which is already supported by the build system with APPEND_..._FLAGS.

For clang, that would be extra -fprofile-instr-generate -fcoverage-mapping and compiling and running the tests as usual. I guess for gcc that would be --coverage instead.

I know this would be different from how autotools works, but I would propose to have a shell script in contrib/ that compiles with coverage enabled (e.g. by running cmake -DAPPEND_..._FLAGS="-fprofile-instr-generate -fcoverage-mapping"), then runs the tests and gathers the coverage (actually 2 shell scripts - one for clang and one for gcc).

For example one may want to gather coverage on debug builds or release builds?

Is this something that is commonly done? What would the purpose of this be?

I know this would be different from how autotools works, but I would propose to have a shell script

Not sure about this. I prefer to work with the build system directly without having to understand a wrapper script first if I want to tweak something.

hebasto · 2024-05-25T14:45:06Z

Switched from the "Coverage" build type to the "coverage" preset. It improves flexibility and hopefully addresses @vasild's cmake: Add Coverage script #191 (comment).
Now supports both LCOV v1 and LCOV v2. Hence, it is possible to test on Ubuntu 24.04.

hebasto · 2024-05-25T14:59:02Z

Friendly ping code coverage connoisseurs @maflcko @dergoegge :)

Your opinion will be much appreciated by all people in the CMake working group.

fanquake · 2024-05-29T15:13:51Z

cmake/script/CoverageInclude.cmake.in

+  @ONLY
+)
+
+set(LCOV_OPTS --ignore mismatch)


I don't think this should be hardcoded here (it's not needed with lcov 1.x, and isn't the only ignore that might be required with 2.x)?

Thanks! Deleted.

cmake/script/CoverageInclude.cmake.in

hebasto · 2024-05-30T09:25:55Z

Addressed the recent @fanquake's comments.

The doc change has been aligned with bitcoin#30192.

dergoegge · 2024-05-30T10:36:11Z

doc/developer-notes.md

-./configure --enable-lcov
-make
-make cov
+cmake -B build --preset coverage


This is different from the examples in the PR description and also skips building the fuzz binary, is that expected?

The example in the PR description also does not build the fuzz binary:

Tests: test_bitcoin ........................ ON test_bitcoin-qt ..................... OFF bench_bitcoin ....................... OFF fuzz binary ......................... OFF

The PR description has been updated.

UPD. Added --ignore-errors mismatch

hebasto · 2024-05-31T17:11:08Z

Re-tested, fixed bugs.

Reverted back to use of the "Coverage" build type because this guarantees that the Coverage.cmake script finds unit tests when using multi-config generators.

The PR description has been updated.

fanquake · 2024-06-03T10:50:12Z

CMakeLists.txt

@@ -484,6 +484,22 @@ else()
  unset(c_flags_debug_overridden)
 endif()

+set(CMAKE_CXX_FLAGS_COVERAGE "-Og --coverage -fprofile-update=prefer-atomic")


fprofile-update=prefer-atomic

Not sure this should be in here? Don't think it's needed with Clang, and only sometimes with GCC?

Would it hurt if applied in such cases? Or do you want me to remove it?

Not sure this should be in here?

Deleted.

The PR description has been updated.

dergoegge · 2024-06-03T11:15:49Z

Writing data to fuzz_filtered.info
Summary coverage rate:
  lines......: 20.2% (19593 of 97038 lines)
  functions..: 20.4% (18829 of 92323 functions)
  branches...: no data found
Reading data file fuzz_coverage.info
Found 1190 entries.
Found common filename prefix "/home/dergoegge/workspace/bitcoin"
Writing .css and .png files.
Generating output.
genhtml: ERROR: cannot read /home/dergoegge/workspace/bitcoin/build/src/include/boost/visit_each.hpp
Processing file build/src/include/boost/visit_each.hpp

My lcov version is quite old (1.0), maybe that's why this fails. I've had to fiddle with the versions in the past and I'm sure this would work with the right versions etc. (everything seemed to work here besides the actual html generation).

I've switched to using llvm's native coverage reporting (i.e. compiling with -fprofile-instr-generate -fcoverage-mapping) which has been a breeze so far. I switched because I kept running into weird lcov problems like this and the gcov reports are also not the best (e.g. inline branch coverage is reported in a really weird format and I can't find any docs).

hebasto · 2024-06-05T14:39:05Z

Rebased.

maflcko · 2024-06-06T11:15:56Z

In the first OR in the pull request description in the second branch, you forgot to specify DCMAKE_CXX_FLAGS and the build type?

maflcko

lgtm

hebasto · 2024-06-06T11:19:06Z

In the first OR in the pull request description in the second branch, you forgot to specify DCMAKE_CXX_FLAGS and the build type?

This OR is related to the script invocation only:

$ cmake -DLCOV_OPTS="--ignore-errors mismatch" -P build/Coverage.cmake
# OR
$ cmake \
    -DJOBS=$(nproc) \
    -DEXTENDED_FUNCTIONAL_TESTS=ON \
    -DLCOV_OPTS="--rc branch_coverage=1 --ignore-errors mismatch" \
    -P build/Coverage.cmake

vasild · 2024-06-11T09:15:44Z

Further improvements, that would deviate from autotools:

Reconsider -Og because it has different meanings for GCC and Clang. Optimizations could result in bogus line numbers in the output, maybe just use -O0 -g.
Rename cmake/script/Coverage.cmake to GccCoverage.cmake because Clang uses a different workflow: https://clang.llvm.org/docs/SourceBasedCodeCoverage.html. Introduce ClangCoverage.cmake.
Drop the build target Coverage. Passing extra compiler flags is more suitable for a CMake option, rather than a build target. IMO that is very similar to -DWERROR=ON for example. Introduce -DCOVERAGE=OFF|GCC|CLANG.

hebasto added the enhancement New feature or request label May 7, 2024

hebasto force-pushed the 240506-cmake-EW branch from 62a97b7 to 2da8214 Compare May 7, 2024 23:21

hebasto added this to the UI, 23th May milestone May 8, 2024

hebasto force-pushed the 240506-cmake-EW branch 2 times, most recently from 5989b1f to fb5f30a Compare May 8, 2024 13:39

hebasto force-pushed the 240506-cmake-EW branch from fb5f30a to 739adea Compare May 8, 2024 15:31

vasild reviewed May 13, 2024

View reviewed changes

hebasto force-pushed the cmake-staging branch from 1d76056 to e8e0e69 Compare May 21, 2024 11:17

hebasto force-pushed the 240506-cmake-EW branch 2 times, most recently from b675988 to 8321299 Compare May 25, 2024 14:42

hebasto mentioned this pull request May 25, 2024

make cov fails with lcov-2 bitcoin/bitcoin#28468

Closed

1 task

hebasto mentioned this pull request May 29, 2024

build: remove --enable-lcov-branch-coverage bitcoin/bitcoin#30192

Merged

fanquake reviewed May 29, 2024

View reviewed changes

cmake/script/CoverageInclude.cmake.in Outdated Show resolved Hide resolved

hebasto force-pushed the 240506-cmake-EW branch from 8321299 to 003faac Compare May 30, 2024 09:25

dergoegge reviewed May 30, 2024

View reviewed changes

hebasto force-pushed the 240506-cmake-EW branch from 003faac to f89ade3 Compare May 31, 2024 17:04

fanquake reviewed Jun 3, 2024

View reviewed changes

hebasto force-pushed the cmake-staging branch from 0e6def0 to 597da83 Compare June 5, 2024 14:35

hebasto force-pushed the 240506-cmake-EW branch from f89ade3 to 129b0fe Compare June 5, 2024 14:38

cmake: Add Coverage and CoverageFuzz scripts

7669f29

hebasto force-pushed the 240506-cmake-EW branch from 129b0fe to 7669f29 Compare June 6, 2024 09:47

maflcko approved these changes Jun 6, 2024

View reviewed changes

hebasto merged commit 2677998 into cmake-staging Jun 8, 2024
36 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

cmake: Add `Coverage` script #191

cmake: Add `Coverage` script #191

hebasto commented May 6, 2024 •

edited

Loading

hebasto commented May 8, 2024

hebasto commented May 9, 2024

vasild May 13, 2024

hebasto May 16, 2024 •

edited

Loading

vasild May 13, 2024

hebasto May 16, 2024

vasild May 13, 2024

TheCharlatan May 24, 2024

hebasto commented May 25, 2024 •

edited

Loading

hebasto commented May 25, 2024

fanquake May 29, 2024

hebasto May 30, 2024

hebasto commented May 30, 2024 •

edited

Loading

dergoegge May 30, 2024

dergoegge May 30, 2024

hebasto May 30, 2024

hebasto May 30, 2024

hebasto commented May 31, 2024

fanquake Jun 3, 2024

hebasto Jun 3, 2024

hebasto Jun 6, 2024

dergoegge commented Jun 3, 2024

hebasto commented Jun 5, 2024

maflcko commented Jun 6, 2024

maflcko left a comment

hebasto commented Jun 6, 2024 •

edited

Loading

vasild commented Jun 11, 2024

cmake: Add Coverage script #191

cmake: Add Coverage script #191

Conversation

hebasto commented May 6, 2024 • edited Loading

hebasto commented May 8, 2024

hebasto commented May 9, 2024

Choose a reason for hiding this comment

hebasto May 16, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented May 25, 2024 • edited Loading

hebasto commented May 25, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented May 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hebasto commented May 31, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dergoegge commented Jun 3, 2024

hebasto commented Jun 5, 2024

maflcko commented Jun 6, 2024

maflcko left a comment

Choose a reason for hiding this comment

hebasto commented Jun 6, 2024 • edited Loading

vasild commented Jun 11, 2024

cmake: Add `Coverage` script #191

cmake: Add `Coverage` script #191

hebasto commented May 6, 2024 •

edited

Loading

hebasto May 16, 2024 •

edited

Loading

hebasto commented May 25, 2024 •

edited

Loading

hebasto commented May 30, 2024 •

edited

Loading

hebasto commented Jun 6, 2024 •

edited

Loading