* [Tarantool-patches] [PATCH] cmake: add code coverage support @ 2023-07-25 13:52 Sergey Bronnikov via Tarantool-patches 2023-07-26 15:02 ` Sergey Kaplun via Tarantool-patches 2023-08-08 8:16 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 2 replies; 10+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2023-07-25 13:52 UTC (permalink / raw) To: tarantool-patches, Sergey Kaplun, max.kokryashkin From: Sergey Bronnikov <sergeyb@tarantool.org> The patch adds building code coverage report using gcovr [1] and gcov. gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage executes regression tests, proccess *.gcno and *.gcda files with gcov, builds detailed HTML report and prints summary about code coverage. ``` $ cmake -S . -B build -DENABLE_COVERAGE=ON $ cmake --build build --parallel $ cmake --build build --target LuaJIT-coverage <snipped> lines: 84.1% (26056 out of 30997) functions: 88.8% (2055 out of 2314) branches: 71.5% (14801 out of 20703) ``` 1. https://gcovr.com/ 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr --- CMakeLists.txt | 9 ++++++ test/CMakeLists.txt | 40 +++++++++++++++++++++++++++ test/tarantool-c-tests/CMakeLists.txt | 2 +- 3 files changed, 50 insertions(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 6ef24bba..8bc63b90 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) ) endif() +set(ENABLE_COVERAGE_DEFAULT OFF) +option(ENABLE_COVERAGE "Enable integration with gcovr, a code coverage program" ${ENABLE_COVERAGE_DEFAULT}) +if (ENABLE_COVERAGE) + AppendFlags(CMAKE_C_FLAGS + -fprofile-arcs + -ftest-coverage + ) +endif() + # Auxiliary flags for main targets (libraries, binaries). AppendFlags(TARGET_C_FLAGS -D_FILE_OFFSET_BITS=64 diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index 47296a22..9b0f11d8 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -59,6 +59,44 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS tarantool-tests ) +find_program(GCOVR gcovr) +find_program(GCOV gcov) +set(COVERAGE_HTML_REPORT "luajit-coverage.html") +set(COVERAGE_JSON_REPORT "luajit-coverage.json") +if(ENABLE_COVERAGE) + if(NOT GCOVR OR NOT GCOV) + add_custom_target(${PROJECT_NAME}-coverage) + add_custom_command(TARGET ${PROJECT_NAME}-coverage + COMMENT "Either `gcovr' or `gcov` not found, so ${PROJECT_NAME}-coverage target is dummy" + ) + return() + endif() + + add_custom_target(${PROJECT_NAME}-coverage) + add_custom_command(TARGET ${PROJECT_NAME}-coverage + COMMENT "Building coverage report" + COMMAND + ${GCOVR} + # See https://gcovr.com/en/stable/guide/configuration.html + --root ${PROJECT_SOURCE_DIR} + --filter ${PROJECT_SOURCE_DIR}/src + --print-summary + --output ${COVERAGE_HTML_REPORT} + --coveralls ${COVERAGE_JSON_REPORT} + --html + --html-title "LuaJIT Code Coverage Report" + --html-details + --sort-percentage + --branches + --decisions + --verbose + -j ${CMAKE_BUILD_PARALLEL_LEVEL} + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} + ) + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") +endif(ENABLE_COVERAGE) + if(LUAJIT_USE_TEST) if(POLICY CMP0037) if(CMAKE_VERSION VERSION_LESS 3.11) @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) ${PROJECT_NAME}-test ${PROJECT_NAME}-luacheck ) + + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) endif() diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt index 17255345..8cd76b44 100644 --- a/test/tarantool-c-tests/CMakeLists.txt +++ b/test/tarantool-c-tests/CMakeLists.txt @@ -45,7 +45,7 @@ foreach(test_source ${tests}) OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" ) - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY}) + target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) LIST(APPEND TESTS_COMPILED ${exe}) endforeach() -- 2.34.1 ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-07-25 13:52 [Tarantool-patches] [PATCH] cmake: add code coverage support Sergey Bronnikov via Tarantool-patches @ 2023-07-26 15:02 ` Sergey Kaplun via Tarantool-patches 2023-07-27 8:42 ` Sergey Bronnikov via Tarantool-patches 2023-08-08 8:16 ` Sergey Bronnikov via Tarantool-patches 1 sibling, 1 reply; 10+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2023-07-26 15:02 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: max.kokryashkin, tarantool-patches Hi, Sergey! Thanks for the patch! It's very interesting to see coverage statistics of the LuaJIT by our tests: have much more work to do! :) Also, it's interesting to compare test suites with each other (done with lcov, sorted alphabetically): | lua-Harness-tests: | lines......: 54.2% (13550 of 25023 lines) | functions..: 62.4% (1103 of 1769 functions) | branches...: 37.3% (8293 of 22222 branches) | LuaJIT-tests: | lines......: 71.3% (17846 of 25023 lines) | functions..: 74.1% (1310 of 1769 functions) | branches...: 54.7% (12155 of 22222 branches) | PUC-Rio-Lua-5.1-tests: | lines......: 51.2% (12800 of 25023 lines) | functions..: 57.9% (1025 of 1769 functions) | branches...: 36.0% (7997 of 22222 branches) | tarantool-c-tests: | lines......: 33.3% (8210 of 24676 lines) | functions..: 38.6% (672 of 1740 functions) | branches...: 20.4% (4490 of 22014 branches) | tarantool-tests: | lines......: 69.6% (17405 of 25023 lines) | functions..: 76.7% (1357 of 1769 functions) | branches...: 49.9% (11090 of 22222 branches) | Overall: | lines......: 86.0% (21509 of 25023 lines) | functions..: 90.8% (1606 of 1769 functions) | branches...: 66.7% (14812 of 22222 branches) Please, consider my comments below. On 25.07.23, Sergey Bronnikov wrote: > From: Sergey Bronnikov <sergeyb@tarantool.org> > > The patch adds building code coverage report using gcovr [1] and gcov. > gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage Do we really need any features provided by gcovr that are omitted in lcov? It's not a problem for me, but there are many more dependencies for gcovr than for lcov. Also, it uses python that isn't such available (I mean installed by default) as perl. | $ equery depgraph dev-util/gcovr | ... | * dependency graph for dev-util/gcovr-6.0 | `-- dev-util/gcovr-6.0 [~amd64 keyword] | `-- dev-python/jinja-3.1.2 (dev-python/jinja) amd64 | `-- dev-python/lxml-4.9.2 (dev-python/lxml) amd64 | `-- dev-python/pygments-2.15.1 (dev-python/pygments) amd64 | `-- dev-python/yaxmldiff-0.1.0 (dev-python/yaxmldiff) amd64 | `-- dev-lang/python-3.11.4 (dev-lang/python) amd64 | `-- dev-python/pytest-timeout-2.1.0 (dev-python/pytest-timeout) amd64 | `-- dev-python/pytest-7.3.2 (>=dev-python/pytest-7.3.1) amd64 | `-- dev-python/gpep517-13 (>=dev-python/gpep517-13) amd64 | `-- dev-python/setuptools-67.8.0-r1 (>=dev-python/setuptools-67.8.0-r1) amd64 | [ dev-util/gcovr-6.0 stats: packages (10), max depth (1) ] | $ equery depgraph dev-util/lcov | ... | * dependency graph for dev-util/lcov-1.15 | `-- dev-util/lcov-1.15 ~amd64 | `-- dev-lang/perl-5.38.0-r1 (dev-lang/perl) ~amd64 | `-- dev-perl/JSON-4.100.0 (dev-perl/JSON) ~amd64 | `-- dev-perl/PerlIO-gzip-0.200.0-r1 (dev-perl/PerlIO-gzip) amd64 | [ dev-util/lcov-1.15 stats: packages (4), max depth (1) ] If we don't really need "decisions" metrics that are provided by default by gcovr I suggest to consider to use lcov instead, since I don't see any other advantages. > executes regression tests, proccess *.gcno and *.gcda files with gcov, > builds detailed HTML report and prints summary about code coverage. > > ``` > $ cmake -S . -B build -DENABLE_COVERAGE=ON I suggest to use -DLUAJIT_ENABLE_COVERAGE=ON to be consistent with other options. > $ cmake --build build --parallel > $ cmake --build build --target LuaJIT-coverage > > <snipped> > > lines: 84.1% (26056 out of 30997) > functions: 88.8% (2055 out of 2314) > branches: 71.5% (14801 out of 20703) > ``` > > 1. https://gcovr.com/ > 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr > --- > CMakeLists.txt | 9 ++++++ > test/CMakeLists.txt | 40 +++++++++++++++++++++++++++ > test/tarantool-c-tests/CMakeLists.txt | 2 +- > 3 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 6ef24bba..8bc63b90 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) > ) > endif() > > +set(ENABLE_COVERAGE_DEFAULT OFF) > +option(ENABLE_COVERAGE "Enable integration with gcovr, a code coverage program" ${ENABLE_COVERAGE_DEFAULT}) Minor: Please split the line to the several, to avoid too long lines. > +if (ENABLE_COVERAGE) > + AppendFlags(CMAKE_C_FLAGS > + -fprofile-arcs > + -ftest-coverage Should we use just --coverage? | --coverage | This option is used to compile and link code instrumented | for coverage analysis. The option is a synonym for | -fprofile-arcs -ftest-coverage (when compiling) and -lgcov | (when linking). See man gcc(1) for details. > + ) > +endif() > + > # Auxiliary flags for main targets (libraries, binaries). > AppendFlags(TARGET_C_FLAGS > -D_FILE_OFFSET_BITS=64 > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > index 47296a22..9b0f11d8 100644 > --- a/test/CMakeLists.txt > +++ b/test/CMakeLists.txt > @@ -59,6 +59,44 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS > tarantool-tests > ) > > +find_program(GCOVR gcovr) > +find_program(GCOV gcov) > +set(COVERAGE_HTML_REPORT "luajit-coverage.html") > +set(COVERAGE_JSON_REPORT "luajit-coverage.json") Should we add ${PROJECT_BUILD_DIR} prefix to be more graceful for out-of-source build? Also, I see tons of html helper files in the working directory. Is there any way to use some <coverage/> directory for all this stuff instead? > +if(ENABLE_COVERAGE) > + if(NOT GCOVR OR NOT GCOV) > + add_custom_target(${PROJECT_NAME}-coverage) > + add_custom_command(TARGET ${PROJECT_NAME}-coverage > + COMMENT "Either `gcovr' or `gcov` not found, so ${PROJECT_NAME}-coverage target is dummy" Minor: Please split the line to the several, to avoid too long lines. > + ) > + return() I suppose, we should create dummy target instead. | cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DLUAJIT_ENABLE_GC64=OFF -DENABLE_COVERAGE=OFF && make -j | -- [SetVersion] Reading version from VCS: v2.1.0-beta3-331-g4a26e6a1 | -- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 4 | CMake Error at test/CMakeLists.txt:118 (add_dependencies): | Cannot add target-level dependencies to non-existent target | "LuaJIT-coverage". > + endif() > + > + add_custom_target(${PROJECT_NAME}-coverage) > + add_custom_command(TARGET ${PROJECT_NAME}-coverage > + COMMENT "Building coverage report" > + COMMAND > + ${GCOVR} > + # See https://gcovr.com/en/stable/guide/configuration.html > + --root ${PROJECT_SOURCE_DIR} > + --filter ${PROJECT_SOURCE_DIR}/src I suppose some files like *.dasc files or buildvm sources should be excluded too, since they are not informative. > + --print-summary > + --output ${COVERAGE_HTML_REPORT} > + --coveralls ${COVERAGE_JSON_REPORT} I see also output > + --html > + --html-title "LuaJIT Code Coverage Report" > + --html-details > + --sort-percentage I can't see an option for decreasing percentage of uncovered lines? Is its default value? If it is, then it should be used (as far as we want to see uncovered cases first). > + --branches > + --decisions Do we need this option? AFAICS, this is mostly about Functional Safety applications (road vehicles, etc.). > + --verbose I suppose that we should use --verbose option only when VERBOSE=1 env var is set. Also, I'm suprised by lines like this: | (DEBUG) Parsing coverage data for file /home/burii/reviews/luajit/gcov/src/lj_tab.h | (DEBUG) Starting the decision analysis | (DEBUG) Decision Analysis finished! | (DEBUG) Finding source file corresponding to a gcov data file Is it the part of the --verbose options? > + -j ${CMAKE_BUILD_PARALLEL_LEVEL} > + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} > + ) > + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") > + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") > +endif(ENABLE_COVERAGE) > + > if(LUAJIT_USE_TEST) > if(POLICY CMP0037) > if(CMAKE_VERSION VERSION_LESS 3.11) > @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) > ${PROJECT_NAME}-test > ${PROJECT_NAME}-luacheck > ) > + > + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) Is it necessary to rerun all tests to generate this analisys every time? > endif() > diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt > index 17255345..8cd76b44 100644 > --- a/test/tarantool-c-tests/CMakeLists.txt > +++ b/test/tarantool-c-tests/CMakeLists.txt > @@ -45,7 +45,7 @@ foreach(test_source ${tests}) > OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" > RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" > ) > - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY}) > + target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) Why is it done unconditionally? > LIST(APPEND TESTS_COMPILED ${exe}) > endforeach() > > -- > 2.34.1 > -- Best regards, Sergey Kaplun ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-07-26 15:02 ` Sergey Kaplun via Tarantool-patches @ 2023-07-27 8:42 ` Sergey Bronnikov via Tarantool-patches 2023-07-30 12:55 ` Maxim Kokryashkin via Tarantool-patches 2023-07-30 13:34 ` Maxim Kokryashkin via Tarantool-patches 0 siblings, 2 replies; 10+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2023-07-27 8:42 UTC (permalink / raw) To: Sergey Kaplun, Sergey Bronnikov; +Cc: tarantool-patches, max.kokryashkin Hello, Sergey! Thanks for review! On 7/26/23 18:02, Sergey Kaplun wrote: > Hi, Sergey! > Thanks for the patch! > It's very interesting to see coverage statistics of the LuaJIT by our > tests: have much more work to do! :) > > Also, it's interesting to compare test suites with each other (done with > lcov, sorted alphabetically): > | lua-Harness-tests: > | lines......: 54.2% (13550 of 25023 lines) > | functions..: 62.4% (1103 of 1769 functions) > | branches...: 37.3% (8293 of 22222 branches) > | LuaJIT-tests: > | lines......: 71.3% (17846 of 25023 lines) > | functions..: 74.1% (1310 of 1769 functions) > | branches...: 54.7% (12155 of 22222 branches) > | PUC-Rio-Lua-5.1-tests: > | lines......: 51.2% (12800 of 25023 lines) > | functions..: 57.9% (1025 of 1769 functions) > | branches...: 36.0% (7997 of 22222 branches) > | tarantool-c-tests: > | lines......: 33.3% (8210 of 24676 lines) > | functions..: 38.6% (672 of 1740 functions) > | branches...: 20.4% (4490 of 22014 branches) > | tarantool-tests: > | lines......: 69.6% (17405 of 25023 lines) > | functions..: 76.7% (1357 of 1769 functions) > | branches...: 49.9% (11090 of 22222 branches) > > | Overall: > | lines......: 86.0% (21509 of 25023 lines) > | functions..: 90.8% (1606 of 1769 functions) > | branches...: 66.7% (14812 of 22222 branches) > > Please, consider my comments below. It is interesting, thanks for measuring it! > > On 25.07.23, Sergey Bronnikov wrote: >> From: Sergey Bronnikov <sergeyb@tarantool.org> >> >> The patch adds building code coverage report using gcovr [1] and gcov. >> gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage > Do we really need any features provided by gcovr that are omitted in > lcov? It's not a problem for me, but there are many more dependencies > for gcovr than for lcov. Also, it uses python that isn't such > available (I mean installed by default) as perl. gcovr has exactly three dependencies, see [1]. Other dependencies specified for gcovr ebuild are "test" dependencies, required for those who will make patches and run tests for gcovr. I don't get why ebuild maintainer made these dependencies mandatory. 1. https://github.com/gcovr/gcovr/blob/27e6b3aaec7c91f54fa8e7531f8d652bf6aeadd4/setup.py#L54 BTW lcov has a bigger number of dependencies than specified in ebuild (or these dependencies are included in a Perl distribution, see [2]: - Capture::Tiny - DateTime - Devel::Cover - Digest::MD5 - File::Spec - at least one flavor of JSON module. In order of performance/preference: - JSON::XS - Cpanel::JSON::XS - JSON::PP - JSON - Memory::Process - Time::HiRes 2. https://github.com/linux-test-project/lcov > > | $ equery depgraph dev-util/gcovr > | ... > | * dependency graph for dev-util/gcovr-6.0 > | `-- dev-util/gcovr-6.0 [~amd64 keyword] > | `-- dev-python/jinja-3.1.2 (dev-python/jinja) amd64 > | `-- dev-python/lxml-4.9.2 (dev-python/lxml) amd64 > | `-- dev-python/pygments-2.15.1 (dev-python/pygments) amd64 > | `-- dev-python/yaxmldiff-0.1.0 (dev-python/yaxmldiff) amd64 > | `-- dev-lang/python-3.11.4 (dev-lang/python) amd64 > | `-- dev-python/pytest-timeout-2.1.0 (dev-python/pytest-timeout) amd64 > | `-- dev-python/pytest-7.3.2 (>=dev-python/pytest-7.3.1) amd64 > | `-- dev-python/gpep517-13 (>=dev-python/gpep517-13) amd64 > | `-- dev-python/setuptools-67.8.0-r1 (>=dev-python/setuptools-67.8.0-r1) amd64 > | [ dev-util/gcovr-6.0 stats: packages (10), max depth (1) ] > | $ equery depgraph dev-util/lcov > | ... > | * dependency graph for dev-util/lcov-1.15 > | `-- dev-util/lcov-1.15 ~amd64 > | `-- dev-lang/perl-5.38.0-r1 (dev-lang/perl) ~amd64 > | `-- dev-perl/JSON-4.100.0 (dev-perl/JSON) ~amd64 > | `-- dev-perl/PerlIO-gzip-0.200.0-r1 (dev-perl/PerlIO-gzip) amd64 > | [ dev-util/lcov-1.15 stats: packages (4), max depth (1) ] > > If we don't really need "decisions" metrics that are provided by default > by gcovr I suggest to consider to use lcov instead, since I don't see > any other advantages. > >> executes regression tests, proccess *.gcno and *.gcda files with gcov, >> builds detailed HTML report and prints summary about code coverage. >> >> ``` >> $ cmake -S . -B build -DENABLE_COVERAGE=ON > I suggest to use -DLUAJIT_ENABLE_COVERAGE=ON to be consistent with other > options. I don't mind, renamed to LUAJIT_ENABLE_COVERAGE. > >> $ cmake --build build --parallel >> $ cmake --build build --target LuaJIT-coverage >> >> <snipped> >> >> lines: 84.1% (26056 out of 30997) >> functions: 88.8% (2055 out of 2314) >> branches: 71.5% (14801 out of 20703) >> ``` >> >> 1. https://gcovr.com/ >> 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr >> --- >> CMakeLists.txt | 9 ++++++ >> test/CMakeLists.txt | 40 +++++++++++++++++++++++++++ >> test/tarantool-c-tests/CMakeLists.txt | 2 +- >> 3 files changed, 50 insertions(+), 1 deletion(-) >> >> diff --git a/CMakeLists.txt b/CMakeLists.txt >> index 6ef24bba..8bc63b90 100644 >> --- a/CMakeLists.txt >> +++ b/CMakeLists.txt >> @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) >> ) >> endif() >> >> +set(ENABLE_COVERAGE_DEFAULT OFF) >> +option(ENABLE_COVERAGE "Enable integration with gcovr, a code coverage program" ${ENABLE_COVERAGE_DEFAULT}) > Minor: Please split the line to the several, to avoid too long lines. Fixed. > >> +if (ENABLE_COVERAGE) >> + AppendFlags(CMAKE_C_FLAGS >> + -fprofile-arcs >> + -ftest-coverage > Should we use just --coverage? Sure, updated. > > | --coverage > | This option is used to compile and link code instrumented > | for coverage analysis. The option is a synonym for > | -fprofile-arcs -ftest-coverage (when compiling) and -lgcov > | (when linking). > > See man gcc(1) for details. > >> + ) >> +endif() >> + >> # Auxiliary flags for main targets (libraries, binaries). >> AppendFlags(TARGET_C_FLAGS >> -D_FILE_OFFSET_BITS=64 >> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt >> index 47296a22..9b0f11d8 100644 >> --- a/test/CMakeLists.txt >> +++ b/test/CMakeLists.txt >> @@ -59,6 +59,44 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS >> tarantool-tests >> ) >> >> +find_program(GCOVR gcovr) >> +find_program(GCOV gcov) >> +set(COVERAGE_HTML_REPORT "luajit-coverage.html") >> +set(COVERAGE_JSON_REPORT "luajit-coverage.json") > Should we add ${PROJECT_BUILD_DIR} prefix to be more graceful for > out-of-source build? > Also, I see tons of html helper files in the working directory. Is there > any way to use some <coverage/> directory for all this stuff instead? Updated: --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -61,9 +61,10 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS find_program(GCOVR gcovr) find_program(GCOV gcov) -set(COVERAGE_HTML_REPORT "luajit-coverage.html") -set(COVERAGE_JSON_REPORT "luajit-coverage.json") -if(ENABLE_COVERAGE) +set(COVERAGE_DIR "${PROJECT_BINARY_DIR}/coverage") +set(COVERAGE_HTML_REPORT "${COVERAGE_DIR}/luajit.html") +set(COVERAGE_JSON_REPORT "${COVERAGE_DIR}/luajit.json") > >> +if(ENABLE_COVERAGE) >> + if(NOT GCOVR OR NOT GCOV) >> + add_custom_target(${PROJECT_NAME}-coverage) >> + add_custom_command(TARGET ${PROJECT_NAME}-coverage >> + COMMENT "Either `gcovr' or `gcov` not found, so ${PROJECT_NAME}-coverage target is dummy" > Minor: Please split the line to the several, to avoid too long lines. Done. > >> + ) >> + return() > I suppose, we should create dummy target instead. > | cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DLUAJIT_ENABLE_GC64=OFF -DENABLE_COVERAGE=OFF && make -j > | -- [SetVersion] Reading version from VCS: v2.1.0-beta3-331-g4a26e6a1 > | -- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 4 > | CMake Error at test/CMakeLists.txt:118 (add_dependencies): > | Cannot add target-level dependencies to non-existent target > | "LuaJIT-coverage". > >> + endif() >> + >> + add_custom_target(${PROJECT_NAME}-coverage) >> + add_custom_command(TARGET ${PROJECT_NAME}-coverage >> + COMMENT "Building coverage report" >> + COMMAND >> + ${GCOVR} >> + # See https://gcovr.com/en/stable/guide/configuration.html >> + --root ${PROJECT_SOURCE_DIR} >> + --filter ${PROJECT_SOURCE_DIR}/src > I suppose some files like *.dasc files or buildvm sources should be > excluded too, since they are not informative. Done. >> + --print-summary >> + --output ${COVERAGE_HTML_REPORT} >> + --coveralls ${COVERAGE_JSON_REPORT} > I see also output > >> + --html >> + --html-title "LuaJIT Code Coverage Report" >> + --html-details >> + --sort-percentage > I can't see an option for decreasing percentage of uncovered lines? > Is its default value? If it is, then it should be used (as far as we want > to see uncovered cases first). Seems it can sort in that order only. > >> + --branches >> + --decisions > Do we need this option? AFAICS, this is mostly about Functional Safety > applications (road vehicles, etc.). I would leave it. > >> + --verbose > I suppose that we should use --verbose option only when VERBOSE=1 env > var is set. > > Also, I'm suprised by lines like this: > | (DEBUG) Parsing coverage data for file /home/burii/reviews/luajit/gcov/src/lj_tab.h > | (DEBUG) Starting the decision analysis > | (DEBUG) Decision Analysis finished! > | (DEBUG) Finding source file corresponding to a gcov data file > Is it the part of the --verbose options? Right, it is enabled only when --verbose is passed. Removed option --verbose at all, I believe we don't need it in normal use. > >> + -j ${CMAKE_BUILD_PARALLEL_LEVEL} >> + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} >> + ) >> + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") >> + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") >> +endif(ENABLE_COVERAGE) >> + >> if(LUAJIT_USE_TEST) >> if(POLICY CMP0037) >> if(CMAKE_VERSION VERSION_LESS 3.11) >> @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) >> ${PROJECT_NAME}-test >> ${PROJECT_NAME}-luacheck >> ) >> + >> + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) > Is it necessary to rerun all tests to generate this analisys every > time? What could be a reasons to regenerate coverage report without rerunning tests? I can make a target LuaJIT-coverage that will generate report and add target coverage that will run tests and then call LuaJIT-coverage. What do you think? > >> endif() >> diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt >> index 17255345..8cd76b44 100644 >> --- a/test/tarantool-c-tests/CMakeLists.txt >> +++ b/test/tarantool-c-tests/CMakeLists.txt >> @@ -45,7 +45,7 @@ foreach(test_source ${tests}) >> OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" >> RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" >> ) >> - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY}) >> + target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) > Why is it done unconditionally? Ah, missed it. Thanks! --- a/test/tarantool-c-tests/CMakeLists.txt +++ b/test/tarantool-c-tests/CMakeLists.txt @@ -45,7 +45,11 @@ foreach(test_source ${tests}) OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" ) - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) + set(libtest-libs libtest ${LUAJIT_LIBRARY}) + if (LUAJIT_ENABLE_COVERAGE) + set(libtest-libs ${libtest-libs} --coverage) + endif (LUAJIT_ENABLE_COVERAGE) + target_link_libraries(${exe} ${libtest-libs}) LIST(APPEND TESTS_COMPILED ${exe}) endforeach() > >> LIST(APPEND TESTS_COMPILED ${exe}) >> endforeach() >> >> -- >> 2.34.1 >> ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-07-27 8:42 ` Sergey Bronnikov via Tarantool-patches @ 2023-07-30 12:55 ` Maxim Kokryashkin via Tarantool-patches 2023-07-30 13:34 ` Maxim Kokryashkin via Tarantool-patches 1 sibling, 0 replies; 10+ messages in thread From: Maxim Kokryashkin via Tarantool-patches @ 2023-07-30 12:55 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: max.kokryashkin, tarantool-patches, Sergey Bronnikov [-- Attachment #1: Type: text/plain, Size: 12786 bytes --] Hi! Just want to add a bit to the Perl vs Python topic. Python comes pre-installed on most of the Linux distributions, including RHEL, CentOS, Fedora, Debian, Ubuntu and other less popular. It is also true for the modern OSX versions. So, unless you really want to run coverage on something like FreeBSD, then there is virtually no difference whatsoever. -- Best regards, Maxim Kokryashkin > >>Hello, Sergey! Thanks for review! >> >> >>On 7/26/23 18:02, Sergey Kaplun wrote: >>> Hi, Sergey! >>> Thanks for the patch! >>> It's very interesting to see coverage statistics of the LuaJIT by our >>> tests: have much more work to do! :) >>> >>> Also, it's interesting to compare test suites with each other (done with >>> lcov, sorted alphabetically): >>> | lua-Harness-tests: >>> | lines......: 54.2% (13550 of 25023 lines) >>> | functions..: 62.4% (1103 of 1769 functions) >>> | branches...: 37.3% (8293 of 22222 branches) >>> | LuaJIT-tests: >>> | lines......: 71.3% (17846 of 25023 lines) >>> | functions..: 74.1% (1310 of 1769 functions) >>> | branches...: 54.7% (12155 of 22222 branches) >>> | PUC-Rio-Lua-5.1-tests: >>> | lines......: 51.2% (12800 of 25023 lines) >>> | functions..: 57.9% (1025 of 1769 functions) >>> | branches...: 36.0% (7997 of 22222 branches) >>> | tarantool-c-tests: >>> | lines......: 33.3% (8210 of 24676 lines) >>> | functions..: 38.6% (672 of 1740 functions) >>> | branches...: 20.4% (4490 of 22014 branches) >>> | tarantool-tests: >>> | lines......: 69.6% (17405 of 25023 lines) >>> | functions..: 76.7% (1357 of 1769 functions) >>> | branches...: 49.9% (11090 of 22222 branches) >>> >>> | Overall: >>> | lines......: 86.0% (21509 of 25023 lines) >>> | functions..: 90.8% (1606 of 1769 functions) >>> | branches...: 66.7% (14812 of 22222 branches) >>> >>> Please, consider my comments below. >>It is interesting, thanks for measuring it! >>> >>> On 25.07.23, Sergey Bronnikov wrote: >>>> From: Sergey Bronnikov < sergeyb@tarantool.org > >>>> >>>> The patch adds building code coverage report using gcovr [1] and gcov. >>>> gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage >>> Do we really need any features provided by gcovr that are omitted in >>> lcov? It's not a problem for me, but there are many more dependencies >>> for gcovr than for lcov. Also, it uses python that isn't such >>> available (I mean installed by default) as perl. >> >>gcovr has exactly three dependencies, see [1]. >> >>Other dependencies specified for gcovr ebuild are "test" dependencies, >>required for >> >>those who will make patches and run tests for gcovr. I don't get why >>ebuild maintainer >> >>made these dependencies mandatory. >> >>1. >>https://github.com/gcovr/gcovr/blob/27e6b3aaec7c91f54fa8e7531f8d652bf6aeadd4/setup.py#L54 >> >> >>BTW lcov has a bigger number of dependencies than specified >> >>in ebuild (or these dependencies are included in a Perl distribution, >>see [2]: >> >> >> - Capture::Tiny >> - DateTime >> - Devel::Cover >> - Digest::MD5 >> - File::Spec >> - at least one flavor of JSON module. >> In order of performance/preference: >> - JSON::XS >> - Cpanel::JSON::XS >> - JSON::PP >> - JSON >> - Memory::Process >> - Time::HiRes >> >> >>2. https://github.com/linux-test-project/lcov >> >> >>> >>> | $ equery depgraph dev-util/gcovr >>> | ... >>> | * dependency graph for dev-util/gcovr-6.0 >>> | `-- dev-util/gcovr-6.0 [~amd64 keyword] >>> | `-- dev-python/jinja-3.1.2 (dev-python/jinja) amd64 >>> | `-- dev-python/lxml-4.9.2 (dev-python/lxml) amd64 >>> | `-- dev-python/pygments-2.15.1 (dev-python/pygments) amd64 >>> | `-- dev-python/yaxmldiff-0.1.0 (dev-python/yaxmldiff) amd64 >>> | `-- dev-lang/python-3.11.4 (dev-lang/python) amd64 >>> | `-- dev-python/pytest-timeout-2.1.0 (dev-python/pytest-timeout) amd64 >>> | `-- dev-python/pytest-7.3.2 (>=dev-python/pytest-7.3.1) amd64 >>> | `-- dev-python/gpep517-13 (>=dev-python/gpep517-13) amd64 >>> | `-- dev-python/setuptools-67.8.0-r1 (>=dev-python/setuptools-67.8.0-r1) amd64 >>> | [ dev-util/gcovr-6.0 stats: packages (10), max depth (1) ] >>> | $ equery depgraph dev-util/lcov >>> | ... >>> | * dependency graph for dev-util/lcov-1.15 >>> | `-- dev-util/lcov-1.15 ~amd64 >>> | `-- dev-lang/perl-5.38.0-r1 (dev-lang/perl) ~amd64 >>> | `-- dev-perl/JSON-4.100.0 (dev-perl/JSON) ~amd64 >>> | `-- dev-perl/PerlIO-gzip-0.200.0-r1 (dev-perl/PerlIO-gzip) amd64 >>> | [ dev-util/lcov-1.15 stats: packages (4), max depth (1) ] >>> >>> If we don't really need "decisions" metrics that are provided by default >>> by gcovr I suggest to consider to use lcov instead, since I don't see >>> any other advantages. >>> >>>> executes regression tests, proccess *.gcno and *.gcda files with gcov, >>>> builds detailed HTML report and prints summary about code coverage. >>>> >>>> ``` >>>> $ cmake -S . -B build -DENABLE_COVERAGE=ON >>> I suggest to use -DLUAJIT_ENABLE_COVERAGE=ON to be consistent with other >>> options. >>I don't mind, renamed to LUAJIT_ENABLE_COVERAGE. >>> >>>> $ cmake --build build --parallel >>>> $ cmake --build build --target LuaJIT-coverage >>>> >>>> <snipped> >>>> >>>> lines: 84.1% (26056 out of 30997) >>>> functions: 88.8% (2055 out of 2314) >>>> branches: 71.5% (14801 out of 20703) >>>> ``` >>>> >>>> 1. https://gcovr.com/ >>>> 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr >>>> --- >>>> CMakeLists.txt | 9 ++++++ >>>> test/CMakeLists.txt | 40 +++++++++++++++++++++++++++ >>>> test/tarantool-c-tests/CMakeLists.txt | 2 +- >>>> 3 files changed, 50 insertions(+), 1 deletion(-) >>>> >>>> diff --git a/CMakeLists.txt b/CMakeLists.txt >>>> index 6ef24bba..8bc63b90 100644 >>>> --- a/CMakeLists.txt >>>> +++ b/CMakeLists.txt >>>> @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) >>>> ) >>>> endif() >>>> >>>> +set(ENABLE_COVERAGE_DEFAULT OFF) >>>> +option(ENABLE_COVERAGE "Enable integration with gcovr, a code coverage program" ${ENABLE_COVERAGE_DEFAULT}) >>> Minor: Please split the line to the several, to avoid too long lines. >> >>Fixed. >> >> >>> >>>> +if (ENABLE_COVERAGE) >>>> + AppendFlags(CMAKE_C_FLAGS >>>> + -fprofile-arcs >>>> + -ftest-coverage >>> Should we use just --coverage? >>Sure, updated. >>> >>> | --coverage >>> | This option is used to compile and link code instrumented >>> | for coverage analysis. The option is a synonym for >>> | -fprofile-arcs -ftest-coverage (when compiling) and -lgcov >>> | (when linking). >>> >>> See man gcc(1) for details. >>> >>>> + ) >>>> +endif() >>>> + >>>> # Auxiliary flags for main targets (libraries, binaries). >>>> AppendFlags(TARGET_C_FLAGS >>>> -D_FILE_OFFSET_BITS=64 >>>> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt >>>> index 47296a22..9b0f11d8 100644 >>>> --- a/test/CMakeLists.txt >>>> +++ b/test/CMakeLists.txt >>>> @@ -59,6 +59,44 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS >>>> tarantool-tests >>>> ) >>>> >>>> +find_program(GCOVR gcovr) >>>> +find_program(GCOV gcov) >>>> +set(COVERAGE_HTML_REPORT "luajit-coverage.html") >>>> +set(COVERAGE_JSON_REPORT "luajit-coverage.json") >>> Should we add ${PROJECT_BUILD_DIR} prefix to be more graceful for >>> out-of-source build? >>> Also, I see tons of html helper files in the working directory. Is there >>> any way to use some <coverage/> directory for all this stuff instead? >> >>Updated: >> >> >>--- a/test/CMakeLists.txt >>+++ b/test/CMakeLists.txt >>@@ -61,9 +61,10 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS >> >> find_program(GCOVR gcovr) >> find_program(GCOV gcov) >>-set(COVERAGE_HTML_REPORT "luajit-coverage.html") >>-set(COVERAGE_JSON_REPORT "luajit-coverage.json") >>-if(ENABLE_COVERAGE) >>+set(COVERAGE_DIR "${PROJECT_BINARY_DIR}/coverage") >>+set(COVERAGE_HTML_REPORT "${COVERAGE_DIR}/luajit.html") >>+set(COVERAGE_JSON_REPORT "${COVERAGE_DIR}/luajit.json") >> >> >>> >>>> +if(ENABLE_COVERAGE) >>>> + if(NOT GCOVR OR NOT GCOV) >>>> + add_custom_target(${PROJECT_NAME}-coverage) >>>> + add_custom_command(TARGET ${PROJECT_NAME}-coverage >>>> + COMMENT "Either `gcovr' or `gcov` not found, so ${PROJECT_NAME}-coverage target is dummy" >>> Minor: Please split the line to the several, to avoid too long lines. >> >> >>Done. >> >>> >>>> + ) >>>> + return() >>> I suppose, we should create dummy target instead. >>> | cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DLUAJIT_ENABLE_GC64=OFF -DENABLE_COVERAGE=OFF && make -j >>> | -- [SetVersion] Reading version from VCS: v2.1.0-beta3-331-g4a26e6a1 >>> | -- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 4 >>> | CMake Error at test/CMakeLists.txt:118 (add_dependencies): >>> | Cannot add target-level dependencies to non-existent target >>> | "LuaJIT-coverage". >>> >>>> + endif() >>>> + >>>> + add_custom_target(${PROJECT_NAME}-coverage) >>>> + add_custom_command(TARGET ${PROJECT_NAME}-coverage >>>> + COMMENT "Building coverage report" >>>> + COMMAND >>>> + ${GCOVR} >>>> + # See https://gcovr.com/en/stable/guide/configuration.html >>>> + --root ${PROJECT_SOURCE_DIR} >>>> + --filter ${PROJECT_SOURCE_DIR}/src >>> I suppose some files like *.dasc files or buildvm sources should be >>> excluded too, since they are not informative. >> >>Done. >> >> >>>> + --print-summary >>>> + --output ${COVERAGE_HTML_REPORT} >>>> + --coveralls ${COVERAGE_JSON_REPORT} >>> I see also output >>> >>>> + --html >>>> + --html-title "LuaJIT Code Coverage Report" >>>> + --html-details >>>> + --sort-percentage >>> I can't see an option for decreasing percentage of uncovered lines? >>> Is its default value? If it is, then it should be used (as far as we want >>> to see uncovered cases first). >> >> >>Seems it can sort in that order only. >> >>> >>>> + --branches >>>> + --decisions >>> Do we need this option? AFAICS, this is mostly about Functional Safety >>> applications (road vehicles, etc.). >> >>I would leave it. >> >> >>> >>>> + --verbose >>> I suppose that we should use --verbose option only when VERBOSE=1 env >>> var is set. >>> >>> Also, I'm suprised by lines like this: >>> | (DEBUG) Parsing coverage data for file /home/burii/reviews/luajit/gcov/src/lj_tab.h >>> | (DEBUG) Starting the decision analysis >>> | (DEBUG) Decision Analysis finished! >>> | (DEBUG) Finding source file corresponding to a gcov data file >>> Is it the part of the --verbose options? >> >>Right, it is enabled only when --verbose is passed. Removed option >>--verbose at all, >> >>I believe we don't need it in normal use. >> >> >>> >>>> + -j ${CMAKE_BUILD_PARALLEL_LEVEL} >>>> + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} >>>> + ) >>>> + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") >>>> + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") >>>> +endif(ENABLE_COVERAGE) >>>> + >>>> if(LUAJIT_USE_TEST) >>>> if(POLICY CMP0037) >>>> if(CMAKE_VERSION VERSION_LESS 3.11) >>>> @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) >>>> ${PROJECT_NAME}-test >>>> ${PROJECT_NAME}-luacheck >>>> ) >>>> + >>>> + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) >>> Is it necessary to rerun all tests to generate this analisys every >>> time? >> >>What could be a reasons to regenerate coverage report without rerunning >>tests? >> >>I can make a target LuaJIT-coverage that will generate report and add >>target coverage that will >> >>run tests and then call LuaJIT-coverage. What do you think? >> >> >>> >>>> endif() >>>> diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt >>>> index 17255345..8cd76b44 100644 >>>> --- a/test/tarantool-c-tests/CMakeLists.txt >>>> +++ b/test/tarantool-c-tests/CMakeLists.txt >>>> @@ -45,7 +45,7 @@ foreach(test_source ${tests}) >>>> OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" >>>> RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" >>>> ) >>>> - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY}) >>>> + target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) >>> Why is it done unconditionally? >> >>Ah, missed it. Thanks! >> >> >>--- a/test/tarantool-c-tests/CMakeLists.txt >>+++ b/test/tarantool-c-tests/CMakeLists.txt >>@@ -45,7 +45,11 @@ foreach(test_source ${tests}) >> OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" >> RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" >> ) >>- target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) >>+ set(libtest-libs libtest ${LUAJIT_LIBRARY}) >>+ if (LUAJIT_ENABLE_COVERAGE) >>+ set(libtest-libs ${libtest-libs} --coverage) >>+ endif (LUAJIT_ENABLE_COVERAGE) >>+ target_link_libraries(${exe} ${libtest-libs}) >> LIST(APPEND TESTS_COMPILED ${exe}) >> endforeach() >> >>> >>>> LIST(APPEND TESTS_COMPILED ${exe}) >>>> endforeach() >>>> >>>> -- >>>> 2.34.1 >>>> > [-- Attachment #2: Type: text/html, Size: 15665 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-07-27 8:42 ` Sergey Bronnikov via Tarantool-patches 2023-07-30 12:55 ` Maxim Kokryashkin via Tarantool-patches @ 2023-07-30 13:34 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 15:25 ` Sergey Bronnikov via Tarantool-patches 1 sibling, 1 reply; 10+ messages in thread From: Maxim Kokryashkin via Tarantool-patches @ 2023-07-30 13:34 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: max.kokryashkin, tarantool-patches, Sergey Bronnikov Hi! Thanks for the patch and the fixes! Please consider my comments below. There were neither v2 letter, nor diffs for all of the changes, so I'll bring the missing ones by myself. Here is the commit message on the branch: | cmake: add code coverage support | | The patch adds building code coverage support using gcovr [1] and gcov. | gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage | executes regression tests, proccess *.gcno and *.gcda files with gcov, Typo: s/process/processes/ | builds detailed HTML report and prints summary about code coverage. Typo: s/detailed/a detailed Typo: s/summary/a summary/ | | ``` | $ cmake -S . -B build -DENABLE_COVERAGE=ON | $ cmake --build build --parallel | $ cmake --build build --target LuaJIT-coverage | | <snipped> | | lines: 84.1% (26056 out of 30997) | functions: 88.8% (2055 out of 2314) | branches: 71.5% (14801 out of 20703) | ``` | | 1. https://gcovr.com/ | 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr On Thu, Jul 27, 2023 at 11:42:50AM +0300, Sergey Bronnikov via Tarantool-patches wrote: > Hello, Sergey! Thanks for review! > > > On 7/26/23 18:02, Sergey Kaplun wrote: > > Hi, Sergey! > > Thanks for the patch! > > It's very interesting to see coverage statistics of the LuaJIT by our > > tests: have much more work to do! :) > > > > Also, it's interesting to compare test suites with each other (done with > > lcov, sorted alphabetically): > > | lua-Harness-tests: > > | lines......: 54.2% (13550 of 25023 lines) > > | functions..: 62.4% (1103 of 1769 functions) > > | branches...: 37.3% (8293 of 22222 branches) > > | LuaJIT-tests: > > | lines......: 71.3% (17846 of 25023 lines) > > | functions..: 74.1% (1310 of 1769 functions) > > | branches...: 54.7% (12155 of 22222 branches) > > | PUC-Rio-Lua-5.1-tests: > > | lines......: 51.2% (12800 of 25023 lines) > > | functions..: 57.9% (1025 of 1769 functions) > > | branches...: 36.0% (7997 of 22222 branches) > > | tarantool-c-tests: > > | lines......: 33.3% (8210 of 24676 lines) > > | functions..: 38.6% (672 of 1740 functions) > > | branches...: 20.4% (4490 of 22014 branches) > > | tarantool-tests: > > | lines......: 69.6% (17405 of 25023 lines) > > | functions..: 76.7% (1357 of 1769 functions) > > | branches...: 49.9% (11090 of 22222 branches) > > > > | Overall: > > | lines......: 86.0% (21509 of 25023 lines) > > | functions..: 90.8% (1606 of 1769 functions) > > | branches...: 66.7% (14812 of 22222 branches) > > > > Please, consider my comments below. > It is interesting, thanks for measuring it! > > > > On 25.07.23, Sergey Bronnikov wrote: > > > From: Sergey Bronnikov <sergeyb@tarantool.org> > > > > > > The patch adds building code coverage report using gcovr [1] and gcov. > > > gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage > > Do we really need any features provided by gcovr that are omitted in > > lcov? It's not a problem for me, but there are many more dependencies > > for gcovr than for lcov. Also, it uses python that isn't such > > available (I mean installed by default) as perl. > > gcovr has exactly three dependencies, see [1]. > > Other dependencies specified for gcovr ebuild are "test" dependencies, > required for > > those who will make patches and run tests for gcovr. I don't get why ebuild > maintainer > > made these dependencies mandatory. > > 1. https://github.com/gcovr/gcovr/blob/27e6b3aaec7c91f54fa8e7531f8d652bf6aeadd4/setup.py#L54 > > > BTW lcov has a bigger number of dependencies than specified > > in ebuild (or these dependencies are included in a Perl distribution, see > [2]: > > > - Capture::Tiny > - DateTime > - Devel::Cover > - Digest::MD5 > - File::Spec > - at least one flavor of JSON module. > In order of performance/preference: > - JSON::XS > - Cpanel::JSON::XS > - JSON::PP > - JSON > - Memory::Process > - Time::HiRes > > > 2. https://github.com/linux-test-project/lcov > > > > > > | $ equery depgraph dev-util/gcovr > > | ... > > | * dependency graph for dev-util/gcovr-6.0 > > | `-- dev-util/gcovr-6.0 [~amd64 keyword] > > | `-- dev-python/jinja-3.1.2 (dev-python/jinja) amd64 > > | `-- dev-python/lxml-4.9.2 (dev-python/lxml) amd64 > > | `-- dev-python/pygments-2.15.1 (dev-python/pygments) amd64 > > | `-- dev-python/yaxmldiff-0.1.0 (dev-python/yaxmldiff) amd64 > > | `-- dev-lang/python-3.11.4 (dev-lang/python) amd64 > > | `-- dev-python/pytest-timeout-2.1.0 (dev-python/pytest-timeout) amd64 > > | `-- dev-python/pytest-7.3.2 (>=dev-python/pytest-7.3.1) amd64 > > | `-- dev-python/gpep517-13 (>=dev-python/gpep517-13) amd64 > > | `-- dev-python/setuptools-67.8.0-r1 (>=dev-python/setuptools-67.8.0-r1) amd64 > > | [ dev-util/gcovr-6.0 stats: packages (10), max depth (1) ] > > | $ equery depgraph dev-util/lcov > > | ... > > | * dependency graph for dev-util/lcov-1.15 > > | `-- dev-util/lcov-1.15 ~amd64 > > | `-- dev-lang/perl-5.38.0-r1 (dev-lang/perl) ~amd64 > > | `-- dev-perl/JSON-4.100.0 (dev-perl/JSON) ~amd64 > > | `-- dev-perl/PerlIO-gzip-0.200.0-r1 (dev-perl/PerlIO-gzip) amd64 > > | [ dev-util/lcov-1.15 stats: packages (4), max depth (1) ] > > > > If we don't really need "decisions" metrics that are provided by default > > by gcovr I suggest to consider to use lcov instead, since I don't see > > any other advantages. > > > > > executes regression tests, proccess *.gcno and *.gcda files with gcov, > > > builds detailed HTML report and prints summary about code coverage. > > > > > > ``` > > > $ cmake -S . -B build -DENABLE_COVERAGE=ON > > I suggest to use -DLUAJIT_ENABLE_COVERAGE=ON to be consistent with other > > options. > I don't mind, renamed to LUAJIT_ENABLE_COVERAGE. > > > > > $ cmake --build build --parallel > > > $ cmake --build build --target LuaJIT-coverage > > > > > > <snipped> > > > > > > lines: 84.1% (26056 out of 30997) > > > functions: 88.8% (2055 out of 2314) > > > branches: 71.5% (14801 out of 20703) > > > ``` > > > > > > 1. https://gcovr.com/ > > > 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr > > > --- > > > CMakeLists.txt | 9 ++++++ > > > test/CMakeLists.txt | 40 +++++++++++++++++++++++++++ > > > test/tarantool-c-tests/CMakeLists.txt | 2 +- > > > 3 files changed, 50 insertions(+), 1 deletion(-) > > > > > > diff --git a/CMakeLists.txt b/CMakeLists.txt > > > index 6ef24bba..8bc63b90 100644 > > > --- a/CMakeLists.txt > > > +++ b/CMakeLists.txt > > > @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) > > > ) > > > endif() > > > +set(ENABLE_COVERAGE_DEFAULT OFF) > > > +option(ENABLE_COVERAGE "Enable integration with gcovr, a code coverage program" ${ENABLE_COVERAGE_DEFAULT}) > > Minor: Please split the line to the several, to avoid too long lines. > > Fixed. > > > > > > > +if (ENABLE_COVERAGE) > > > + AppendFlags(CMAKE_C_FLAGS > > > + -fprofile-arcs > > > + -ftest-coverage > > Should we use just --coverage? > Sure, updated. Now the alignments seems kinda strange. I believe it would be better to make it a signle line now, like: | AppendFlags(CMAKE_C_FLAGS --coverage) > > > > | --coverage > > | This option is used to compile and link code instrumented > > | for coverage analysis. The option is a synonym for > > | -fprofile-arcs -ftest-coverage (when compiling) and -lgcov > > | (when linking). > > > > See man gcc(1) for details. > > > > > + ) > > > +endif() > > > + > > > # Auxiliary flags for main targets (libraries, binaries). > > > AppendFlags(TARGET_C_FLAGS > > > -D_FILE_OFFSET_BITS=64 I suggest moving most of the logic with finding gcov/running coverage into a separate cmake module, just to make the test-related one more readable. What do you think? > > > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > > > index 47296a22..9b0f11d8 100644 > > > --- a/test/CMakeLists.txt > > > +++ b/test/CMakeLists.txt > > > @@ -59,6 +59,44 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS > > > tarantool-tests > > > ) > > > +find_program(GCOVR gcovr) > > > +find_program(GCOV gcov) > > > +set(COVERAGE_HTML_REPORT "luajit-coverage.html") > > > +set(COVERAGE_JSON_REPORT "luajit-coverage.json") > > Should we add ${PROJECT_BUILD_DIR} prefix to be more graceful for > > out-of-source build? > > Also, I see tons of html helper files in the working directory. Is there > > any way to use some <coverage/> directory for all this stuff instead? > > Updated: > > > --- a/test/CMakeLists.txt > +++ b/test/CMakeLists.txt > @@ -61,9 +61,10 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS > > find_program(GCOVR gcovr) > find_program(GCOV gcov) > -set(COVERAGE_HTML_REPORT "luajit-coverage.html") > -set(COVERAGE_JSON_REPORT "luajit-coverage.json") > -if(ENABLE_COVERAGE) > +set(COVERAGE_DIR "${PROJECT_BINARY_DIR}/coverage") > +set(COVERAGE_HTML_REPORT "${COVERAGE_DIR}/luajit.html") > +set(COVERAGE_JSON_REPORT "${COVERAGE_DIR}/luajit.json") > > > > > > > +if(ENABLE_COVERAGE) > > > + if(NOT GCOVR OR NOT GCOV) > > > + add_custom_target(${PROJECT_NAME}-coverage) > > > + add_custom_command(TARGET ${PROJECT_NAME}-coverage > > > + COMMENT "Either `gcovr' or `gcov` not found, so ${PROJECT_NAME}-coverage target is dummy" > > Minor: Please split the line to the several, to avoid too long lines. > > > Done. > > > > > > + ) > > > + return() > > I suppose, we should create dummy target instead. > > | cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DLUAJIT_ENABLE_GC64=OFF -DENABLE_COVERAGE=OFF && make -j > > | -- [SetVersion] Reading version from VCS: v2.1.0-beta3-331-g4a26e6a1 > > | -- [SetBuildParallelLevel] CMAKE_BUILD_PARALLEL_LEVEL is 4 > > | CMake Error at test/CMakeLists.txt:118 (add_dependencies): > > | Cannot add target-level dependencies to non-existent target > > | "LuaJIT-coverage". > > > > > + endif() > > > + > > > + add_custom_target(${PROJECT_NAME}-coverage) > > > + add_custom_command(TARGET ${PROJECT_NAME}-coverage > > > + COMMENT "Building coverage report" > > > + COMMAND > > > + ${GCOVR} > > > + # See https://gcovr.com/en/stable/guide/configuration.html > > > + --root ${PROJECT_SOURCE_DIR} > > > + --filter ${PROJECT_SOURCE_DIR}/src > > I suppose some files like *.dasc files or buildvm sources should be > > excluded too, since they are not informative. > > Done. You did not include the actual diff for the change, so I've checked it on GitHub. Here are two comments, that were added: | # Exclude DynASM files, that contains a low-level VM code for CPUs. Typo: s/that contains/that contain/ | # Exclude buildvm source code, it's a project's infrastructure. Typo: s/a project's/the project's/ > > > > > + --print-summary > > > + --output ${COVERAGE_HTML_REPORT} > > > + --coveralls ${COVERAGE_JSON_REPORT} > > I see also output > > > > > + --html > > > + --html-title "LuaJIT Code Coverage Report" > > > + --html-details > > > + --sort-percentage > > I can't see an option for decreasing percentage of uncovered lines? > > Is its default value? If it is, then it should be used (as far as we want > > to see uncovered cases first). > > > Seems it can sort in that order only. > > > > > > + --branches > > > + --decisions > > Do we need this option? AFAICS, this is mostly about Functional Safety > > applications (road vehicles, etc.). > > I would leave it. > > > > > > > + --verbose > > I suppose that we should use --verbose option only when VERBOSE=1 env > > var is set. > > > > Also, I'm suprised by lines like this: > > | (DEBUG) Parsing coverage data for file /home/burii/reviews/luajit/gcov/src/lj_tab.h > > | (DEBUG) Starting the decision analysis > > | (DEBUG) Decision Analysis finished! > > | (DEBUG) Finding source file corresponding to a gcov data file > > Is it the part of the --verbose options? > > Right, it is enabled only when --verbose is passed. Removed option --verbose > at all, > > I believe we don't need it in normal use. > > > > > > > + -j ${CMAKE_BUILD_PARALLEL_LEVEL} > > > + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} > > > + ) > > > + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") > > > + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") > > > +endif(ENABLE_COVERAGE) > > > + > > > if(LUAJIT_USE_TEST) > > > if(POLICY CMP0037) > > > if(CMAKE_VERSION VERSION_LESS 3.11) > > > @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) > > > ${PROJECT_NAME}-test > > > ${PROJECT_NAME}-luacheck > > > ) > > > + > > > + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) > > Is it necessary to rerun all tests to generate this analisys every > > time? > > What could be a reasons to regenerate coverage report without rerunning > tests? > > I can make a target LuaJIT-coverage that will generate report and add target > coverage that will > > run tests and then call LuaJIT-coverage. What do you think? Seems adequate to me. > > > > > > > endif() > > > diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt > > > index 17255345..8cd76b44 100644 > > > --- a/test/tarantool-c-tests/CMakeLists.txt > > > +++ b/test/tarantool-c-tests/CMakeLists.txt > > > @@ -45,7 +45,7 @@ foreach(test_source ${tests}) > > > OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" > > > RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" > > > ) > > > - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY}) > > > + target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) > > Why is it done unconditionally? > > Ah, missed it. Thanks! > > > --- a/test/tarantool-c-tests/CMakeLists.txt > +++ b/test/tarantool-c-tests/CMakeLists.txt > @@ -45,7 +45,11 @@ foreach(test_source ${tests}) > OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" > RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" > ) > - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) > + set(libtest-libs libtest ${LUAJIT_LIBRARY}) > + if (LUAJIT_ENABLE_COVERAGE) > + set(libtest-libs ${libtest-libs} --coverage) > + endif (LUAJIT_ENABLE_COVERAGE) > + target_link_libraries(${exe} ${libtest-libs}) > LIST(APPEND TESTS_COMPILED ${exe}) > endforeach() > > > > > > LIST(APPEND TESTS_COMPILED ${exe}) > > > endforeach() > > > -- > > > 2.34.1 > > > Best regards, Maxim Kokryashkin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-07-30 13:34 ` Maxim Kokryashkin via Tarantool-patches @ 2023-08-01 15:25 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 0 replies; 10+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2023-08-01 15:25 UTC (permalink / raw) To: Maxim Kokryashkin; +Cc: max.kokryashkin, tarantool-patches, Sergey Bronnikov Hi, Max! thanks for your comments. See my comments below. On 7/30/23 16:34, Maxim Kokryashkin wrote: <snipped> > Here is the commit message on the branch: > | cmake: add code coverage support > | > | The patch adds building code coverage support using gcovr [1] and gcov. > | gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage > | executes regression tests, proccess *.gcno and *.gcda files with gcov, > Typo: s/process/processes/ > | builds detailed HTML report and prints summary about code coverage. > Typo: s/detailed/a detailed > Typo: s/summary/a summary/ Fixed. <snipped> >> >>>> +if (ENABLE_COVERAGE) >>>> + AppendFlags(CMAKE_C_FLAGS >>>> + -fprofile-arcs >>>> + -ftest-coverage >>> Should we use just --coverage? >> Sure, updated. > Now the alignments seems kinda strange. I believe it would be > better to make it a signle line now, like: > | AppendFlags(CMAKE_C_FLAGS --coverage) Fixed. Actually I left it on a separate line intentionally. This will minimize a diff in case adding more options to CFLAGS. >>> | --coverage >>> | This option is used to compile and link code instrumented >>> | for coverage analysis. The option is a synonym for >>> | -fprofile-arcs -ftest-coverage (when compiling) and -lgcov >>> | (when linking). >>> >>> See man gcc(1) for details. >>> >>>> + ) >>>> +endif() >>>> + >>>> # Auxiliary flags for main targets (libraries, binaries). >>>> AppendFlags(TARGET_C_FLAGS >>>> -D_FILE_OFFSET_BITS=64 > I suggest moving most of the logic with finding gcov/running coverage into > a separate cmake module, just to make the test-related one more readable. > What do you think? Good idea. Moved to cmake/CodeCoverage.cmake. <snipped> >>>> + endif() >>>> + >>>> + add_custom_target(${PROJECT_NAME}-coverage) >>>> + add_custom_command(TARGET ${PROJECT_NAME}-coverage >>>> + COMMENT "Building coverage report" >>>> + COMMAND >>>> + ${GCOVR} >>>> + # See https://gcovr.com/en/stable/guide/configuration.html >>>> + --root ${PROJECT_SOURCE_DIR} >>>> + --filter ${PROJECT_SOURCE_DIR}/src >>> I suppose some files like *.dasc files or buildvm sources should be >>> excluded too, since they are not informative. >> Done. > You did not include the actual diff for the change, so I've checked it > on GitHub. Here are two comments, that were added: > | # Exclude DynASM files, that contains a low-level VM code for CPUs. > Typo: s/that contains/that contain/ > > | # Exclude buildvm source code, it's a project's infrastructure. > Typo: s/a project's/the project's/ Fixed. >> >>>> + --print-summary >>>> + --output ${COVERAGE_HTML_REPORT} >>>> + --coveralls ${COVERAGE_JSON_REPORT} >>> I see also output >>> >>>> + --html >>>> + --html-title "LuaJIT Code Coverage Report" >>>> + --html-details >>>> + --sort-percentage >>> I can't see an option for decreasing percentage of uncovered lines? >>> Is its default value? If it is, then it should be used (as far as we want >>> to see uncovered cases first). >> >> Seems it can sort in that order only. >> >>>> + --branches >>>> + --decisions >>> Do we need this option? AFAICS, this is mostly about Functional Safety >>> applications (road vehicles, etc.). >> I would leave it. >> >> >>>> + --verbose >>> I suppose that we should use --verbose option only when VERBOSE=1 env >>> var is set. >>> >>> Also, I'm suprised by lines like this: >>> | (DEBUG) Parsing coverage data for file /home/burii/reviews/luajit/gcov/src/lj_tab.h >>> | (DEBUG) Starting the decision analysis >>> | (DEBUG) Decision Analysis finished! >>> | (DEBUG) Finding source file corresponding to a gcov data file >>> Is it the part of the --verbose options? >> Right, it is enabled only when --verbose is passed. Removed option --verbose >> at all, >> >> I believe we don't need it in normal use. >> >> >>>> + -j ${CMAKE_BUILD_PARALLEL_LEVEL} >>>> + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} >>>> + ) >>>> + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") >>>> + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") >>>> +endif(ENABLE_COVERAGE) >>>> + >>>> if(LUAJIT_USE_TEST) >>>> if(POLICY CMP0037) >>>> if(CMAKE_VERSION VERSION_LESS 3.11) >>>> @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) >>>> ${PROJECT_NAME}-test >>>> ${PROJECT_NAME}-luacheck >>>> ) >>>> + >>>> + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) >>> Is it necessary to rerun all tests to generate this analisys every >>> time? >> What could be a reasons to regenerate coverage report without rerunning >> tests? >> >> I can make a target LuaJIT-coverage that will generate report and add target >> coverage that will >> >> run tests and then call LuaJIT-coverage. What do you think? > Seems adequate to me. Added: --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -78,6 +78,9 @@ if(LUAJIT_USE_TEST) ) if (LUAJIT_ENABLE_COVERAGE) - add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) + add_custom_target(coverage DEPENDS + ${PROJECT_NAME}-test + ${PROJECT_NAME}-coverage + ) endif (LUAJIT_ENABLE_COVERAGE) endif() <snipped> > Maxim Kokryashkin ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-07-25 13:52 [Tarantool-patches] [PATCH] cmake: add code coverage support Sergey Bronnikov via Tarantool-patches 2023-07-26 15:02 ` Sergey Kaplun via Tarantool-patches @ 2023-08-08 8:16 ` Sergey Bronnikov via Tarantool-patches 2023-08-08 10:47 ` Sergey Kaplun via Tarantool-patches 1 sibling, 1 reply; 10+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2023-08-08 8:16 UTC (permalink / raw) To: Sergey Bronnikov, tarantool-patches, Sergey Kaplun, max.kokryashkin Hello, Unfortunately we cannot control order of running dependencies for target. On practice we can get a situation when target LuaJIT-coverage, that generates a code coverage report, runs before finishing target LuaJIT-test, that generates code coverage data. Therefore target "coverage" that combined LuaJIT-test and LuaJIT-coverage, has been removed by patch below. Updated patches force-pushed. diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index 78abf2e1..13ea6c5b 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -50,8 +50,10 @@ jobs: - name: build run: cmake --build . --parallel working-directory: ${{ env.BUILDDIR }} - - name: test and generate code coverage report - run: cmake --build ${{ env.BUILDDIR }} --parallel --target coverage + - name: run regression tests + run: cmake --build ${{ env.BUILDDIR }} --parallel --target test + - name: generate code coverage report + run: cmake --build ${{ env.BUILDDIR }} --parallel --target LuaJIT-coverage - name: send code coverage to coveralls.io run: | curl -LO https://coveralls.io/coveralls-linux.tar.gz diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt index e23d6d45..47296a22 100644 --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -76,11 +76,4 @@ if(LUAJIT_USE_TEST) ${PROJECT_NAME}-test ${PROJECT_NAME}-luacheck ) - - if (LUAJIT_ENABLE_COVERAGE) - add_custom_target(coverage DEPENDS - ${PROJECT_NAME}-test - ${PROJECT_NAME}-coverage - ) - endif (LUAJIT_ENABLE_COVERAGE) endif() Sergey On 7/25/23 16:52, Sergey Bronnikov wrote: > From: Sergey Bronnikov <sergeyb@tarantool.org> > > The patch adds building code coverage report using gcovr [1] and gcov. > gcovr is a better version of lcov, see [2]. CMake target LuaJIT-coverage > executes regression tests, proccess *.gcno and *.gcda files with gcov, > builds detailed HTML report and prints summary about code coverage. > > ``` > $ cmake -S . -B build -DENABLE_COVERAGE=ON > $ cmake --build build --parallel > $ cmake --build build --target LuaJIT-coverage > > <snipped> > > lines: 84.1% (26056 out of 30997) > functions: 88.8% (2055 out of 2314) > branches: 71.5% (14801 out of 20703) > ``` > > 1. https://gcovr.com/ > 2. https://gcovr.com/en/stable/faq.html#what-is-the-difference-between-lcov-and-gcovr > --- > CMakeLists.txt | 9 ++++++ > test/CMakeLists.txt | 40 +++++++++++++++++++++++++++ > test/tarantool-c-tests/CMakeLists.txt | 2 +- > 3 files changed, 50 insertions(+), 1 deletion(-) > > diff --git a/CMakeLists.txt b/CMakeLists.txt > index 6ef24bba..8bc63b90 100644 > --- a/CMakeLists.txt > +++ b/CMakeLists.txt > @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) > ) > endif() > > +set(ENABLE_COVERAGE_DEFAULT OFF) > +option(ENABLE_COVERAGE "Enable integration with gcovr, a code coverage program" ${ENABLE_COVERAGE_DEFAULT}) > +if (ENABLE_COVERAGE) > + AppendFlags(CMAKE_C_FLAGS > + -fprofile-arcs > + -ftest-coverage > + ) > +endif() > + > # Auxiliary flags for main targets (libraries, binaries). > AppendFlags(TARGET_C_FLAGS > -D_FILE_OFFSET_BITS=64 > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > index 47296a22..9b0f11d8 100644 > --- a/test/CMakeLists.txt > +++ b/test/CMakeLists.txt > @@ -59,6 +59,44 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS > tarantool-tests > ) > > +find_program(GCOVR gcovr) > +find_program(GCOV gcov) > +set(COVERAGE_HTML_REPORT "luajit-coverage.html") > +set(COVERAGE_JSON_REPORT "luajit-coverage.json") > +if(ENABLE_COVERAGE) > + if(NOT GCOVR OR NOT GCOV) > + add_custom_target(${PROJECT_NAME}-coverage) > + add_custom_command(TARGET ${PROJECT_NAME}-coverage > + COMMENT "Either `gcovr' or `gcov` not found, so ${PROJECT_NAME}-coverage target is dummy" > + ) > + return() > + endif() > + > + add_custom_target(${PROJECT_NAME}-coverage) > + add_custom_command(TARGET ${PROJECT_NAME}-coverage > + COMMENT "Building coverage report" > + COMMAND > + ${GCOVR} > + # See https://gcovr.com/en/stable/guide/configuration.html > + --root ${PROJECT_SOURCE_DIR} > + --filter ${PROJECT_SOURCE_DIR}/src > + --print-summary > + --output ${COVERAGE_HTML_REPORT} > + --coveralls ${COVERAGE_JSON_REPORT} > + --html > + --html-title "LuaJIT Code Coverage Report" > + --html-details > + --sort-percentage > + --branches > + --decisions > + --verbose > + -j ${CMAKE_BUILD_PARALLEL_LEVEL} > + WORKING_DIRECTORY ${PROJECT_SOURCE_DIR} > + ) > + message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}") > + message(STATUS "Code coverage JSON report: ${COVERAGE_JSON_REPORT}") > +endif(ENABLE_COVERAGE) > + > if(LUAJIT_USE_TEST) > if(POLICY CMP0037) > if(CMAKE_VERSION VERSION_LESS 3.11) > @@ -76,4 +114,6 @@ if(LUAJIT_USE_TEST) > ${PROJECT_NAME}-test > ${PROJECT_NAME}-luacheck > ) > + > + add_dependencies(${PROJECT_NAME}-coverage ${PROJECT_NAME}-test) > endif() > diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt > index 17255345..8cd76b44 100644 > --- a/test/tarantool-c-tests/CMakeLists.txt > +++ b/test/tarantool-c-tests/CMakeLists.txt > @@ -45,7 +45,7 @@ foreach(test_source ${tests}) > OUTPUT_NAME "${exe}${C_TEST_SUFFIX}" > RUNTIME_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}" > ) > - target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY}) > + target_link_libraries(${exe} libtest ${LUAJIT_LIBRARY} --coverage) > LIST(APPEND TESTS_COMPILED ${exe}) > endforeach() > ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-08-08 8:16 ` Sergey Bronnikov via Tarantool-patches @ 2023-08-08 10:47 ` Sergey Kaplun via Tarantool-patches 2023-08-08 19:25 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 1 reply; 10+ messages in thread From: Sergey Kaplun via Tarantool-patches @ 2023-08-08 10:47 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: Sergey Bronnikov, tarantool-patches, max.kokryashkin Hi, Sergey! Thanks for the fixes! LGTM, except the last minor nit ;). On 08.08.23, Sergey Bronnikov wrote: > Hello, <snipped> > > > diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml > index 78abf2e1..13ea6c5b 100644 > --- a/.github/workflows/coverage.yml > +++ b/.github/workflows/coverage.yml > @@ -50,8 +50,10 @@ jobs: > - name: build > run: cmake --build . --parallel > working-directory: ${{ env.BUILDDIR }} > - - name: test and generate code coverage report > - run: cmake --build ${{ env.BUILDDIR }} --parallel --target coverage > + - name: run regression tests > + run: cmake --build ${{ env.BUILDDIR }} --parallel --target test I suppose it should be LuaJIT-test, since we don't want to run linters here. See also [1]. > + - name: generate code coverage report > + run: cmake --build ${{ env.BUILDDIR }} --parallel --target LuaJIT-coverage > - name: send code coverage to coveralls.io > run: | > curl -LO https://coveralls.io/coveralls-linux.tar.gz <snipped> > > [1]: https://github.com/tarantool/luajit/actions/runs/5794532687/job/15704307515#step:7:46 -- Best regards, Sergey Kaplun ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-08-08 10:47 ` Sergey Kaplun via Tarantool-patches @ 2023-08-08 19:25 ` Sergey Bronnikov via Tarantool-patches 2023-08-14 7:46 ` Maxim Kokryashkin via Tarantool-patches 0 siblings, 1 reply; 10+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2023-08-08 19:25 UTC (permalink / raw) To: Sergey Kaplun; +Cc: Sergey Bronnikov, tarantool-patches, max.kokryashkin Sergey, On 8/8/23 13:47, Sergey Kaplun wrote: <snipped> >> >> diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml >> index 78abf2e1..13ea6c5b 100644 >> --- a/.github/workflows/coverage.yml >> +++ b/.github/workflows/coverage.yml >> @@ -50,8 +50,10 @@ jobs: >> - name: build >> run: cmake --build . --parallel >> working-directory: ${{ env.BUILDDIR }} >> - - name: test and generate code coverage report >> - run: cmake --build ${{ env.BUILDDIR }} --parallel --target coverage >> + - name: run regression tests >> + run: cmake --build ${{ env.BUILDDIR }} --parallel --target test > I suppose it should be LuaJIT-test, since we don't want to run linters > here. See also [1]. > > <snipped> Fixed, thanks! --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -51,7 +51,7 @@ jobs: run: cmake --build . --parallel working-directory: ${{ env.BUILDDIR }} - name: run regression tests - run: cmake --build ${{ env.BUILDDIR }} --parallel --target test + run: cmake --build ${{ env.BUILDDIR }} --parallel --target LuaJIT-test - name: generate code coverage report run: cmake --build ${{ env.BUILDDIR }} --parallel --target LuaJIT-coverage - name: send code coverage to coveralls.io ^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [Tarantool-patches] [PATCH] cmake: add code coverage support 2023-08-08 19:25 ` Sergey Bronnikov via Tarantool-patches @ 2023-08-14 7:46 ` Maxim Kokryashkin via Tarantool-patches 0 siblings, 0 replies; 10+ messages in thread From: Maxim Kokryashkin via Tarantool-patches @ 2023-08-14 7:46 UTC (permalink / raw) To: Sergey Bronnikov; +Cc: Sergey Bronnikov, max.kokryashkin, tarantool-patches [-- Attachment #1: Type: text/plain, Size: 1628 bytes --] Hi, Sergey! Thanks for the fixes! LGTM -- Best regards, Maxim Kokryashkin > >>Sergey, >> >> >>On 8/8/23 13:47, Sergey Kaplun wrote: >> >> >><snipped> >> >>>> >>>> diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml >>>> index 78abf2e1..13ea6c5b 100644 >>>> --- a/.github/workflows/coverage.yml >>>> +++ b/.github/workflows/coverage.yml >>>> @@ -50,8 +50,10 @@ jobs: >>>> - name: build >>>> run: cmake --build . --parallel >>>> working-directory: ${{ env.BUILDDIR }} >>>> - - name: test and generate code coverage report >>>> - run: cmake --build ${{ env.BUILDDIR }} --parallel --target coverage >>>> + - name: run regression tests >>>> + run: cmake --build ${{ env.BUILDDIR }} --parallel --target test >>> I suppose it should be LuaJIT-test, since we don't want to run linters >>> here. See also [1]. >>> >>> <snipped> >> >>Fixed, thanks! >> >> >>--- a/.github/workflows/coverage.yml >>+++ b/.github/workflows/coverage.yml >>@@ -51,7 +51,7 @@ jobs: >> run: cmake --build . --parallel >> working-directory: ${{ env.BUILDDIR }} >> - name: run regression tests >>- run: cmake --build ${{ env.BUILDDIR }} --parallel --target test >>+ run: cmake --build ${{ env.BUILDDIR }} --parallel --target >>LuaJIT-test >> - name: generate code coverage report >> run: cmake --build ${{ env.BUILDDIR }} --parallel --target >>LuaJIT-coverage >> - name: send code coverage to coveralls.io > [-- Attachment #2: Type: text/html, Size: 2775 bytes --] ^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2023-08-14 7:46 UTC | newest] Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2023-07-25 13:52 [Tarantool-patches] [PATCH] cmake: add code coverage support Sergey Bronnikov via Tarantool-patches 2023-07-26 15:02 ` Sergey Kaplun via Tarantool-patches 2023-07-27 8:42 ` Sergey Bronnikov via Tarantool-patches 2023-07-30 12:55 ` Maxim Kokryashkin via Tarantool-patches 2023-07-30 13:34 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 15:25 ` Sergey Bronnikov via Tarantool-patches 2023-08-08 8:16 ` Sergey Bronnikov via Tarantool-patches 2023-08-08 10:47 ` Sergey Kaplun via Tarantool-patches 2023-08-08 19:25 ` Sergey Bronnikov via Tarantool-patches 2023-08-14 7:46 ` Maxim Kokryashkin via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox