build: Create single test binary xrpl_tests#7327
Conversation
There was a problem hiding this comment.
Pull request overview
This PR consolidates the libxrpl gtest-based test suites into a single xrpl_tests binary while preserving CTest discovery for individual gtest cases.
Changes:
- Adds a shared
xrpl_testsexecutable linked withGTest::gtest_main. - Updates
xrpl_add_test()to append sources and libraries to the shared test target. - Removes duplicated per-module
main.cppfiles and runsxrpl_testsdirectly in CI.
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
src/tests/libxrpl/CMakeLists.txt |
Defines the combined test executable and wires all libxrpl test groups into it. |
cmake/XrplAddTest.cmake |
Changes the helper to append sources/libraries to xrpl_tests. |
.github/workflows/reusable-build-test-config.yml |
Runs the combined test binary directly in CI. |
.gersemi/definitions.cmake |
Updates formatter metadata for the new LIBRARIES argument. |
src/tests/libxrpl/*/main.cpp |
Removes duplicated gtest entry points now provided by gtest_main. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #7327 +/- ##
=========================================
- Coverage 82.4% 82.4% -0.0%
=========================================
Files 1011 1011
Lines 76477 76477
Branches 7326 7316 -10
=========================================
- Hits 63004 62997 -7
- Misses 13473 13480 +7 🚀 New features to boost your workflow:
|
| xrpl_tests | ||
| PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}" | ||
| ) | ||
| target_link_libraries(xrpl_tests PRIVATE GTest::gtest_main) |
There was a problem hiding this comment.
It is usually more convenient to have main file for tests in case we will need to do some global initialisation there. But I'm also ok to leave it as it is for now.
There was a problem hiding this comment.
Good point, I added it :)
| xrpl_add_test(protocol_autogen) | ||
| target_link_libraries(xrpl.test.protocol_autogen PRIVATE xrpl.imports.test) | ||
| add_dependencies(xrpl.tests xrpl.test.protocol_autogen) | ||
| xrpl_add_test(basics LIBRARIES xrpl.imports.test) |
There was a problem hiding this comment.
Could you please explain, why do we still need xrpl_add_test()? Why couldn't we create one target executable with all the needed linked libraries and sources?
There was a problem hiding this comment.
Because xrpl_add_test does isolate_headers, and it depends on the directory, so can't be done for a single target.
|
This PR has conflicts, please resolve them in order for the PR to be reviewed. |
7f023c5 to
057dae7
Compare
|
All conflicts have been resolved. Assigned reviewers can now start or resume their review. |
High Level Overview of Change
Note: this doesn't change in any way integrated tests.
Instead of having multiple binaries we'll only have 1.
And we'll put this binary in the build directory and run it directly.
And we remove duplicated
main.cppfiles and link withgtest::gtest_maininstead.Also, we keep it possible to launch via
ctest, but it discovers individulat gtest tests correctly now.Context of Change
API Impact
libxrplchange (any change that may affectlibxrplor dependents oflibxrpl)