From 61929f936f3a834a99762aafaf79f9f1ce9cf6d6 Mon Sep 17 00:00:00 2001 From: Evgeniy Shcherbina Date: Wed, 9 Feb 2022 11:26:51 +0300 Subject: GoogleTest: Fix escaping in test names Due to add_command() being a macro it introduced excessive and nonobvious escaping in different parts of the script. Because of one of such places the resulting script would have an erroneous ${TEST_LIST} if the user data (in test parameters) had a semicolon. To eliminate this non-obvious escaping, add_command() was converted to function. Updated the escaping accordingly. Fixes: #23059 --- Modules/GoogleTestAddTests.cmake | 41 +++++++--------- .../GoogleTest-discovery-check-test-list.cmake | 6 +++ ...gleTest-discovery-flush-script-check-list.cmake | 5 ++ .../GoogleTestDiscoveryFlushScript.cmake | 14 ++++++ .../GoogleTest/GoogleTestDiscoveryTestList.cmake | 14 ++++++ Tests/RunCMake/GoogleTest/RunCMakeTest.cmake | 54 ++++++++++++++++++++++ Tests/RunCMake/GoogleTest/flush_script_test.cpp | 19 ++++++++ Tests/RunCMake/GoogleTest/test_list_test.cpp | 18 ++++++++ 8 files changed, 147 insertions(+), 24 deletions(-) create mode 100644 Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake create mode 100644 Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake create mode 100644 Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake create mode 100644 Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake create mode 100644 Tests/RunCMake/GoogleTest/flush_script_test.cpp create mode 100644 Tests/RunCMake/GoogleTest/test_list_test.cpp diff --git a/Modules/GoogleTestAddTests.cmake b/Modules/GoogleTestAddTests.cmake index cef2e8a..2bd0cc9 100644 --- a/Modules/GoogleTestAddTests.cmake +++ b/Modules/GoogleTestAddTests.cmake @@ -9,7 +9,7 @@ set(flush_tests_MODE WRITE) # Flushes script to ${_CTEST_FILE} macro(flush_script) file(${flush_tests_MODE} "${_CTEST_FILE}" "${script}") - set(flush_tests_MODE APPEND) + set(flush_tests_MODE APPEND PARENT_SCOPE) set(script "") endmacro() @@ -20,24 +20,22 @@ macro(flush_tests_buffer) set(tests_buffer "") endmacro() -macro(add_command NAME TEST_NAME) - set(_args "") - foreach(_arg ${ARGN}) - if(_arg MATCHES "[^-./:a-zA-Z0-9_]") - string(APPEND _args " [==[${_arg}]==]") +function(add_command NAME TEST_NAME) + set(args "") + foreach(arg ${ARGN}) + if(arg MATCHES "[^-./:a-zA-Z0-9_]") + string(APPEND args " [==[${arg}]==]") else() - string(APPEND _args " ${_arg}") + string(APPEND args " ${arg}") endif() endforeach() - string(APPEND script "${NAME}(${TEST_NAME} ${_args})\n") - string(LENGTH "${script}" _script_len) - if(${_script_len} GREATER "50000") + string(APPEND script "${NAME}(${TEST_NAME} ${args})\n") + string(LENGTH "${script}" script_len) + if(${script_len} GREATER "50000") flush_script() endif() - # Unsets macro local variables to prevent leakage outside of this macro. - unset(_args) - unset(_script_len) -endmacro() + set(script "${script}" PARENT_SCOPE) +endfunction() function(generate_testname_guards OUTPUT OPEN_GUARD_VAR CLOSE_GUARD_VAR) set(open_guard "[=[") @@ -164,7 +162,6 @@ function(gtest_discover_tests_impl) endif() string(CONFIGURE "${test_name_template}" testname) - # sanitize test name for further processing downstream # unescape [] if(open_sb) string(REPLACE "${open_sb}" "[" testname "${testname}") @@ -172,13 +169,9 @@ function(gtest_discover_tests_impl) if(close_sb) string(REPLACE "${close_sb}" "]" testname "${testname}") endif() - # escape \ - string(REPLACE [[\]] [[\\]] testname "${testname}") - # escape $ - string(REPLACE [[$]] [[\$]] testname "${testname}") set(guarded_testname "${open_guard}${testname}${close_guard}") - # ...and add to script + # add to script add_command(add_test "${guarded_testname}" ${_TEST_EXECUTOR} @@ -199,14 +192,14 @@ function(gtest_discover_tests_impl) "${guarded_testname}" PROPERTIES WORKING_DIRECTORY "${_TEST_WORKING_DIR}" - SKIP_REGULAR_EXPRESSION "\\\\[ SKIPPED \\\\]" + SKIP_REGULAR_EXPRESSION "\\[ SKIPPED \\]" ${properties} ) - # possibly unbalanced square brackets render lists invalid so skip such tests in _TEST_LIST + # possibly unbalanced square brackets render lists invalid so skip such tests in ${_TEST_LIST} if(NOT "${testname}" MATCHES [=[(\[|\])]=]) # escape ; - string(REPLACE [[;]] [[\;]] testname "${testname}") + string(REPLACE [[;]] [[\\;]] testname "${testname}") list(APPEND tests_buffer "${testname}") list(LENGTH tests_buffer tests_buffer_length) if(${tests_buffer_length} GREATER "250") @@ -221,7 +214,7 @@ function(gtest_discover_tests_impl) # Create a list of all discovered tests, which users may use to e.g. set # properties on the tests flush_tests_buffer() - add_command(set "" ${_TEST_LIST} ${tests}) + add_command(set "" ${_TEST_LIST} "${tests}") # Write CTest script flush_script() diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake new file mode 100644 index 0000000..94169e7 --- /dev/null +++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-check-test-list.cmake @@ -0,0 +1,6 @@ +list(LENGTH test_list_test_TESTS LIST_SIZE) +set(EXPECTED_SIZE 2) +if(NOT LIST_SIZE EQUAL ${EXPECTED_SIZE}) + message("TEST_LIST should have ${EXPECTED_SIZE} elements but it has ${LIST_SIZE}") + message("The unexpected list: [${test_list_test_TESTS}]") +endif() diff --git a/Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake new file mode 100644 index 0000000..1ae222f --- /dev/null +++ b/Tests/RunCMake/GoogleTest/GoogleTest-discovery-flush-script-check-list.cmake @@ -0,0 +1,5 @@ +list(LENGTH flush_script_test_TESTS LIST_SIZE) +set(EXPECTED_LIST_SIZE 4) +if(NOT LIST_SIZE EQUAL ${EXPECTED_LIST_SIZE}) + message("TEST_LIST should have ${EXPECTED_LIST_SIZE} elements but it has ${LIST_SIZE}") +endif() diff --git a/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake new file mode 100644 index 0000000..2c138c7 --- /dev/null +++ b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryFlushScript.cmake @@ -0,0 +1,14 @@ +enable_language(CXX) +include(GoogleTest) + +enable_testing() + +include(xcode_sign_adhoc.cmake) + +add_executable(flush_script_test flush_script_test.cpp) +xcode_sign_adhoc(flush_script_test) +gtest_discover_tests( + flush_script_test +) +set_property(DIRECTORY APPEND PROPERTY TEST_INCLUDE_FILES + ${CMAKE_CURRENT_SOURCE_DIR}/GoogleTest-discovery-flush-script-check-list.cmake) diff --git a/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake new file mode 100644 index 0000000..5f4f859 --- /dev/null +++ b/Tests/RunCMake/GoogleTest/GoogleTestDiscoveryTestList.cmake @@ -0,0 +1,14 @@ +enable_language(CXX) +include(GoogleTest) + +enable_testing() + +include(xcode_sign_adhoc.cmake) + +add_executable(test_list_test test_list_test.cpp) +xcode_sign_adhoc(test_list_test) +gtest_discover_tests( + test_list_test +) +set_property(DIRECTORY APPEND PROPERTY TEST_INCLUDE_FILES + ${CMAKE_CURRENT_SOURCE_DIR}/GoogleTest-discovery-check-test-list.cmake) diff --git a/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake b/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake index 33a4b43..695f562 100644 --- a/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake +++ b/Tests/RunCMake/GoogleTest/RunCMakeTest.cmake @@ -236,6 +236,58 @@ function(run_GoogleTest_discovery_multi_config) endfunction() +function(run_GoogleTest_discovery_test_list DISCOVERY_MODE) + # Use a single build tree for a few tests without cleaning. + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTest-discovery-test-list-build) + set(RunCMake_TEST_NO_CLEAN 1) + if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG) + set(RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug) + endif() + file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}") + file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}") + + run_cmake_with_options(GoogleTestDiscoveryTestList -DCMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE=${DISCOVERY_MODE}) + + run_cmake_command(GoogleTest-discovery-test-list-build + ${CMAKE_COMMAND} + --build . + --config Debug + --target test_list_test + ) + + run_cmake_command(GoogleTest-discovery-test-list-test + ${CMAKE_CTEST_COMMAND} + -C Debug + --no-label-summary + ) +endfunction() + +function(run_GoogleTest_discovery_flush_script DISCOVERY_MODE) + # Use a single build tree for a few tests without cleaning. + set(RunCMake_TEST_BINARY_DIR ${RunCMake_BINARY_DIR}/GoogleTest-discovery-flush-script-build) + set(RunCMake_TEST_NO_CLEAN 1) + if(NOT RunCMake_GENERATOR_IS_MULTI_CONFIG) + set(RunCMake_TEST_OPTIONS -DCMAKE_BUILD_TYPE=Debug) + endif() + file(REMOVE_RECURSE "${RunCMake_TEST_BINARY_DIR}") + file(MAKE_DIRECTORY "${RunCMake_TEST_BINARY_DIR}") + + run_cmake_with_options(GoogleTestDiscoveryFlushScript -DCMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE=${DISCOVERY_MODE}) + + run_cmake_command(GoogleTest-discovery-flush-script-build + ${CMAKE_COMMAND} + --build . + --config Debug + --target flush_script_test + ) + + run_cmake_command(GoogleTest-discovery-flush-script-test + ${CMAKE_CTEST_COMMAND} + -C Debug + --no-label-summary + ) +endfunction() + foreach(DISCOVERY_MODE POST_BUILD PRE_TEST) message("Testing ${DISCOVERY_MODE} discovery mode via CMAKE_GTEST_DISCOVER_TESTS_DISCOVERY_MODE global override...") run_GoogleTest(${DISCOVERY_MODE}) @@ -246,6 +298,8 @@ foreach(DISCOVERY_MODE POST_BUILD PRE_TEST) NOT "${DISCOVERY_MODE};${RunCMake_GENERATOR}" MATCHES "^POST_BUILD;Visual Studio 9") run_GoogleTest_discovery_arg_change(${DISCOVERY_MODE}) endif() + run_GoogleTest_discovery_test_list(${DISCOVERY_MODE}) + run_GoogleTest_discovery_flush_script(${DISCOVERY_MODE}) endforeach() if(RunCMake_GENERATOR_IS_MULTI_CONFIG) diff --git a/Tests/RunCMake/GoogleTest/flush_script_test.cpp b/Tests/RunCMake/GoogleTest/flush_script_test.cpp new file mode 100644 index 0000000..9473bb5 --- /dev/null +++ b/Tests/RunCMake/GoogleTest/flush_script_test.cpp @@ -0,0 +1,19 @@ +#include +#include + +int main(int argc, char** argv) +{ + // Note: GoogleTest.cmake doesn't actually depend on Google Test as such; + // it only requires that we produces output in the expected format when + // invoked with --gtest_list_tests. Thus, we fake that here. This allows us + // to test the module without actually needing Google Test. + if (argc > 1 && std::string(argv[1]) == "--gtest_list_tests") { + std::cout << "flush_script_test.\n"; + const size_t flushThreshold = 50000; + const size_t testCaseNum = 4; + std::string testName(flushThreshold / (testCaseNum - 1), 'T'); + for (size_t i = 0; i < testCaseNum; ++i) + std::cout << " " << testName.c_str() << "\n"; + } + return 0; +} diff --git a/Tests/RunCMake/GoogleTest/test_list_test.cpp b/Tests/RunCMake/GoogleTest/test_list_test.cpp new file mode 100644 index 0000000..c9f9512 --- /dev/null +++ b/Tests/RunCMake/GoogleTest/test_list_test.cpp @@ -0,0 +1,18 @@ +#include +#include + +int main(int argc, char** argv) +{ + // Note: GoogleTest.cmake doesn't actually depend on Google Test as such; + // it only requires that we produces output in the expected format when + // invoked with --gtest_list_tests. Thus, we fake that here. This allows us + // to test the module without actually needing Google Test. + if (argc > 1 && std::string(argv[1]) == "--gtest_list_tests") { + std::cout << "test_list_test/test.\n"; + std::cout << " case/0 # GetParam() = \"semicolon;\"\n"; + std::cout << " case/1 # GetParam() = 'osb['\n"; + std::cout << " case/2 # GetParam() = 'csb]'\n"; + std::cout << " case/3 # GetParam() = 'S p a c e s'\n"; + } + return 0; +} -- cgit v0.12