forked from catchorg/Catch2
Fix <target>_TESTS variable generated by catch_discover_tests
There were 2 separate bugs, one recently added and one ancient (since roughly the first version of the registration script). 1) The recent refactoring of how the JSON output from Catch2 is parsed caused shadowing between the variable containing the per-test JSON fragments and the accumulator of test names for <target>_TESTS variable. This led to the variable containing both the names and the JSON fragments, and thus being completely wrong. 2) The test name accumulation has never accounted for characters that need escaping to be present in a CMake list. This means that e.g. test names with semicolon in them would end up with two elements in the test list. Both of these are now fixed, at the cost of extra complexity and my sanity as I had to learn more about CMake escaping rules. (CMake escaping rules are dumb)
This commit is contained in:
Martin Hořeňovský
@@ -58,6 +58,11 @@ same as the Catch name; see also ``TEST_PREFIX`` and ``TEST_SUFFIX``.
|
||||
directory property. The set of discovered tests is made accessible to such a
|
||||
script via the ``<target>_TESTS`` variable.
|
||||
|
||||
Note that ``<target>_TESTS`` variable contains test names with brackets
|
||||
("[", "]") escaped into ASCII char 2, 3 respectively, to work around CMake's
|
||||
list parsing rules. You have to unescape them back for each element to get
|
||||
the original names.
|
||||
|
||||
The options are:
|
||||
|
||||
``target``
|
||||
|
||||
@@ -12,14 +12,28 @@
|
||||
# exist in JSON unescaped.
|
||||
#
|
||||
# 0x01 <=> ';' (CMake list separator)
|
||||
# 0x02 == element boundary marker used while splitting the tests array
|
||||
# 0x03 <-> '[' (opens a CMake bracket-argument context)
|
||||
# 0x04 <-> ']' (closes a CMake bracket-argument context)
|
||||
# 0x02 <-> '[' (opens a CMake bracket-argument context)
|
||||
# 0x03 <-> ']' (closes a CMake bracket-argument context)
|
||||
# 0x04 == element boundary marker used while splitting the tests array
|
||||
#
|
||||
string(ASCII 1 _SemicolonEscape)
|
||||
string(ASCII 2 _BoundaryEscape)
|
||||
string(ASCII 3 _OpenBracketEscape)
|
||||
string(ASCII 4 _CloseBracketEscape)
|
||||
string(ASCII 2 _OpenBracketEscape)
|
||||
string(ASCII 3 _CloseBracketEscape)
|
||||
string(ASCII 4 _BoundaryEscape)
|
||||
|
||||
|
||||
# As far as I can tell, the only way of having unclosed '[' or ']' in the
|
||||
# list of test names we define for CTest, is to escape them into completely
|
||||
# different characters. Otherwise, they will break parsing of unrelated
|
||||
# semicolons (inserted by CMake after parsing the list from the user-facing
|
||||
# space separated format). This means that the consumers of the list have
|
||||
# to unescape them back, but that's CMake :v
|
||||
#
|
||||
# 0x02 <-> '['
|
||||
# 0x03 <-> ']'
|
||||
#
|
||||
string(ASCII 2 _OpenBracketListingEscape)
|
||||
string(ASCII 3 _CloseBracketListingEscape)
|
||||
|
||||
|
||||
# Placeholder bytes in the listing would break our parsing hack, so
|
||||
@@ -221,6 +235,10 @@ function(catch_discover_tests_impl)
|
||||
set(script)
|
||||
set(suite)
|
||||
set(tests)
|
||||
# Holds the list of all registered test names **as a string**, not list.
|
||||
# This avoids issue of CMake removing semicolon escapes even inside
|
||||
# bracket-quoted strings.
|
||||
set(_test_names)
|
||||
|
||||
if(WIN32)
|
||||
set(dl_paths_variable_name PATH)
|
||||
@@ -431,13 +449,25 @@ function(catch_discover_tests_impl)
|
||||
string(APPEND script "${_Command}")
|
||||
endif()
|
||||
|
||||
list(APPEND tests "${prefix}${plain_name}${suffix}")
|
||||
# The test name has to be escaped using the same rules as prepare_command
|
||||
# uses for the arguments, so that it keeps being single element in list
|
||||
# even with weird characters and semicolons.
|
||||
set(full_list_name "${prefix}${plain_name}${suffix}")
|
||||
string(REPLACE ";" "\\;" full_list_name "${full_list_name}")
|
||||
string(REPLACE "[" "${_OpenBracketListingEscape}" full_list_name "${full_list_name}")
|
||||
string(REPLACE "]" "${_CloseBracketListingEscape}" full_list_name "${full_list_name}")
|
||||
if(full_list_name MATCHES "[^-./:a-zA-Z0-9_]")
|
||||
# The space before the start of quote is important, so that we get
|
||||
# space-separated list in the final file.
|
||||
string(APPEND _test_names " [==[${full_list_name}]==]")
|
||||
else()
|
||||
string(APPEND _test_names " ${full_list_name}")
|
||||
endif()
|
||||
endforeach()
|
||||
|
||||
# Create a list of all discovered tests, which users may use to e.g. set
|
||||
# properties on the tests
|
||||
prepare_command(set ${_TEST_LIST} ${tests})
|
||||
string(APPEND script "${_Command}")
|
||||
string(APPEND script "set(${_TEST_LIST}${_test_names})\n")
|
||||
|
||||
# Write any script leftovers we have
|
||||
file(APPEND "${_CTEST_FILE}" "${script}")
|
||||
|
||||
@@ -7,6 +7,7 @@
|
||||
|
||||
# SPDX-License-Identifier: BSL-1.0
|
||||
|
||||
import glob
|
||||
import os
|
||||
import subprocess
|
||||
import sys
|
||||
@@ -65,7 +66,6 @@ def build_project(sources_dir, output_base_path, catch2_path):
|
||||
return build_dir
|
||||
|
||||
|
||||
|
||||
def get_test_names(build_path: str) -> List[TestInfo]:
|
||||
# For now we assume that Windows builds are done using MSBuild under
|
||||
# Debug configuration. This means that we need to add "Debug" folder
|
||||
@@ -97,6 +97,7 @@ def get_test_names(build_path: str) -> List[TestInfo]:
|
||||
|
||||
return tests
|
||||
|
||||
|
||||
def get_ctest_listing(build_path):
|
||||
old_path = os.getcwd()
|
||||
os.chdir(build_path)
|
||||
@@ -137,6 +138,7 @@ def extract_tests_from_ctest(ctest_output) -> List[TestInfo]:
|
||||
|
||||
return test_infos
|
||||
|
||||
|
||||
def check_DL_PATHS(ctest_output):
|
||||
ctest_response = json.loads(ctest_output)
|
||||
tests = ctest_response['tests']
|
||||
@@ -146,6 +148,98 @@ def check_DL_PATHS(ctest_output):
|
||||
if property['name'] == 'ENVIRONMENT_MODIFICATION':
|
||||
assert len(property['value']) == 2, f"The test provides 2 arguments to DL_PATHS, but instead found {len(property['value'])}"
|
||||
|
||||
|
||||
def add_test_list_extractor(build_path: str) -> str:
|
||||
# The actual CTest script file has one of two names:
|
||||
# * `<target>-<short-hash>_tests.cmake` on single-config generators
|
||||
# * `<target>-<short-hash>_tests-<Config>.cmake` on multi-config generators
|
||||
#
|
||||
# We know the target name (`tests`), so we glob for the hash part
|
||||
patterns = [
|
||||
os.path.join(build_path, 'tests-*_tests.cmake'),
|
||||
os.path.join(build_path, 'tests-*_tests-Debug.cmake'),
|
||||
]
|
||||
matches = []
|
||||
for pattern in patterns:
|
||||
matches.extend(glob.glob(pattern))
|
||||
if len(matches) != 1:
|
||||
print(f"Found {len(matches)} CTest files in '{build_path}'. Expected only 1.")
|
||||
exit(5)
|
||||
test_script_file = matches[0]
|
||||
basename = os.path.basename(test_script_file)
|
||||
extractor_fname = os.path.join(build_path, 'extractor.cmake')
|
||||
with open(extractor_fname, 'w') as f:
|
||||
f.write(fr"""
|
||||
cmake_minimum_required(VERSION 3.19)
|
||||
|
||||
cmake_policy(VERSION 3.19...4.4)
|
||||
|
||||
# This dummies out the `add_test` and `set_tests_properties` commands
|
||||
# inside the CTest script, so we can include it during CMake script call.
|
||||
macro(add_test)
|
||||
endmacro()
|
||||
macro(set_tests_properties)
|
||||
endmacro()
|
||||
|
||||
include(${{CMAKE_CURRENT_LIST_DIR}}/{basename})
|
||||
|
||||
list(LENGTH tests_TESTS num_tests)
|
||||
message(STATUS "NUM TESTS: ${{num_tests}}")
|
||||
|
||||
# '[' and ']' were escaped into ASCII 2 and 3 respectively, we have to
|
||||
# unescape them back here. Note that this has to be done per-element,
|
||||
# or CMake's list parsing breaks (which is why they were escaped).
|
||||
string(ASCII 2 _LeftBracketListingEscape)
|
||||
string(ASCII 3 _RightBracketListingEscape)
|
||||
|
||||
foreach(test IN LISTS tests_TESTS)
|
||||
string(REPLACE "${{_LeftBracketListingEscape}}" "[" test "${{test}}")
|
||||
string(REPLACE "${{_RightBracketListingEscape}}" "]" test "${{test}}")
|
||||
string(REPLACE "\\" "\\\\" test "${{test}}")
|
||||
string(REPLACE "\r" "\\r" test "${{test}}")
|
||||
string(REPLACE "\n" "\\n" test "${{test}}")
|
||||
message(STATUS "TEST_NAME: ${{test}}")
|
||||
endforeach()
|
||||
""")
|
||||
return extractor_fname
|
||||
|
||||
|
||||
def extract_tests_list_from_ctest_script(build_path: str) -> List[str]:
|
||||
extractor = add_test_list_extractor(build_path)
|
||||
|
||||
cmd = ['cmake', '-P', extractor]
|
||||
|
||||
try:
|
||||
result = subprocess.run(cmd,
|
||||
capture_output = True,
|
||||
check = True,
|
||||
text = True)
|
||||
except subprocess.CalledProcessError as err:
|
||||
print('Error when calling CTest test extractor')
|
||||
print(f'cmd: {err.cmd}')
|
||||
print(f'stderr: {err.stderr}')
|
||||
print(f'stdout: {err.stdout}')
|
||||
exit(4)
|
||||
|
||||
lines = result.stdout.strip().split('\n')
|
||||
test_num_line = lines[0]
|
||||
test_lines = lines[1:]
|
||||
|
||||
test_num_prefix = '-- NUM TESTS: '
|
||||
assert test_num_prefix in test_num_line, test_num_line
|
||||
test_num_line = test_num_line[len(test_num_prefix):]
|
||||
num_tests = int(test_num_line)
|
||||
|
||||
assert num_tests == len(test_lines), len(test_lines)
|
||||
test_name_prefix = '-- TEST_NAME: '
|
||||
assert all(test_name_prefix in x for x in test_lines)
|
||||
test_names = [x[len(test_name_prefix):] for x in test_lines]
|
||||
|
||||
# Unescape the names, so that names with literal newlines have newlines in them again
|
||||
test_names = [x.encode('utf-8').decode('unicode-escape') for x in test_names]
|
||||
return test_names
|
||||
|
||||
|
||||
def escape_catch2_test_names(infos: List[TestInfo]):
|
||||
escaped = []
|
||||
for info in infos:
|
||||
@@ -155,6 +249,7 @@ def escape_catch2_test_names(infos: List[TestInfo]):
|
||||
escaped.append(TestInfo(name, info.tags))
|
||||
return escaped
|
||||
|
||||
|
||||
if __name__ == '__main__':
|
||||
if len(sys.argv) != 3:
|
||||
print(f'Usage: {sys.argv[0]} path-to-catch2-cml output-path')
|
||||
@@ -165,7 +260,8 @@ if __name__ == '__main__':
|
||||
|
||||
build_path = build_project(sources_dir, output_base_path, catch2_path)
|
||||
|
||||
catch_test_names = escape_catch2_test_names(get_test_names(build_path))
|
||||
raw_catch_test_names = get_test_names(build_path)
|
||||
catch_test_names = escape_catch2_test_names(raw_catch_test_names)
|
||||
ctest_output = get_ctest_listing(build_path)
|
||||
ctest_test_names = extract_tests_from_ctest(ctest_output)
|
||||
|
||||
@@ -182,7 +278,20 @@ if __name__ == '__main__':
|
||||
if mismatched:
|
||||
print(f"Found {mismatched} mismatched tests catch test names and ctest test commands!")
|
||||
exit(1)
|
||||
print(f"{len(catch_test_names)} tests matched")
|
||||
print(f"{len(catch_test_names)} tests matched in CTest listing")
|
||||
|
||||
test_list_names = sorted(extract_tests_list_from_ctest_script(build_path))
|
||||
expected_names = sorted(info.name for info in raw_catch_test_names)
|
||||
if test_list_names != expected_names:
|
||||
print("TEST_LIST variable (tests_TESTS) does not match Catch2 test listing!")
|
||||
for name in test_list_names:
|
||||
if name not in expected_names:
|
||||
print(f" TEST_LIST name '{name}' not in Catch2 listing")
|
||||
for name in expected_names:
|
||||
if name not in test_list_names:
|
||||
print(f" Catch2 name '{name}' not in TEST_LIST")
|
||||
exit(1)
|
||||
print(f"{len(test_list_names)} tests matched in TEST_LIST variable")
|
||||
|
||||
cmake_version = get_cmake_version()
|
||||
if cmake_version >= (3, 27):
|
||||
|
||||
@@ -40,7 +40,11 @@ public:
|
||||
|
||||
TEST_CASE_METHOD(TestCaseFixture, "A test case as method", "[tagstagstags]") {}
|
||||
|
||||
TEST_CASE("Unclosed right ) parenthesis") {}
|
||||
TEST_CASE("Unclosed left ( parenthesis") {}
|
||||
|
||||
TEST_CASE( "Newlines\nAnd\rOther\n\tWhitespace", "[whitespace-going-wild]" ) {}
|
||||
TEST_CASE( "Escaped \\n newline and \\r other whitespace", "[whitespace-going-wild]") {}
|
||||
|
||||
// Some JSON-like and JSON-adjacent characters and substrings in the test names/tags
|
||||
// This serves to test that the parse-json-via-string-splitting hack in
|
||||
|
||||
Reference in New Issue
Block a user