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 E335E597C0C; Sun, 6 Aug 2023 14:39:52 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E335E597C0C DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1691321993; bh=PoTIYE/2d1C5xEj8I/QILGVO6z4yClB2pHIZjvwbygE=; 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=ETV8f7ZHmJvwdGTgxEZfLN23nat728P644KMfnWGUnCzJUtrHG+Wt3V36cv1Oqqeb +7THGtS4jog2TbZbdC78GElxG7PxTTBCCTB3Nd5srxlQtMPl/TdCsIoAHmzCfJsKHU pSn7eAp46hdujrWKXoTxsSlb8739mzx036I4LVZc= Received: from smtpng3.i.mail.ru (smtpng3.i.mail.ru [94.100.177.149]) (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 E54B9597C0C for ; Sun, 6 Aug 2023 14:39:50 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E54B9597C0C Received: by smtpng3.m.smailru.net with esmtpa (envelope-from ) id 1qSc6v-0002sH-WF; Sun, 06 Aug 2023 14:39:50 +0300 Date: Sun, 6 Aug 2023 14:35:03 +0300 To: Sergey Bronnikov Message-ID: References: <27qwncp6oubrmexp6bwsur5nnm5xnlxaby3slxk4rrk2beyuin@5wtqxgcetnfk> <63fb786f-b593-6da6-78b6-cfd9b2435740@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <63fb786f-b593-6da6-78b6-cfd9b2435740@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD94DC436DAD9FF8606346AF824961DF68B0E5570B41D736741182A05F5380850404C228DA9ACA6FE27BDEA943F6FECC2635B91A3F926AE466EAB4DDC37D58BC0B25D563E286E6EA0C9 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE755BE8F535441E38CEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637593B4F2A76947A308638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8CF9C276E8E91139873E19030DFE6390A117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF80095D1E57F4578A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F4460429728776938767073520C65AC60A1F0286FE2CC0D3CB04F14752D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269A417C69337E82CC2E827F84554CEF50127C277FBC8AE2E8BF1175FABE1C0F9B6AAAE862A0553A39223F8577A6DFFEA7C1156E5889A6D309143847C11F186F3C59DAA53EE0834AAEE X-C1DE0DAB: 0D63561A33F958A5277666F318AB663BF440D46F6081E8CD949BDA4FF2B88B8CF87CCE6106E1FC07E67D4AC08A07B9B01DAA61796BF5227BCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF6BDC569597F08EF6FE8310161B0B02FED449F55702164B90515B0CA17DDC2735F2F1C324C53BDAF26CFA265C340F28BAD93F7AF7AFAF65B5B3AB7C43733583D8461A413F07889F2102C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojl9DvqjyDU9PlN/7AwTzm9A== X-DA7885C5: 7024BF2DADC2915A85299F245C1CDD06F7570F7568B589E519C175EFA57C1230262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986B0F60FE8890B44BD5A83D28A60297DF760FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: Sergey Bronnikov , max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" 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? > > > > > > > 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 > >> +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. > >> + ) > >> + 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() =================================================================== > # 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? > >> -- Best regards, Sergey Kaplun