[Tarantool-patches] [PATCH 0/2 v2] Add code coverage support

[Tarantool-patches] [PATCH 0/2 v2] Add code coverage support

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Patch-series adds code coverage support to LuaJIT build infrastructure
and Github Actions.

Changes v2:
- addressed comments from Max Kokryashkin and Sergey Kaplun
- added integration with Coveralls

Branch: https://github.com/tarantool/luajit/tree/ligurio/code-coverage
Coveralls: https://coveralls.io/github/tarantool/luajit
Patch v1: https://lists.tarantool.org/tarantool-patches/58297844-8590-cf70-b3d5-3ed8908aef9a@tarantool.org/T/#t

Sergey Bronnikov (2):
  cmake: add code coverage support
  ci: support coveralls

 .github/actions/setup-linux/action.yml |  1 +
 .github/workflows/coverage.yml         | 60 ++++++++++++++++++++++++++
 CMakeLists.txt                         |  9 ++++
 cmake/CodeCoverage.cmake               | 45 +++++++++++++++++++
 test/CMakeLists.txt                    |  7 +++
 test/tarantool-c-tests/CMakeLists.txt  |  6 ++-
 6 files changed, 127 insertions(+), 1 deletion(-)
 create mode 100644 .github/workflows/coverage.yml
 create mode 100644 cmake/CodeCoverage.cmake

-- 
2.34.1

[Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support

[Sergey Bronnikov via Tarantool-patches]

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
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()
 
+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"
+  )
+  message(WARNING "Either `gcovr' or `gcov` not found, \
+so ${PROJECT_NAME}-coverage target is dummy")
+  return()
+endif()
+
+file(MAKE_DIRECTORY ${COVERAGE_DIR})
+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}
+      --object-directory ${PROJECT_BINARY_DIR}
+      --filter ${PROJECT_SOURCE_DIR}/src
+      # 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/"
+      --print-summary
+      --output ${COVERAGE_HTML_REPORT}
+      --cobertura ${COVERAGE_XML_REPORT}
+      --html
+      --html-title "Tarantool LuaJIT Code Coverage Report"
+      --html-details
+      --sort-percentage
+      --branches
+      --decisions
+      -j ${CMAKE_BUILD_PARALLEL_LEVEL}
+  WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+)
+
+message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}")
+message(STATUS "Code coverage XML report: ${COVERAGE_XML_REPORT}")
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 47296a22..e23d6d45 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -76,4 +76,11 @@ 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()
diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
index 17255345..d74e99fc 100644
--- 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})
+  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()
 
-- 
2.34.1

Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support

[Maxim Kokryashkin via Tarantool-patches]

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?

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/
> 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()
>  
> +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"
> +  )
> +  message(WARNING "Either `gcovr' or `gcov` not found, \
> +so ${PROJECT_NAME}-coverage target is dummy")
Nit: Something is wrong with alignment here.
> +  return()
> +endif()
> +
> +file(MAKE_DIRECTORY ${COVERAGE_DIR})
> +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}
> +      --object-directory ${PROJECT_BINARY_DIR}
> +      --filter ${PROJECT_SOURCE_DIR}/src
> +      # 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/"
> +      --print-summary
> +      --output ${COVERAGE_HTML_REPORT}
> +      --cobertura ${COVERAGE_XML_REPORT}
> +      --html
> +      --html-title "Tarantool LuaJIT Code Coverage Report"
> +      --html-details
> +      --sort-percentage
> +      --branches
> +      --decisions
> +      -j ${CMAKE_BUILD_PARALLEL_LEVEL}
> +  WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
> +)
> +
> +message(STATUS "Code coverage HTML report: ${COVERAGE_HTML_REPORT}")
> +message(STATUS "Code coverage XML report: ${COVERAGE_XML_REPORT}")
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 47296a22..e23d6d45 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -76,4 +76,11 @@ 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()
> diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
> index 17255345..d74e99fc 100644
> --- 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})
> +  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()
>  
> -- 
> 2.34.1
> 

Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support

[Sergey Bronnikov via Tarantool-patches]

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.



>
> 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()
>>   
>> +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"
>> +  )
>> +  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.
<snipped>
>>

Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support

[Sergey Kaplun via 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 <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

> >> +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()
 
===================================================================

<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?

<snipped>

> >>

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support

[Sergey Bronnikov via 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 <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>



>

Re: [Tarantool-patches] [PATCH 1/2 v2] cmake: add code coverage support

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the fixes!
LGTM, except for a single comment below.

On Mon, Aug 07, 2023 at 04:39:27PM +0300, Sergey Bronnikov wrote:
> 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)

I believe it would be better to do that in the `test/CMakeLists.txt`
instead of the main one, since coverage is semantically relevant to tests.
Feel free to ignore.

> > > > > +
> > > > >    # 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>
> 
> 
> 
> > 

[Tarantool-patches] [PATCH 2/2 v2] ci: support coveralls

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

The patch adds a workflow that runs regression test suites, produces a
summary about current code coverage and send code coverage data to
Coveralls. Coveralls is a web-service that lets you inspect every detail
of your coverage. See Tarantool's LuaJIT page on Coveralls [1].

1. https://coveralls.io/github/tarantool/luajit
---
 .github/actions/setup-linux/action.yml |  1 +
 .github/workflows/coverage.yml         | 60 ++++++++++++++++++++++++++
 2 files changed, 61 insertions(+)
 create mode 100644 .github/workflows/coverage.yml

diff --git a/.github/actions/setup-linux/action.yml b/.github/actions/setup-linux/action.yml
index f0171b83..71619a60 100644
--- a/.github/actions/setup-linux/action.yml
+++ b/.github/actions/setup-linux/action.yml
@@ -16,4 +16,5 @@ runs:
       run: |
         apt -y update
         apt -y install cmake gcc make ninja-build perl
+        pip3 install gcovr
       shell: bash
diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
new file mode 100644
index 00000000..9fff06c7
--- /dev/null
+++ b/.github/workflows/coverage.yml
@@ -0,0 +1,60 @@
+name: Code coverage
+
+on:
+  push:
+    branches-ignore:
+      - '**-notest'
+      - 'upstream-**'
+    tags-ignore:
+      - '**'
+
+concurrency:
+  # An update of a developer branch cancels the previously
+  # scheduled workflow run for this branch. However, the default
+  # branch, and long-term branch (tarantool/release/2.11,
+  # tarantool/release/2.10, etc) workflow runs are never canceled.
+  #
+  # We use a trick here: define the concurrency group as 'workflow
+  # run ID' + # 'workflow run attempt' because it is a unique
+  # combination for any run. So it effectively discards grouping.
+  #
+  # XXX: we cannot use `github.sha` as a unique identifier because
+  # pushing a tag may cancel a run that works on a branch push
+  # event.
+  group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
+    && format('{0}-{1}', github.run_id, github.run_attempt)
+    || format('{0}-{1}', github.workflow, github.ref) }}
+  cancel-in-progress: true
+
+jobs:
+  coverage:
+    strategy:
+      fail-fast: false
+    runs-on: [self-hosted, regular, x86_64, Linux]
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          fetch-depth: 0
+          submodules: recursive
+      - name: setup Linux
+        uses: ./.github/actions/setup-linux
+      - name: configure
+        run: >
+          cmake -S . -B ${{ env.BUILDDIR }}
+          -G Ninja
+          -DCMAKE_BUILD_TYPE=RelWithDebInfo
+          -DLUAJIT_ENABLE_COVERAGE=ON
+          -DLUAJIT_ENABLE_GC64=ON
+      - 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: send code coverage to coveralls.io
+        run: |
+          curl -LO https://coveralls.io/coveralls-linux.tar.gz
+          tar xvzf coveralls-linux.tar.gz
+          ./coveralls -f ./coverage/luajit.xml
+        working-directory: ${{ env.BUILDDIR }}
+        env:
+          COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-- 
2.34.1

Re: [Tarantool-patches] [PATCH 2/2 v2] ci: support coveralls

[Maxim Kokryashkin via Tarantool-patches]

Hi, Sergey!
Thanks for the patch!
LGTM, except for a few nits regarding the commit message.

Best regards,
Maxim Kokryashkin

On Tue, Aug 01, 2023 at 09:46:10PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> The patch adds a workflow that runs regression test suites, produces a
> summary about current code coverage and send code coverage data to
Typo: s/about/of/
Typo: s/send code/sends code/
> Coveralls. Coveralls is a web-service that lets you inspect every detail
Typo: s/web-service/web service/
> of your coverage. See Tarantool's LuaJIT page on Coveralls [1].
> 
> 1. https://coveralls.io/github/tarantool/luajit
> ---
>  .github/actions/setup-linux/action.yml |  1 +
>  .github/workflows/coverage.yml         | 60 ++++++++++++++++++++++++++
>  2 files changed, 61 insertions(+)
>  create mode 100644 .github/workflows/coverage.yml
> 
> diff --git a/.github/actions/setup-linux/action.yml b/.github/actions/setup-linux/action.yml
> index f0171b83..71619a60 100644
> --- a/.github/actions/setup-linux/action.yml
> +++ b/.github/actions/setup-linux/action.yml
> @@ -16,4 +16,5 @@ runs:
>        run: |
>          apt -y update
>          apt -y install cmake gcc make ninja-build perl
> +        pip3 install gcovr
>        shell: bash
> diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
> new file mode 100644
> index 00000000..9fff06c7
> --- /dev/null
> +++ b/.github/workflows/coverage.yml
> @@ -0,0 +1,60 @@
> +name: Code coverage
> +
> +on:
> +  push:
> +    branches-ignore:
> +      - '**-notest'
> +      - 'upstream-**'
> +    tags-ignore:
> +      - '**'
> +
> +concurrency:
> +  # An update of a developer branch cancels the previously
> +  # scheduled workflow run for this branch. However, the default
> +  # branch, and long-term branch (tarantool/release/2.11,
> +  # tarantool/release/2.10, etc) workflow runs are never canceled.
> +  #
> +  # We use a trick here: define the concurrency group as 'workflow
> +  # run ID' + # 'workflow run attempt' because it is a unique
> +  # combination for any run. So it effectively discards grouping.
> +  #
> +  # XXX: we cannot use `github.sha` as a unique identifier because
> +  # pushing a tag may cancel a run that works on a branch push
> +  # event.
> +  group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
> +    && format('{0}-{1}', github.run_id, github.run_attempt)
> +    || format('{0}-{1}', github.workflow, github.ref) }}
> +  cancel-in-progress: true
> +
> +jobs:
> +  coverage:
> +    strategy:
> +      fail-fast: false
> +    runs-on: [self-hosted, regular, x86_64, Linux]
> +    steps:
> +      - uses: actions/checkout@v3
> +        with:
> +          fetch-depth: 0
> +          submodules: recursive
> +      - name: setup Linux
> +        uses: ./.github/actions/setup-linux
> +      - name: configure
> +        run: >
> +          cmake -S . -B ${{ env.BUILDDIR }}
> +          -G Ninja
> +          -DCMAKE_BUILD_TYPE=RelWithDebInfo
> +          -DLUAJIT_ENABLE_COVERAGE=ON
> +          -DLUAJIT_ENABLE_GC64=ON
> +      - 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: send code coverage to coveralls.io
> +        run: |
> +          curl -LO https://coveralls.io/coveralls-linux.tar.gz
> +          tar xvzf coveralls-linux.tar.gz
> +          ./coveralls -f ./coverage/luajit.xml
> +        working-directory: ${{ env.BUILDDIR }}
> +        env:
> +          COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
> -- 
> 2.34.1
> 

Re: [Tarantool-patches] [PATCH 2/2 v2] ci: support coveralls

[Sergey Bronnikov via Tarantool-patches]

Hi, Max

On 8/2/23 11:18, Maxim Kokryashkin via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few nits regarding the commit message.
>
> Best regards,
> Maxim Kokryashkin
>
> On Tue, Aug 01, 2023 at 09:46:10PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> The patch adds a workflow that runs regression test suites, produces a
>> summary about current code coverage and send code coverage data to
> Typo: s/about/of/
> Typo: s/send code/sends code/

Fixed!


>> Coveralls. Coveralls is a web-service that lets you inspect every detail
> Typo: s/web-service/web service/

Fixed.


<snipped>

Re: [Tarantool-patches] [PATCH 2/2 v2] ci: support coveralls

[Sergey Kaplun via Tarantool-patches]

Hi, Maxim, Sergey!
Thanks for the patch and the review!
LGTM, just a minor comments below.

On 02.08.23, Maxim Kokryashkin wrote:
> Hi, Sergey!
> Thanks for the patch!
> LGTM, except for a few nits regarding the commit message.
> 
> Best regards,
> Maxim Kokryashkin
> 
> On Tue, Aug 01, 2023 at 09:46:10PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
> > From: Sergey Bronnikov <sergeyb@tarantool.org>
> > 
> > The patch adds a workflow that runs regression test suites, produces a
> > summary about current code coverage and send code coverage data to
> Typo: s/about/of/
> Typo: s/send code/sends code/
> > Coveralls. Coveralls is a web-service that lets you inspect every detail
> Typo: s/web-service/web service/
> > of your coverage. See Tarantool's LuaJIT page on Coveralls [1].
> > 
> > 1. https://coveralls.io/github/tarantool/luajit
> > ---
> >  .github/actions/setup-linux/action.yml |  1 +
> >  .github/workflows/coverage.yml         | 60 ++++++++++++++++++++++++++
> >  2 files changed, 61 insertions(+)
> >  create mode 100644 .github/workflows/coverage.yml
> > 
> > diff --git a/.github/actions/setup-linux/action.yml b/.github/actions/setup-linux/action.yml
> > index f0171b83..71619a60 100644
> > --- a/.github/actions/setup-linux/action.yml
> > +++ b/.github/actions/setup-linux/action.yml
> > @@ -16,4 +16,5 @@ runs:
> >        run: |
> >          apt -y update
> >          apt -y install cmake gcc make ninja-build perl
> > +        pip3 install gcovr

Should it be done in the separate action, like it is done for ASan?
Because we don't need gcovr for all our linux testing.

> >        shell: bash
> > diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
> > new file mode 100644
> > index 00000000..9fff06c7
> > --- /dev/null
> > +++ b/.github/workflows/coverage.yml
> > @@ -0,0 +1,60 @@
> > +name: Code coverage
> > +
> > +on:
> > +  push:
> > +    branches-ignore:
> > +      - '**-notest'
> > +      - 'upstream-**'
> > +    tags-ignore:
> > +      - '**'
> > +
> > +concurrency:
> > +  # An update of a developer branch cancels the previously
> > +  # scheduled workflow run for this branch. However, the default
> > +  # branch, and long-term branch (tarantool/release/2.11,
> > +  # tarantool/release/2.10, etc) workflow runs are never canceled.
> > +  #
> > +  # We use a trick here: define the concurrency group as 'workflow
> > +  # run ID' + # 'workflow run attempt' because it is a unique
> > +  # combination for any run. So it effectively discards grouping.
> > +  #
> > +  # XXX: we cannot use `github.sha` as a unique identifier because
> > +  # pushing a tag may cancel a run that works on a branch push
> > +  # event.
> > +  group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
> > +    && format('{0}-{1}', github.run_id, github.run_attempt)
> > +    || format('{0}-{1}', github.workflow, github.ref) }}
> > +  cancel-in-progress: true
> > +
> > +jobs:
> > +  coverage:
> > +    strategy:
> > +      fail-fast: false
> > +    runs-on: [self-hosted, regular, x86_64, Linux]
> > +    steps:
> > +      - uses: actions/checkout@v3
> > +        with:
> > +          fetch-depth: 0
> > +          submodules: recursive
> > +      - name: setup Linux
> > +        uses: ./.github/actions/setup-linux
> > +      - name: configure
> > +        run: >
> > +          cmake -S . -B ${{ env.BUILDDIR }}
> > +          -G Ninja
> > +          -DCMAKE_BUILD_TYPE=RelWithDebInfo
> > +          -DLUAJIT_ENABLE_COVERAGE=ON
> > +          -DLUAJIT_ENABLE_GC64=ON

Is there a way to joing GC64/non-GC64 testsing?
Same for ARM64.

> > +      - 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

I see no test target here, so will we get the correct coverage results?
Am I missing something? If yes, than comment is desirable.

> > +      - name: send code coverage to coveralls.io
> > +        run: |
> > +          curl -LO https://coveralls.io/coveralls-linux.tar.gz
> > +          tar xvzf coveralls-linux.tar.gz
> > +          ./coveralls -f ./coverage/luajit.xml
> > +        working-directory: ${{ env.BUILDDIR }}
> > +        env:
> > +          COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
> > -- 
> > 2.34.1
> > 

-- 
Best regards,
Sergey Kaplun

Re: [Tarantool-patches] [PATCH 2/2 v2] ci: support coveralls

[Sergey Bronnikov via Tarantool-patches]

Hi, Sergey

thanks for a long-awaited review!

On 8/6/23 14:41, Sergey Kaplun via Tarantool-patches wrote:
> Hi, Maxim, Sergey!
> Thanks for the patch and the review!
> LGTM, just a minor comments below.
>
> On 02.08.23, Maxim Kokryashkin wrote:
>> Hi, Sergey!
>> Thanks for the patch!
>> LGTM, except for a few nits regarding the commit message.
>>
>> Best regards,
>> Maxim Kokryashkin
>>
>> On Tue, Aug 01, 2023 at 09:46:10PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
>>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>>
>>> The patch adds a workflow that runs regression test suites, produces a
>>> summary about current code coverage and send code coverage data to
>> Typo: s/about/of/
>> Typo: s/send code/sends code/
>>> Coveralls. Coveralls is a web-service that lets you inspect every detail
>> Typo: s/web-service/web service/
>>> of your coverage. See Tarantool's LuaJIT page on Coveralls [1].
>>>
>>> 1. https://coveralls.io/github/tarantool/luajit
>>> ---
>>>   .github/actions/setup-linux/action.yml |  1 +
>>>   .github/workflows/coverage.yml         | 60 ++++++++++++++++++++++++++
>>>   2 files changed, 61 insertions(+)
>>>   create mode 100644 .github/workflows/coverage.yml
>>>
>>> diff --git a/.github/actions/setup-linux/action.yml b/.github/actions/setup-linux/action.yml
>>> index f0171b83..71619a60 100644
>>> --- a/.github/actions/setup-linux/action.yml
>>> +++ b/.github/actions/setup-linux/action.yml
>>> @@ -16,4 +16,5 @@ runs:
>>>         run: |
>>>           apt -y update
>>>           apt -y install cmake gcc make ninja-build perl
>>> +        pip3 install gcovr
> Should it be done in the separate action, like it is done for ASan?
> Because we don't need gcovr for all our linux testing.

Well, added a separate GH action for that.


>
>>>         shell: bash
>>> diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml
>>> new file mode 100644
>>> index 00000000..9fff06c7
>>> --- /dev/null
>>> +++ b/.github/workflows/coverage.yml
>>> @@ -0,0 +1,60 @@
>>> +name: Code coverage
>>> +
>>> +on:
>>> +  push:
>>> +    branches-ignore:
>>> +      - '**-notest'
>>> +      - 'upstream-**'
>>> +    tags-ignore:
>>> +      - '**'
>>> +
>>> +concurrency:
>>> +  # An update of a developer branch cancels the previously
>>> +  # scheduled workflow run for this branch. However, the default
>>> +  # branch, and long-term branch (tarantool/release/2.11,
>>> +  # tarantool/release/2.10, etc) workflow runs are never canceled.
>>> +  #
>>> +  # We use a trick here: define the concurrency group as 'workflow
>>> +  # run ID' + # 'workflow run attempt' because it is a unique
>>> +  # combination for any run. So it effectively discards grouping.
>>> +  #
>>> +  # XXX: we cannot use `github.sha` as a unique identifier because
>>> +  # pushing a tag may cancel a run that works on a branch push
>>> +  # event.
>>> +  group: ${{ startsWith(github.ref, 'refs/heads/tarantool/')
>>> +    && format('{0}-{1}', github.run_id, github.run_attempt)
>>> +    || format('{0}-{1}', github.workflow, github.ref) }}
>>> +  cancel-in-progress: true
>>> +
>>> +jobs:
>>> +  coverage:
>>> +    strategy:
>>> +      fail-fast: false
>>> +    runs-on: [self-hosted, regular, x86_64, Linux]
>>> +    steps:
>>> +      - uses: actions/checkout@v3
>>> +        with:
>>> +          fetch-depth: 0
>>> +          submodules: recursive
>>> +      - name: setup Linux
>>> +        uses: ./.github/actions/setup-linux
>>> +      - name: configure
>>> +        run: >
>>> +          cmake -S . -B ${{ env.BUILDDIR }}
>>> +          -G Ninja
>>> +          -DCMAKE_BUILD_TYPE=RelWithDebInfo
>>> +          -DLUAJIT_ENABLE_COVERAGE=ON
>>> +          -DLUAJIT_ENABLE_GC64=ON
> Is there a way to joing GC64/non-GC64 testsing?
> Same for ARM64.

Yes, there is a thing named "parallel build" that allows reporting

code coverage data from a number of CI jobs, see [1].

We definitely need this for jobs with tests that covers non-common parts 
of code like GC64/ARM64 etc,

but in scope of other patch series.


1. https://docs.coveralls.io/parallel-builds#whats-a-parallel-build


>>> +      - 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
> I see no test target here, so will we get the correct coverage results?
> Am I missing something? If yes, than comment is desirable.

- target "LuaJIT-coverage" runs gcov and gcovr and builds a code 
coverage report

- target "coverage" connected to "test" (running tests) and 
"LuaJIT-coverage" (builds code coverage report)

I suppose comment is not required here.

>
>>> +      - name: send code coverage to coveralls.io
>>> +        run: |
>>> +          curl -LO https://coveralls.io/coveralls-linux.tar.gz
>>> +          tar xvzf coveralls-linux.tar.gz
>>> +          ./coveralls -f ./coverage/luajit.xml
>>> +        working-directory: ${{ env.BUILDDIR }}
>>> +        env:
>>> +          COVERALLS_REPO_TOKEN: ${{ secrets.GITHUB_TOKEN }}
>>> -- 
>>> 2.34.1
>>>

Re: [Tarantool-patches] [PATCH 0/2 v2] Add code coverage support

[Igor Munkin via Tarantool-patches]

Sergey,

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 01.08.23, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Patch-series adds code coverage support to LuaJIT build infrastructure
> and Github Actions.
> 
> Changes v2:
> - addressed comments from Max Kokryashkin and Sergey Kaplun
> - added integration with Coveralls
> 
> Branch: https://github.com/tarantool/luajit/tree/ligurio/code-coverage
> Coveralls: https://coveralls.io/github/tarantool/luajit
> Patch v1: https://lists.tarantool.org/tarantool-patches/58297844-8590-cf70-b3d5-3ed8908aef9a@tarantool.org/T/#t
> 
> Sergey Bronnikov (2):
>   cmake: add code coverage support
>   ci: support coveralls
> 
>  .github/actions/setup-linux/action.yml |  1 +
>  .github/workflows/coverage.yml         | 60 ++++++++++++++++++++++++++
>  CMakeLists.txt                         |  9 ++++
>  cmake/CodeCoverage.cmake               | 45 +++++++++++++++++++
>  test/CMakeLists.txt                    |  7 +++
>  test/tarantool-c-tests/CMakeLists.txt  |  6 ++-
>  6 files changed, 127 insertions(+), 1 deletion(-)
>  create mode 100644 .github/workflows/coverage.yml
>  create mode 100644 cmake/CodeCoverage.cmake
> 
> -- 
> 2.34.1
> 

-- 
Best regards,
IM