From 97ec4e8e2e4a86719701746d1cb2426f16292ff8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Ho=C5=99e=C5=88ovsk=C3=BD?= Date: Mon, 10 Aug 2026 11:45:08 +0200 Subject: [PATCH] catch_discover_tests: Escape test-invariant parts of CTest script only once Previously, the `catch_discover_tests` would prepare the entire CTest command (e.g. `add_test(...)` or `set_tests_properties(...)`) first, and then escape it when finished. However, this caused lot of the command args to be escaped over and over again (e.g. executable name or Catch2's reporter args), for no reason, as they were always the same, and thus their escaping was always the same. Until recently, the performance overhead didn't matter as there were many spots which had quadratic runtime in number of tests. However, the recent refactorings fixed these, and this commit now improves the throughput by 10-20%. Also extended the benchmarked COUNTS in `benchmark_discovery.py`, because the performance is now good enough that it is reasonable to benchmark 16k tests. --- .../discover_tests/benchmark_discovery.py | 2 +- extras/CatchAddTests.cmake | 112 ++++++++++-------- tests/CMakeLists.txt | 4 +- ...cmake => TestPrepareCommandFragment.cmake} | 28 ++--- 4 files changed, 80 insertions(+), 66 deletions(-) rename tests/TestScripts/DiscoverTests/{TestPrepareCommand.cmake => TestPrepareCommandFragment.cmake} (55%) diff --git a/benchmarks/discover_tests/benchmark_discovery.py b/benchmarks/discover_tests/benchmark_discovery.py index 55e6d494..ea0c5229 100644 --- a/benchmarks/discover_tests/benchmark_discovery.py +++ b/benchmarks/discover_tests/benchmark_discovery.py @@ -21,7 +21,7 @@ import time HERE = os.path.dirname(os.path.abspath(__file__)) TEMPLATE = os.path.join(HERE, "listing_template.json") SHIM = os.path.join(HERE, "copy_shim.cmake") -COUNTS = [1, 10, 500, 1000, 2000, 4000, 8000] +COUNTS = [1, 10, 500, 1000, 2000, 4000, 8000, 16000] #COUNTS = [1, 10, 100] REPEATS = 5 diff --git a/extras/CatchAddTests.cmake b/extras/CatchAddTests.cmake index fa75b3d6..38a57d36 100644 --- a/extras/CatchAddTests.cmake +++ b/extras/CatchAddTests.cmake @@ -140,35 +140,28 @@ function(split_json_array json_array_var out_var) set(${out_var} "${array_elements}" PARENT_SCOPE) endfunction() -# 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. +# Prepare (a part of) command with bracketed arguments and return it via `out_var`. # -# 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) +# This allows the registration script to escape parts of the test script +# only once, instead of escaping the unchanged arguments over and over again. +function(prepare_command_fragment out_var) set(_args "") - # use ARGV* instead of ARGN, because ARGN splits arrays into multiple arguments math(EXPR _last_arg ${ARGC}-1) - foreach(_n RANGE 1 ${_last_arg}) - set(_arg "${ARGV${_n}}") - if(_arg MATCHES "[^-./:a-zA-Z0-9_]") - set(_args "${_args} [==[${_arg}]==]") # form a bracket_argument - else() - set(_args "${_args} ${_arg}") - endif() - endforeach() - set(_Command "${NAME}(${_args})\n" PARENT_SCOPE) + if(_last_arg GREATER_EQUAL 1) + foreach(_n RANGE 1 ${_last_arg}) + set(_arg "${ARGV${_n}}") + if(_arg MATCHES "[^-./:a-zA-Z0-9_]") + set(_args "${_args} [==[${_arg}]==]") # form a bracket_argument + else() + set(_args "${_args} ${_arg}") + endif() + endforeach() + endif() + set(${out_var} "${_args}" PARENT_SCOPE) endfunction() + # Generates random filename in the temp folder. # Temp folder is retrieved by checking env vars from various platforms. function(make_temp_file_path OUT_VARIABLE FALLBACK_PATH) @@ -395,6 +388,41 @@ function(catch_discover_tests_impl) # so that each test name can be appended file without further processing. set(test_names "set(${_TEST_LIST}") + # Most of the commands/arguments in the CTest script are identical + # for every test registered with one `catch_discover_tests` call. + # To avoid repeating the work in escaping them, we escape them before + # the per-test loop and reuse the escaped fragments. + # + # `add_test` calls are + # add_test() + # Of these, ,,, and are the same between + # all tests. + prepare_command_fragment(_exec_exe_fragment + ${_TEST_EXECUTOR} + "${_TEST_EXECUTABLE}" + ) + prepare_command_fragment(_args_reporter_fragment + ${extra_args} + "${reporter_arg}" + ) + + # `set_tests_properties` calls are + # set_tests_properties( PROPERTIES WORKING_DIRECTORY ) + # set_tests_properties( PROPERTIES ENVIRONMENT_MODIFICATION ) + # Of these, only the changes between tests. + prepare_command_fragment(_properties_fragment + PROPERTIES + WORKING_DIRECTORY "${_TEST_WORKING_DIR}" + ${properties} + ) + # Env modification is optional, so we prepare it in a separate command + if(environment_modifications) + prepare_command_fragment(_env_modification_fragment + PROPERTIES + ENVIRONMENT_MODIFICATION "${environment_modifications}" + ) + endif() + # Each element in the tests is JSON-string representing one test object. # We have to parse it and then turn it into CTest script commands. foreach(single_test IN LISTS tests) @@ -431,24 +459,15 @@ function(catch_discover_tests_impl) set(output_dir_arg "--out ${output_dir}/${output_prefix}${escaped_name_clean}${output_suffix}") endif() - # ...and add to script - prepare_command(add_test - "${prefix}${plain_name}${suffix}" - ${_TEST_EXECUTOR} - "${_TEST_EXECUTABLE}" - "${escaped_name}" - ${extra_args} - "${reporter_arg}" - "${output_dir_arg}" - ) - string(APPEND script "${_Command}") - prepare_command(set_tests_properties - "${prefix}${plain_name}${suffix}" - PROPERTIES - WORKING_DIRECTORY "${_TEST_WORKING_DIR}" - ${properties} - ) - string(APPEND script "${_Command}") + set(full_name "${prefix}${plain_name}${suffix}") + prepare_command_fragment(_full_name_fragment "${full_name}") + prepare_command_fragment(_escaped_name_fragment "${escaped_name}") + prepare_command_fragment(_outdir_fragment "${output_dir_arg}") + + string(APPEND script + "add_test(${_full_name_fragment}${_exec_exe_fragment}${_escaped_name_fragment}${_args_reporter_fragment}${_outdir_fragment})\n") + string(APPEND script + "set_tests_properties(${_full_name_fragment}${_properties_fragment})\n") if(add_tags) string(JSON num_tags LENGTH "${test_tags}") @@ -468,21 +487,16 @@ function(catch_discover_tests_impl) list(APPEND tag_list "${a_tag}") endforeach() - prepare_command(set_tests_properties - "${prefix}${plain_name}${suffix}" + prepare_command_fragment(_labels_fragment PROPERTIES LABELS "${tag_list}" ) - string(APPEND script "${_Command}") + string(APPEND script "set_tests_properties(${_full_name_fragment}${_labels_fragment})\n") endif() endif(add_tags) if(environment_modifications) - prepare_command(set_tests_properties - "${prefix}${plain_name}${suffix}" - PROPERTIES - ENVIRONMENT_MODIFICATION "${environment_modifications}") - string(APPEND script "${_Command}") + string(APPEND script "set_tests_properties(${_full_name_fragment}${_env_modification_fragment})\n") endif() # The test name has to be escaped using the same rules as prepare_command diff --git a/tests/CMakeLists.txt b/tests/CMakeLists.txt index 573c38e7..5a74a9d0 100644 --- a/tests/CMakeLists.txt +++ b/tests/CMakeLists.txt @@ -672,11 +672,11 @@ if(CATCH_ENABLE_CMAKE_HELPER_TESTS) LABELS "uses-python" ) - add_test(NAME "CMakeHelper::PrepareCommand" + add_test(NAME "CMakeHelper::PrepareCommandFragment" COMMAND "${CMAKE_COMMAND}" "-DCATCH_ADD_TESTS_SCRIPT=${CATCH_DIR}/extras/CatchAddTests.cmake" - -P "${CMAKE_CURRENT_LIST_DIR}/TestScripts/DiscoverTests/TestPrepareCommand.cmake" + -P "${CMAKE_CURRENT_LIST_DIR}/TestScripts/DiscoverTests/TestPrepareCommandFragment.cmake" ) add_test(NAME "CMakeHelper::DecomposeJsonArray" diff --git a/tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake b/tests/TestScripts/DiscoverTests/TestPrepareCommandFragment.cmake similarity index 55% rename from tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake rename to tests/TestScripts/DiscoverTests/TestPrepareCommandFragment.cmake index f02bb409..cfcaf0d1 100644 --- a/tests/TestScripts/DiscoverTests/TestPrepareCommand.cmake +++ b/tests/TestScripts/DiscoverTests/TestPrepareCommandFragment.cmake @@ -1,12 +1,12 @@ # SPDX-License-Identifier: BSL-1.0 -# Unit tests for `prepare_command` helper in `extras/CatchAddTests.cmake`. +# Unit tests for `prepare_command_fragment` 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 +# -P TestPrepareCommandFragment.cmake cmake_minimum_required(VERSION 3.19) @@ -37,25 +37,25 @@ function(expect_equal description actual expected) 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_fragment(test_fragment SimpleName /path/to/tests) +expect_equal("Simple arg, no quotes" "${test_fragment}" " SimpleName /path/to/tests") -prepare_command(add_test "Name with spaces") -expect_equal("Spaces in arg, needs quotes" "${_Command}" "add_test( [==[Name with spaces]==])\n") +prepare_command_fragment(test_fragment "Name with spaces") +expect_equal("Spaces in arg, needs quotes" "${test_fragment}" " [==[Name with spaces]==]") -prepare_command(set_tests_properties Foo PROPERTIES LABELS "tagA\;tagB\;tagC") +prepare_command_fragment(test_fragment 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") + "${test_fragment}" " Foo PROPERTIES LABELS [==[tagA\;tagB\;tagC]==]") -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") +set(test_fragment "PRE-EXISTING") +prepare_command_fragment(test_fragment Foo PROPERTIES BAR baz) +expect_equal("out var does no accumulate commands" + "${test_fragment}" " Foo PROPERTIES BAR baz") if(_failures GREATER 0) - message(FATAL_ERROR "${_failures} prepare_command test(s) failed") + message(FATAL_ERROR "${_failures} prepare_command_fragment test(s) failed") else() - message(STATUS "All prepare_command tests passed") + message(STATUS "All prepare_command_fragment tests passed") endif()