From 03704f712b12848d5711881d675b15cb61b6514e Mon Sep 17 00:00:00 2001 From: Bronek Kozicki Date: Tue, 11 Nov 2025 14:55:16 +0000 Subject: [PATCH] chore: Move running of unit tests out of coverage target (#6018) This change makes the progress of unit tests visible and also gives more flexibility when running them. --- .../workflows/reusable-build-test-config.yml | 36 +++++++++---------- BUILD.md | 19 ++++------ cmake/CodeCoverage.cmake | 30 ++++++++++------ cmake/XrplCov.cmake | 9 ++--- cmake/XrplSettings.cmake | 7 ---- src/tests/libxrpl/CMakeLists.txt | 9 +++++ 6 files changed, 59 insertions(+), 51 deletions(-) diff --git a/.github/workflows/reusable-build-test-config.yml b/.github/workflows/reusable-build-test-config.yml index 4a8bc71321..79d21a870a 100644 --- a/.github/workflows/reusable-build-test-config.yml +++ b/.github/workflows/reusable-build-test-config.yml @@ -172,6 +172,24 @@ jobs: -C "${BUILD_TYPE}" \ -j "${PARALLELISM}" + - name: Run the embedded tests + if: ${{ !inputs.build_only }} + working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', inputs.build_dir, inputs.build_type) || inputs.build_dir }} + shell: bash + env: + BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} + run: | + ./rippled --unittest --unittest-jobs "${BUILD_NPROC}" + + - name: Debug failure (Linux) + if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }} + shell: bash + run: | + echo "IPv4 local port range:" + cat /proc/sys/net/ipv4/ip_local_port_range + echo "Netstat:" + netstat -an + - name: Prepare coverage report if: ${{ !inputs.build_only && env.ENABLED_COVERAGE == 'true' }} working-directory: ${{ inputs.build_dir }} @@ -197,21 +215,3 @@ jobs: plugins: noop token: ${{ secrets.CODECOV_TOKEN }} verbose: true - - - name: Run the embedded tests - if: ${{ !inputs.build_only && env.ENABLED_COVERAGE != 'true' }} - working-directory: ${{ runner.os == 'Windows' && format('{0}/{1}', inputs.build_dir, inputs.build_type) || inputs.build_dir }} - shell: bash - env: - BUILD_NPROC: ${{ steps.nproc.outputs.nproc }} - run: | - ./rippled --unittest --unittest-jobs "${BUILD_NPROC}" - - - name: Debug failure (Linux) - if: ${{ failure() && runner.os == 'Linux' && !inputs.build_only }} - shell: bash - run: | - echo "IPv4 local port range:" - cat /proc/sys/net/ipv4/ip_local_port_range - echo "Netstat:" - netstat -an diff --git a/BUILD.md b/BUILD.md index f13204ea4c..39570edbd3 100644 --- a/BUILD.md +++ b/BUILD.md @@ -495,18 +495,18 @@ A coverage report is created when the following steps are completed, in order: 1. `rippled` binary built with instrumentation data, enabled by the `coverage` option mentioned above -2. completed run of unit tests, which populates coverage capture data +2. completed one or more run of the unit tests, which populates coverage capture data 3. completed run of the `gcovr` tool (which internally invokes either `gcov` or `llvm-cov`) to assemble both instrumentation data and the coverage capture data into a coverage report -The above steps are automated into a single target `coverage`. The instrumented +The last step of the above is automated into a single target `coverage`. The instrumented `rippled` binary can also be used for regular development or testing work, at the cost of extra disk space utilization and a small performance hit -(to store coverage capture). In case of a spurious failure of unit tests, it is -possible to re-run the `coverage` target without rebuilding the `rippled` binary -(since it is simply a dependency of the coverage report target). It is also possible -to select only specific tests for the purpose of the coverage report, by setting -the `coverage_test` variable in `cmake` +(to store coverage capture data). Since `rippled` binary is simply a dependency of the +coverage report target, it is possible to re-run the `coverage` target without +rebuilding the `rippled` binary. Note, running of the unit tests before the `coverage` +target is left to the developer. Each such run will append to the coverage data +collected in the build directory. The default coverage report format is `html-details`, but the user can override it to any of the formats listed in `Builds/CMake/CodeCoverage.cmake` @@ -515,11 +515,6 @@ to generate more than one format at a time by setting the `coverage_extra_args` variable in `cmake`. The specific command line used to run the `gcovr` tool will be displayed if the `CODE_COVERAGE_VERBOSE` variable is set. -By default, the code coverage tool runs parallel unit tests with `--unittest-jobs` -set to the number of available CPU cores. This may cause spurious test -errors on Apple. Developers can override the number of unit test jobs with -the `coverage_test_parallelism` variable in `cmake`. - Example use with some cmake variables set: ``` diff --git a/cmake/CodeCoverage.cmake b/cmake/CodeCoverage.cmake index c2b66c9cac..e1b44e656d 100644 --- a/cmake/CodeCoverage.cmake +++ b/cmake/CodeCoverage.cmake @@ -109,6 +109,9 @@ # - add a new function add_code_coverage_to_target # - remove some unused code # +# 2025-11-11, Bronek Kozicki +# - make EXECUTABLE and EXECUTABLE_ARGS optional +# # USAGE: # # 1. Copy this file into your cmake modules path. @@ -317,6 +320,10 @@ function(setup_target_for_coverage_gcovr) set(Coverage_FORMAT xml) endif() + if(NOT DEFINED Coverage_EXECUTABLE AND DEFINED Coverage_EXECUTABLE_ARGS) + message(FATAL_ERROR "EXECUTABLE_ARGS must not be set if EXECUTABLE is not set") + endif() + if("--output" IN_LIST GCOVR_ADDITIONAL_ARGS) message(FATAL_ERROR "Unsupported --output option detected in GCOVR_ADDITIONAL_ARGS! Aborting...") else() @@ -398,17 +405,18 @@ function(setup_target_for_coverage_gcovr) endforeach() # Set up commands which will be run to generate coverage data - # Run tests - set(GCOVR_EXEC_TESTS_CMD - ${Coverage_EXECUTABLE} ${Coverage_EXECUTABLE_ARGS} - ) + # If EXECUTABLE is not set, the user is expected to run the tests manually + # before running the coverage target NAME + if(DEFINED Coverage_EXECUTABLE) + set(GCOVR_EXEC_TESTS_CMD + ${Coverage_EXECUTABLE} ${Coverage_EXECUTABLE_ARGS} + ) + endif() # Create folder if(DEFINED GCOVR_CREATE_FOLDER) set(GCOVR_FOLDER_CMD ${CMAKE_COMMAND} -E make_directory ${GCOVR_CREATE_FOLDER}) - else() - set(GCOVR_FOLDER_CMD echo) # dummy endif() # Running gcovr @@ -425,11 +433,13 @@ function(setup_target_for_coverage_gcovr) if(CODE_COVERAGE_VERBOSE) message(STATUS "Executed command report") - message(STATUS "Command to run tests: ") - string(REPLACE ";" " " GCOVR_EXEC_TESTS_CMD_SPACED "${GCOVR_EXEC_TESTS_CMD}") - message(STATUS "${GCOVR_EXEC_TESTS_CMD_SPACED}") + if(NOT "${GCOVR_EXEC_TESTS_CMD}" STREQUAL "") + message(STATUS "Command to run tests: ") + string(REPLACE ";" " " GCOVR_EXEC_TESTS_CMD_SPACED "${GCOVR_EXEC_TESTS_CMD}") + message(STATUS "${GCOVR_EXEC_TESTS_CMD_SPACED}") + endif() - if(NOT GCOVR_FOLDER_CMD STREQUAL "echo") + if(NOT "${GCOVR_FOLDER_CMD}" STREQUAL "") message(STATUS "Command to create a folder: ") string(REPLACE ";" " " GCOVR_FOLDER_CMD_SPACED "${GCOVR_FOLDER_CMD}") message(STATUS "${GCOVR_FOLDER_CMD_SPACED}") diff --git a/cmake/XrplCov.cmake b/cmake/XrplCov.cmake index 288f74c0ef..b212d60b64 100644 --- a/cmake/XrplCov.cmake +++ b/cmake/XrplCov.cmake @@ -11,6 +11,9 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "MSVC") return() endif() +include(ProcessorCount) +ProcessorCount(PROCESSOR_COUNT) + include(CodeCoverage) # The instructions for these commands come from the `CodeCoverage` module, @@ -26,15 +29,13 @@ list(APPEND GCOVR_ADDITIONAL_ARGS --exclude-throw-branches --exclude-noncode-lines --exclude-unreachable-branches -s - -j ${coverage_test_parallelism}) + -j ${PROCESSOR_COUNT}) setup_target_for_coverage_gcovr( NAME coverage FORMAT ${coverage_format} - EXECUTABLE xrpld - EXECUTABLE_ARGS --unittest$<$:=${coverage_test}> --unittest-jobs ${coverage_test_parallelism} --quiet --unittest-log EXCLUDE "src/test" "src/tests" "include/xrpl/beast/test" "include/xrpl/beast/unit_test" "${CMAKE_BINARY_DIR}/pb-xrpl.libpb" - DEPENDENCIES xrpld + DEPENDENCIES xrpld xrpl.tests ) add_code_coverage_to_target(opts INTERFACE) diff --git a/cmake/XrplSettings.cmake b/cmake/XrplSettings.cmake index be339c135e..a16513afc5 100644 --- a/cmake/XrplSettings.cmake +++ b/cmake/XrplSettings.cmake @@ -51,17 +51,10 @@ if(is_gcc OR is_clang) option(coverage "Generates coverage info." OFF) option(profile "Add profiling flags" OFF) - set(coverage_test_parallelism "${PROCESSOR_COUNT}" CACHE STRING - "Unit tests parallelism for the purpose of coverage report.") set(coverage_format "html-details" CACHE STRING "Output format of the coverage report.") set(coverage_extra_args "" CACHE STRING "Additional arguments to pass to gcovr.") - set(coverage_test "" CACHE STRING - "On gcc & clang, the specific unit test(s) to run for coverage. Default is all tests.") - if(coverage_test AND NOT coverage) - set(coverage ON CACHE BOOL "gcc/clang only" FORCE) - endif() option(wextra "compile with extra gcc/clang warnings enabled" ON) else() set(profile OFF CACHE BOOL "gcc/clang only" FORCE) diff --git a/src/tests/libxrpl/CMakeLists.txt b/src/tests/libxrpl/CMakeLists.txt index 880fdb2948..a2374698d9 100644 --- a/src/tests/libxrpl/CMakeLists.txt +++ b/src/tests/libxrpl/CMakeLists.txt @@ -3,6 +3,9 @@ include(XrplAddTest) # Test requirements. find_package(doctest REQUIRED) +# Custom target for all tests defined in this file +add_custom_target(xrpl.tests) + # Common library dependencies for the rest of the tests. add_library(xrpl.imports.test INTERFACE) target_link_libraries(xrpl.imports.test INTERFACE doctest::doctest xrpl.libxrpl) @@ -10,13 +13,19 @@ target_link_libraries(xrpl.imports.test INTERFACE doctest::doctest xrpl.libxrpl) # One test for each module. xrpl_add_test(basics) target_link_libraries(xrpl.test.basics PRIVATE xrpl.imports.test) +add_dependencies(xrpl.tests xrpl.test.basics) + xrpl_add_test(crypto) target_link_libraries(xrpl.test.crypto PRIVATE xrpl.imports.test) +add_dependencies(xrpl.tests xrpl.test.crypto) + xrpl_add_test(json) target_link_libraries(xrpl.test.json PRIVATE xrpl.imports.test) +add_dependencies(xrpl.tests xrpl.test.json) # Network unit tests are currently not supported on Windows if(NOT WIN32) xrpl_add_test(net) target_link_libraries(xrpl.test.net PRIVATE xrpl.imports.test) + add_dependencies(xrpl.tests xrpl.test.net) endif()