From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Sergey Kaplun <skaplun@tarantool.org> Cc: Sergey Bronnikov <estetus@gmail.com>, max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Subject: Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support Date: Mon, 7 Aug 2023 16:39:27 +0300 [thread overview] Message-ID: <71807b71-5445-1ce9-f073-a708f87eb29b@tarantool.org> (raw) In-Reply-To: <ZM-FZ83YIj26Hpn3@root> Hello, Sergey! On 8/6/23 14:35, Sergey Kaplun wrote: > Hi, Sergey! > Thanks for the patch! > LGTM, just a minor nits below. > > On 02.08.23, Sergey Bronnikov via Tarantool-patches wrote: >> Hi, Max >> >> On 8/2/23 11:06, Maxim Kokryashkin via Tarantool-patches wrote: >>> Hi, Sergey! >>> Thanks for the fixes! >>> LGTM, except for a few comments below. >>> >>> Side note: I see that coverage job in CI is red. Why is that >>> happening? >> This happened because from time to time total code coverage number >> changes a bit. >> >> It is really annoying, to solve this we need to increase the threshold >> in Coveralls service. > I see that now this job is green. Was it fixed? Actually no. I'll ask someone who has access to settings to increase threshold. > >> >> >>> On Tue, Aug 01, 2023 at 09:46:08PM +0300, Sergey Bronnikov via Tarantool-patches 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]. There were two new CMake >>>> targets added: LuaJIT-coverage proccess *.gcno and *.gcda files with >>> Typo: s/process/processes/ >> Fixed. >>>> gcov, builds a detailed HTML report and prints a summary, target >>>> coverage executes LuaJIT-tests and then runs LuaJIT-coverage. Target >>>> LuaJIT-coverage is useful for building code coverage report for a custom >>>> set of regression tests. >>>> >>>> ``` >>>> $ cmake -S . -B build -DENABLE_COVERAGE=ON >>>> $ cmake --build build --parallel --target 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 ++++++ >>>> cmake/CodeCoverage.cmake | 45 +++++++++++++++++++++++++++ >>>> test/CMakeLists.txt | 7 +++++ >>>> test/tarantool-c-tests/CMakeLists.txt | 6 +++- >>>> 4 files changed, 66 insertions(+), 1 deletion(-) >>>> create mode 100644 cmake/CodeCoverage.cmake >>>> >>>> diff --git a/CMakeLists.txt b/CMakeLists.txt >>>> index 6ef24bba..fe6582fa 100644 >>>> --- a/CMakeLists.txt >>>> +++ b/CMakeLists.txt >>>> @@ -116,6 +116,15 @@ if(LUAJIT_ENABLE_WARNINGS) >>>> ) >>>> endif() >>>> > I suggest to add comment here, that the user should run tests _before_ > coverage report, or this may be confusing (yes, I'm this user :)): > > | $ make LuaJIT-coverage > | Building coverage report > | lines: 0.0% (0 out of 23883) > | functions: 0.0% (0 out of 1765) > | branches: 0.0% (0 out of 17131) > | Built target LuaJIT-coverage The difference for LuaJIT-coverage and coverage targets is described in commit message. Comment is already there: > add_custom_command(TARGET ${PROJECT_NAME}-coverage > COMMENT "Building coverage report" > >>>> +set(LUAJIT_ENABLE_COVERAGE_DEFAULT OFF) >>>> +option(LUAJIT_ENABLE_COVERAGE >>>> + "Enable integration with gcovr, a code coverage program" >>>> + ${LUAJIT_ENABLE_COVERAGE_DEFAULT}) >>>> +if (LUAJIT_ENABLE_COVERAGE) >>>> + AppendFlags(CMAKE_C_FLAGS --coverage) >>>> + include(CodeCoverage) >>>> +endif(LUAJIT_ENABLE_COVERAGE) >>>> + >>>> # Auxiliary flags for main targets (libraries, binaries). >>>> AppendFlags(TARGET_C_FLAGS >>>> -D_FILE_OFFSET_BITS=64 >>>> diff --git a/cmake/CodeCoverage.cmake b/cmake/CodeCoverage.cmake >>>> new file mode 100644 >>>> index 00000000..2be7d129 >>>> --- /dev/null >>>> +++ b/cmake/CodeCoverage.cmake >>>> @@ -0,0 +1,45 @@ >>>> +find_program(GCOVR gcovr) >>>> +find_program(GCOV gcov) >>>> + >>>> +set(COVERAGE_DIR "${PROJECT_BINARY_DIR}/coverage") >>>> +set(COVERAGE_HTML_REPORT "${COVERAGE_DIR}/luajit.html") >>>> +set(COVERAGE_XML_REPORT "${COVERAGE_DIR}/luajit.xml") >>>> + >>>> +if(NOT GCOVR OR NOT GCOV) >>>> + add_custom_target(${PROJECT_NAME}-coverage >>>> + COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red "LuaJIT-coverage is a dummy target" > I suggest to split this line into several too. splitted > >>>> + ) >>>> + message(WARNING "Either `gcovr' or `gcov` not found, \ >>>> +so ${PROJECT_NAME}-coverage target is dummy") >>> Nit: Something is wrong with alignment here. >> No, it is intentionally. If you add indentation then these whitespaces >> will be added to a message. > Works just fine with the following diff for me: > > =================================================================== > diff --git a/cmake/CodeCoverage.cmake b/cmake/CodeCoverage.cmake > index 2be7d129..83e23d7f 100644 > --- a/cmake/CodeCoverage.cmake > +++ b/cmake/CodeCoverage.cmake > @@ -9,8 +9,8 @@ if(NOT GCOVR OR NOT GCOV) > add_custom_target(${PROJECT_NAME}-coverage > COMMAND ${CMAKE_COMMAND} -E cmake_echo_color --red "LuaJIT-coverage is a dummy target" > ) > - message(WARNING "Either `gcovr' or `gcov` not found, \ > -so ${PROJECT_NAME}-coverage target is dummy") > + message(WARNING "Either `gcovr' or `gcov` not found, " > + "so ${PROJECT_NAME}-coverage target is dummy") > return() > endif() > > =================================================================== Applied, thanks! > > <snipped> > >> # Exclude DynASM files, that contain a low-level VM code for CPUs. >> --exclude ".*\.dasc" >> # Exclude buildvm source code, it's the project's infrastructure. >> --exclude ".*/host/" > Why don't use ${PROJECT_SOURCE_DIR} instead of .* here? It is not needed here. gcovr searches *.gcda/*.gcno files in PROJECT_BINARY_DIRECTORY and additionally all paths excluded except PROJECT_SOURCE_DIR/src. So absolute path is excessive in regexes specified in --exclude options. > > <snipped> >
next prev parent reply other threads:[~2023-08-07 13:39 UTC|newest] Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top 2023-08-01 18:46 [Tarantool-patches] [PATCH 0/2 v2] Add " Sergey Bronnikov via Tarantool-patches 2023-08-01 18:46 ` [Tarantool-patches] [PATCH 1/2 v2] cmake: add " Sergey Bronnikov via Tarantool-patches 2023-08-02 8:06 ` Maxim Kokryashkin via Tarantool-patches 2023-08-02 8:18 ` Sergey Bronnikov via Tarantool-patches 2023-08-06 11:35 ` Sergey Kaplun via Tarantool-patches 2023-08-07 13:39 ` Sergey Bronnikov via Tarantool-patches [this message] 2023-08-15 8:41 ` Maxim Kokryashkin via Tarantool-patches 2023-08-01 18:46 ` [Tarantool-patches] [PATCH 2/2 v2] ci: support coveralls Sergey Bronnikov via Tarantool-patches 2023-08-02 8:18 ` Maxim Kokryashkin via Tarantool-patches 2023-08-02 8:20 ` Sergey Bronnikov via Tarantool-patches 2023-08-06 11:41 ` Sergey Kaplun via Tarantool-patches 2023-08-07 11:32 ` Sergey Bronnikov via Tarantool-patches 2023-08-21 11:05 ` [Tarantool-patches] [PATCH 0/2 v2] Add code coverage support Igor Munkin via Tarantool-patches
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=71807b71-5445-1ce9-f073-a708f87eb29b@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=estetus@gmail.com \ --cc=max.kokryashkin@gmail.com \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox