Skip to content

build: Create single test binary xrpl_tests#7327

Open
mathbunnyru wants to merge 2 commits into
XRPLF:developfrom
mathbunnyru:single_test_bin
Open

build: Create single test binary xrpl_tests#7327
mathbunnyru wants to merge 2 commits into
XRPLF:developfrom
mathbunnyru:single_test_bin

Conversation

@mathbunnyru
Copy link
Copy Markdown
Contributor

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.cpp files and link with gtest::gtest_main instead.

Also, we keep it possible to launch via ctest, but it discovers individulat gtest tests correctly now.

Context of Change

API Impact

  • Public API: New feature (new methods and/or new fields)
  • Public API: Breaking change (in general, breaking changes should only impact the next api_version)
  • libxrpl change (any change that may affect libxrpl or dependents of libxrpl)
  • Peer protocol change (must be backward compatible or bump the peer protocol version)

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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_tests executable linked with GTest::gtest_main.
  • Updates xrpl_add_test() to append sources and libraries to the shared test target.
  • Removes duplicated per-module main.cpp files and runs xrpl_tests directly 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
Copy link
Copy Markdown

codecov Bot commented May 26, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.4%. Comparing base (f9551ac) to head (f5c3995).

Additional details and impacted files

Impacted file tree graph

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

see 11 files with indirect coverage changes

Impacted file tree graph

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Comment thread src/tests/libxrpl/CMakeLists.txt Outdated
xrpl_tests
PROPERTIES RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}"
)
target_link_libraries(xrpl_tests PRIVATE GTest::gtest_main)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because xrpl_add_test does isolate_headers, and it depends on the directory, so can't be done for a single target.

@github-actions
Copy link
Copy Markdown

This PR has conflicts, please resolve them in order for the PR to be reviewed.

@github-actions
Copy link
Copy Markdown

All conflicts have been resolved. Assigned reviewers can now start or resume their review.

@mathbunnyru mathbunnyru requested a review from kuznetsss May 28, 2026 10:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants