From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id D45B85802D5; Mon, 7 Aug 2023 16:39:29 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D45B85802D5 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1691415570; bh=0LoD+E+zJ8eXtXfEbdvxeWJySRG68RBsaGabn4IWbhI=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=NYYmnRRSb0vvBY+pVgLT6NusxO4B1zmQkGaunrM4jI8fmUUD22fNqJkj37Ad8TLTb NsxJLEGQxYxM1jtzDQzadjx9ACJsFxxMQStDon9VtbJVvP57xKAlmOUs3hCxxwZfBf c3JfhoxrIaZMRel1BTXvb4ls2JfBzMYzQn3fu8Jo= Received: from smtpng2.i.mail.ru (smtpng2.i.mail.ru [94.100.179.3]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id DCBAD57FBB4 for ; Mon, 7 Aug 2023 16:39:28 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org DCBAD57FBB4 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1qT0SF-0003IF-W5; Mon, 07 Aug 2023 16:39:28 +0300 Message-ID: <71807b71-5445-1ce9-f073-a708f87eb29b@tarantool.org> Date: Mon, 7 Aug 2023 16:39:27 +0300 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.13.0 Content-Language: en-US To: Sergey Kaplun References: <27qwncp6oubrmexp6bwsur5nnm5xnlxaby3slxk4rrk2beyuin@5wtqxgcetnfk> <63fb786f-b593-6da6-78b6-cfd9b2435740@tarantool.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD94DC436DAD9FF86068A8B70255D5151F61D5B4853E07EEE2F182A05F5380850404C228DA9ACA6FE2701769F0E492CD2F7382B677E596ECF7962DDC83C8E2C58FE186398615D5DF29E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7C27E92EFAD44F80DEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637D08E1E5B2BD3D3B78638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8C3D27A8003886FFAF2201110D6C9C159117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3A703B70628EAD7BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735200AC5B80A05675ACDF04B652EEC242312D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BA83251EDC214901ED5E8D9A59859A8B68A0907C648FB30DE089D37D7C0E48F6C5571747095F342E88FB05168BE4CE3AF X-C1DE0DAB: 0D63561A33F958A5100558EF4161C31976889ED789FDEA0F10A7332A0F40D1CAF87CCE6106E1FC07E67D4AC08A07B9B0B355ED1E20F5346ACB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF651F3AC48992BA7945EFB7215C6FFFF4EAAD3D31F68CA8671A44B8447D7AB13260B027A3EDF90BC11529AA6C8ABD04CABD7D0421BB17AEDA81A3D74360046275461A413F07889F2102C26D483E81D6BE0DBAE6F56676BC7117BB6831D7356A2DEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj/xSHLV4ZQPugW2Tlvqgptw== X-DA7885C5: 23307C6AF5EDA05C12E11D97D57573A791E852BBDA5DEA8CE2D03F24B0E7F800262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986BEBC2DCA87BEFF3B7C23E82EE14B404F1DD788429FD8613638ED9BB8B05EE7B3AFB559BB5D741EB96D19CD4E7312BAA970A04DAD6CC59E3365FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: Sergey Bronnikov , max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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 >>>> >>>> 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 >>>> >>>> >>>> >>>> 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! > > > >> # 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. > > >