From 64a551e2e7edb85e4f2cd99e5737a587eb06c35b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Fri, 24 Jul 2026 12:22:49 +0200 Subject: [PATCH] Optimize adding test commands in catch_discover_tests The previous approach was for `add_command` to behave as append function via concatenating the command string internally and then saving it into `PARENT_SCOPE`. Because this in practice ended up meaning concatenating a copy of the string inside the function and then overwriting the string outside the function, the performance was lacking. The new approach is for `prepare_command` to only escape & return the command from single call, and the caller is responsible for concatenating the result. Since the actual concatenation no longer crosses function scope boundaries, the performance is much better, even though the scaling is still quadratic. The new approach only takes 1/4-1/5 of the time, saving ~1s at 1k tests, 4.5s at 2k tests and 19s at 4k tests. --- extras/CatchAddTests.cmake | 37 ++++++++--- tests/CMakeLists.txt | 7 +++ .../DiscoverTests/TestPrepareCommand.cmake | 61 +++++++++++++++++++ 3 files changed, 97 insertions(+), 8 deletions(-) create mode 100644 tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake diff --git a/extras/CatchAddTests.cmake b/extras/CatchAddTests.cmake index 5c67c874..8005f31f 100644 --- a/extras/CatchAddTests.cmake +++ b/extras/CatchAddTests.cmake @@ -1,7 +1,21 @@ # Distributed under the OSI-approved BSD 3-Clause License. See accompanying # file Copyright.txt or https://cmake.org/licensing for details. -function(add_command NAME) +# TBD: Further possible optimization is that most arguments for per-test +# `prepare_command` call are constant across one invocation of +# `catch_discover_tests`, and thus need checking and escaping only +# once, instead of for each test. +# This would provide nice speed-up of the actual command preparation, +# but it will make the script much harder to read, and it is utterly +# dwarfed by the quadratic scaling of parsing JSON arrays in CMake. + + +# Prepare command with escaped (bracketed) arguments and return it via `_Command` out variable. +# +# To avoid quadratic performance when concatenating all commands together, +# the actual concatenation must be done by the caller, by appending it +# into a string of all other commands. +function(prepare_command NAME) set(_args "") # use ARGV* instead of ARGN, because ARGN splits arrays into multiple arguments math(EXPR _last_arg ${ARGC}-1) @@ -13,7 +27,7 @@ function(add_command NAME) set(_args "${_args} ${_arg}") endif() endforeach() - set(script "${script}${NAME}(${_args})\n" PARENT_SCOPE) + set(_Command "${NAME}(${_args})\n" PARENT_SCOPE) endfunction() # Generates random filename in the temp folder. @@ -220,7 +234,7 @@ function(catch_discover_tests_impl) endif() # ...and add to script - add_command(add_test + prepare_command(add_test "${prefix}${plain_name}${suffix}" ${_TEST_EXECUTOR} "${_TEST_EXECUTABLE}" @@ -229,12 +243,14 @@ function(catch_discover_tests_impl) "${reporter_arg}" "${output_dir_arg}" ) - add_command(set_tests_properties + string(APPEND script "${_Command}") + prepare_command(set_tests_properties "${prefix}${plain_name}${suffix}" PROPERTIES WORKING_DIRECTORY "${_TEST_WORKING_DIR}" ${properties} ) + string(APPEND script "${_Command}") if(add_tags) string(JSON num_tags LENGTH "${test_tags}") @@ -254,19 +270,21 @@ function(catch_discover_tests_impl) list(APPEND tag_list "${a_tag}") endforeach() - add_command(set_tests_properties + prepare_command(set_tests_properties "${prefix}${plain_name}${suffix}" PROPERTIES LABELS "${tag_list}" ) + string(APPEND script "${_Command}") endif() endif(add_tags) if(environment_modifications) - add_command(set_tests_properties + prepare_command(set_tests_properties "${prefix}${plain_name}${suffix}" PROPERTIES ENVIRONMENT_MODIFICATION "${environment_modifications}") + string(APPEND script "${_Command}") endif() list(APPEND tests "${prefix}${plain_name}${suffix}") @@ -274,13 +292,16 @@ function(catch_discover_tests_impl) # Create a list of all discovered tests, which users may use to e.g. set # properties on the tests - add_command(set ${_TEST_LIST} ${tests}) + prepare_command(set ${_TEST_LIST} ${tests}) + string(APPEND script "${_Command}") # Write CTest script file(WRITE "${_CTEST_FILE}" "${script}") endfunction() -if(CMAKE_SCRIPT_MODE_FILE) +# To enable `include`ing this file in the unit test scripts, we only run +# the impl if an actual `TEST_EXECUTABLE` is provided. +if(CMAKE_SCRIPT_MODE_FILE AND DEFINED TEST_EXECUTABLE) catch_discover_tests_impl( TEST_EXECUTABLE ${TEST_EXECUTABLE} TEST_EXECUTOR ${TEST_EXECUTOR} diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 7429734f..e2aad5ef 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -625,6 +625,13 @@ if(CATCH_ENABLE_CMAKE_HELPER_TESTS) COST 240 LABELS "uses-python" ) + + add_test(NAME "CMakeHelper::PrepareCommand" + COMMAND + "${CMAKE_COMMAND}" + "-DCATCH_ADD_TESTS_SCRIPT=${CATCH_DIR}/extras/CatchAddTests.cmake" + -P "${CMAKE_CURRENT_LIST_DIR}/TestScripts/DiscoverTests/TestPrepareCommand.cmake" + ) endif() foreach(reporterName # "Automake" - the simple .trs format does not support any kind of comments/metadata diff --git a/tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake b/tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake new file mode 100644 index 00000000..f02bb409 --- /dev/null +++ b/tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake @@ -0,0 +1,61 @@ +# SPDX-License-Identifier: BSL-1.0 + +# Unit tests for `prepare_command` helper in `extras/CatchAddTests.cmake`. +# +# Yes, we are at the stage where the script helpers need unit tests. +# +# Run as +# cmake -DCATCH_ADD_TESTS_SCRIPT=/path/to/extras/CatchAddTests.cmake \ +# -P TestPrepareCommand.cmake + + +cmake_minimum_required(VERSION 3.19) + +if(NOT DEFINED CATCH_ADD_TESTS_SCRIPT) + message(FATAL_ERROR "Missing argument `CATCH_ADD_TESTS_SCRIPT`") +endif() + +if(NOT EXISTS "${CATCH_ADD_TESTS_SCRIPT}") + message(FATAL_ERROR "Cannot find CatchAddTests.cmake at '${CATCH_ADD_TESTS_SCRIPT}'") +endif() + +# Pull in the helper functions. Without `TEST_EXECUTABLE` being defined, +# `catch_discover_tests_impl` is not called. +include("${CATCH_ADD_TESTS_SCRIPT}") + +set(_failures 0) + +function(expect_equal description actual expected) + if(actual STREQUAL expected) + message(" [PASS] ${description}") + else() + message(" [FAIL] ${description}") + message(" expected: [${expected}]") + message(" actual: [${actual}]") + math(EXPR _n "${_failures} + 1") + set(_failures "${_n}" PARENT_SCOPE) + endif() +endfunction() + +prepare_command(add_test SimpleName /path/to/tests) +expect_equal("Simple arg, no quotes" "${_Command}" "add_test( SimpleName /path/to/tests)\n") + +prepare_command(add_test "Name with spaces") +expect_equal("Spaces in arg, needs quotes" "${_Command}" "add_test( [==[Name with spaces]==])\n") + +prepare_command(set_tests_properties Foo PROPERTIES LABELS "tagA\;tagB\;tagC") +expect_equal("semicolons in argument are kept and quoted" + "${_Command}" "set_tests_properties( Foo PROPERTIES LABELS [==[tagA\;tagB\;tagC]==])\n") + +set(_Command "PRE-EXISTING") +prepare_command(set_tests_properties Foo PROPERTIES BAR baz) +expect_equal("_Command var does no accumulate commands" + "${_Command}" "set_tests_properties( Foo PROPERTIES BAR baz)\n") + + + +if(_failures GREATER 0) + message(FATAL_ERROR "${_failures} prepare_command test(s) failed") +else() + message(STATUS "All prepare_command tests passed") +endif()