You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
Describe the bug
The name of the test is passed directly to the constructor for the CTExecutionUnit that represents the test. This constructor has the type signature of str test_id, desc* friendly_name, bool expect_fail. If the friendly name is bool for example, then its type is inferred to be type, which is incompatible with desc*, resulting in a crash because a proper overload cannot be found. This crash occurs even when CMAKEPP_LANG_DEBUG_MODE is disabled, since it uses the type information to find the overload and not just for asserting the proper signature.
Expected behavior
At a minimum, the signature of ct_add_test() and ct_add_section() should be checked for proper types. Since these two functions use cmake_parse_arguments() with keyword arguments instead of position-only arguments, using cpp_assert_signature() with the ARGV is not an option. However, we can manually call cpp_assert_type() on the name to guarantee it's the correct type, and give the test developer a proper error message when it's not.
More importantly, we should formalize what are valid test identifiers/names. Currently, the only restriction we have is an implicit one: they must be a valid CMake variable identifier. In #87 we also added the implicit restriction that they must be desc, which precludes tests with names like bool. However, there can be cases where users want to name their tests bool or any other name. If we loosen the signature to allow any type name, then a section named bool will cause the value bool to become a pointer to a function in addition to it being a literal type. I am unsure if this should be allowed or not; so far it has not been an issue but I could see it becoming one in the future.
Additional context
Discovered after merging #87, CMakePPLang has tests with the names bool, desc, path, etc
The text was updated successfully, but these errors were encountered:
Describe the bug
The name of the test is passed directly to the constructor for the CTExecutionUnit that represents the test. This constructor has the type signature of
str test_id, desc* friendly_name, bool expect_fail
. If the friendly name isbool
for example, then its type is inferred to betype
, which is incompatible withdesc*
, resulting in a crash because a proper overload cannot be found. This crash occurs even whenCMAKEPP_LANG_DEBUG_MODE
is disabled, since it uses the type information to find the overload and not just for asserting the proper signature.Expected behavior
At a minimum, the signature of
ct_add_test()
andct_add_section()
should be checked for proper types. Since these two functions usecmake_parse_arguments()
with keyword arguments instead of position-only arguments, usingcpp_assert_signature()
with theARGV
is not an option. However, we can manually callcpp_assert_type()
on the name to guarantee it's the correct type, and give the test developer a proper error message when it's not.More importantly, we should formalize what are valid test identifiers/names. Currently, the only restriction we have is an implicit one: they must be a valid CMake variable identifier. In #87 we also added the implicit restriction that they must be
desc
, which precludes tests with names likebool
. However, there can be cases where users want to name their testsbool
or any other name. If we loosen the signature to allow any type name, then a section namedbool
will cause the valuebool
to become a pointer to a function in addition to it being a literal type. I am unsure if this should be allowed or not; so far it has not been an issue but I could see it becoming one in the future.Additional context
Discovered after merging #87, CMakePPLang has tests with the names
bool
,desc
,path
, etcThe text was updated successfully, but these errors were encountered: