Export and install test_suite for downstream use#186
Export and install test_suite for downstream use#186sgerbino merged 1 commit intocppalliance:developfrom
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review infoConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughThe PR adds a test_suite subproject, exports two new library targets (boost_capy_test_suite, boost_capy_test_suite_main) with install-aware include paths and aliases, updates package config to include test discovery scripts, and installs the test-suite header and CMake discovery files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
An automated preview of the documentation is available at https://186.capy.prtest3.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-02-26 10:50:43 UTC |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
extra/test_suite/CMakeLists.txt (1)
26-28: Redundant include directories.Since
boost_capy_test_suite_mainlinksPUBLICtoboost_capy_test_suite(line 25), it already inherits the include directories. This explicittarget_include_directoriescall is redundant.♻️ Proposed fix: Remove redundant include directories
# Library target that links the previous and adds test_main.cpp add_library(boost_capy_test_suite_main STATIC test_main.cpp) target_link_libraries(boost_capy_test_suite_main PUBLIC boost_capy_test_suite) -target_include_directories(boost_capy_test_suite_main PUBLIC - $<BUILD_INTERFACE:${CMAKE_CURRENT_SOURCE_DIR}> - $<INSTALL_INTERFACE:${CMAKE_INSTALL_INCLUDEDIR}/boost/capy/extra/test_suite>) target_compile_features(boost_capy_test_suite_main PUBLIC cxx_std_17)🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@extra/test_suite/CMakeLists.txt` around lines 26 - 28, Remove the redundant target_include_directories call for boost_capy_test_suite_main: since boost_capy_test_suite_main already links PUBLIC to boost_capy_test_suite (which exports its include dirs), delete the target_include_directories(...) block referencing $<BUILD_INTERFACE:...> and $<INSTALL_INTERFACE:...>; leave the existing target_link_libraries(boost_capy_test_suite_main PUBLIC boost_capy_test_suite) as-is so include directories continue to be propagated.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@extra/test_suite/CMakeLists.txt`:
- Around line 18-20: The INSTALL_INTERFACE uses CMAKE_INSTALL_INCLUDEDIR which
may be undefined because include(GNUInstallDirs) happens after
add_subdirectory(extra/test_suite); fix by ensuring GNUInstallDirs is included
before the subdirectory is processed (i.e., move or add include(GNUInstallDirs)
earlier in the parent CMakeLists so CMAKE_INSTALL_INCLUDEDIR is defined when
target_include_directories in boost_capy_test_suite is evaluated), or
alternatively provide a fallback default for CMAKE_INSTALL_INCLUDEDIR before
referencing it.
---
Nitpick comments:
In `@extra/test_suite/CMakeLists.txt`:
- Around line 26-28: Remove the redundant target_include_directories call for
boost_capy_test_suite_main: since boost_capy_test_suite_main already links
PUBLIC to boost_capy_test_suite (which exports its include dirs), delete the
target_include_directories(...) block referencing $<BUILD_INTERFACE:...> and
$<INSTALL_INTERFACE:...>; leave the existing
target_link_libraries(boost_capy_test_suite_main PUBLIC boost_capy_test_suite)
as-is so include directories continue to be propagated.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
test/unit/CMakeLists.txtis excluded by!**/test/**
📒 Files selected for processing (3)
CMakeLists.txtcmake/boost_capy-config.cmake.inextra/test_suite/CMakeLists.txt
Make boost_capy_test_suite and boost_capy_test_suite_main available to downstream consumers via find_package(boost_capy). Add generator expressions for include dirs, EXPORT_NAME properties, Boost:: aliases, and install rules for targets, headers, and CMake modules.
sgerbino
force-pushed
the
pr/test-suite
branch
from
11581d1 to
0886655
Compare
|
GCOVR code coverage report https://186.capy.prtest3.cppalliance.org/gcovr/index.html Build time: 2026-02-26 10:58:17 UTC |
2 participants
Make boost_capy_test_suite and boost_capy_test_suite_main available to downstream consumers via find_package(boost_capy). Add generator expressions for include dirs, EXPORT_NAME properties, Boost:: aliases, and install rules for targets, headers, and CMake modules.
Summary by CodeRabbit