Add ctest Coverage report - #674
Conversation
|
Function Coverage: 42.04% (758/1803) |
There was a problem hiding this comment.
3 issues found across 5 files
You’re at about 90% of the monthly reviewed-line limit. You may want to disable incremental reviews to conserve quota. Reviews will continue until that limit is exceeded. If you need help avoiding interruptions, please contact contact@cubic.dev.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/CMakeLists.txt">
<violation number="1" location="server/CMakeLists.txt:1945">
P1: When a device-backed test has a renamed or discovered CTest name, this regex does not exclude it. CTest then runs an executable that `coverage` deliberately did not build; exclude the actual names, including `${target}.*` discovered tests.</violation>
<violation number="2" location="server/CMakeLists.txt:1987">
P1: On a fresh build, `coverage` does not build every executable that its CTest invocation runs. Add every executable-backed CTest target to the coverage dependencies, or create the coverage target after the complete test target list is known.</violation>
</file>
<file name="server/cmake/RunCoverageTests.cmake">
<violation number="1" location="server/cmake/RunCoverageTests.cmake:22">
P3: When ctest fails in a way that produces no profiles, GenerateCoverageReport.cmake aborts first with "CTest produced no LLVM profiles", so the real ctest exit code and this script's "CTest failed" message are never surfaced. Check `_coverage_ctest_result` immediately after execute_process and only run the report generation when ctest succeeded, or at least report the ctest failure before including the generator.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| "-DSOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR}/src" | ||
| "-DHTML_DIR=${_coverage_report_dir}/html" | ||
| -P "${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCoverageTests.cmake" | ||
| DEPENDS ${_coverage_deps} |
There was a problem hiding this comment.
P1: On a fresh build, coverage does not build every executable that its CTest invocation runs. Add every executable-backed CTest target to the coverage dependencies, or create the coverage target after the complete test target list is known.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/CMakeLists.txt, line 1987:
<comment>On a fresh build, `coverage` does not build every executable that its CTest invocation runs. Add every executable-backed CTest target to the coverage dependencies, or create the coverage target after the complete test target list is known.</comment>
<file context>
@@ -1881,6 +1914,91 @@ if(DFLASH27B_TESTS)
+ "-DSOURCE_DIR=${CMAKE_CURRENT_SOURCE_DIR}/src"
+ "-DHTML_DIR=${_coverage_report_dir}/html"
+ -P "${CMAKE_CURRENT_SOURCE_DIR}/cmake/RunCoverageTests.cmake"
+ DEPENDS ${_coverage_deps}
+ USES_TERMINAL
+ VERBATIM
</file context>
| if(_coverage_has_device_source) | ||
| # Host-side LLVM coverage cannot instrument CUDA sources. | ||
| # Skip their standalone CTest entry and do not build them. | ||
| list(APPEND _coverage_ctest_exclude "^${_coverage_target}$") |
There was a problem hiding this comment.
P1: When a device-backed test has a renamed or discovered CTest name, this regex does not exclude it. CTest then runs an executable that coverage deliberately did not build; exclude the actual names, including ${target}.* discovered tests.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/CMakeLists.txt, line 1945:
<comment>When a device-backed test has a renamed or discovered CTest name, this regex does not exclude it. CTest then runs an executable that `coverage` deliberately did not build; exclude the actual names, including `${target}.*` discovered tests.</comment>
<file context>
@@ -1881,6 +1914,91 @@ if(DFLASH27B_TESTS)
+ if(_coverage_has_device_source)
+ # Host-side LLVM coverage cannot instrument CUDA sources.
+ # Skip their standalone CTest entry and do not build them.
+ list(APPEND _coverage_ctest_exclude "^${_coverage_target}$")
+ else()
+ list(APPEND _coverage_deps ${_coverage_target})
</file context>
| RESULT_VARIABLE _coverage_ctest_result) | ||
| unset(_coverage_ctest_args) | ||
|
|
||
| include("${CMAKE_CURRENT_LIST_DIR}/GenerateCoverageReport.cmake") |
There was a problem hiding this comment.
P3: When ctest fails in a way that produces no profiles, GenerateCoverageReport.cmake aborts first with "CTest produced no LLVM profiles", so the real ctest exit code and this script's "CTest failed" message are never surfaced. Check _coverage_ctest_result immediately after execute_process and only run the report generation when ctest succeeded, or at least report the ctest failure before including the generator.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/cmake/RunCoverageTests.cmake, line 22:
<comment>When ctest fails in a way that produces no profiles, GenerateCoverageReport.cmake aborts first with "CTest produced no LLVM profiles", so the real ctest exit code and this script's "CTest failed" message are never surfaced. Check `_coverage_ctest_result` immediately after execute_process and only run the report generation when ctest succeeded, or at least report the ctest failure before including the generator.</comment>
<file context>
@@ -0,0 +1,26 @@
+ RESULT_VARIABLE _coverage_ctest_result)
+unset(_coverage_ctest_args)
+
+include("${CMAKE_CURRENT_LIST_DIR}/GenerateCoverageReport.cmake")
+
+if(NOT _coverage_ctest_result EQUAL 0)
</file context>
Add the build targets to measure the code coverage for server C++ code.