Tests: Refactor handling of environment variables when invoking tests.

The current handling had a fairly bad issue: multiple calls to
`set_tests_properties` to set envvars of a same test.

This does not work, only the last call is effective, all previous
ones have absolutely no effect.

This has been addressed by moving all 'set envvar for test' logic into a
single CMake function, `blender_test_set_envvars`.

This function takes optional extra envvars if needed, and define a set
of default ones (currently, `PATH` from `PLATFORM_ENV_INSTALL` if
defined, and the 'nuke' `exitcode=0` `LSAN_OPTIONS` if relevant).

NOTE: The way `blender_test_set_envvars` handles extra envvars passed to
it as parameter is fairly basic and unsafe, in that there is no check
whether a same envvar is defined more than once. Think for now this is
an acceptable limitation.

NOTE: Although this commit _should_ be a non-functional change one, the
unification of the handling of all envvars makes it hard to ensure there is no
side effects.
The `PATH` envvar e.g. was set to either `PLATFORM_ENV_INSTALL` if defined,
or a copy of that variable's definition, but only in Windows case. So technically,
the behavior for this envvar is changed.
This commit is contained in:
Bastien Montagne 2024-01-05 21:13:54 +01:00 committed by Bastien Montagne
parent b0db5363fa
commit 3e744db9fe
2 changed files with 43 additions and 31 deletions

View File

@ -12,6 +12,43 @@ function(get_blender_test_install_dir VARIABLE_NAME)
set(${VARIABLE_NAME} "${TEST_INSTALL_DIR}" PARENT_SCOPE)
endfunction()
# Add the necessary LSAN/ASAN options to the given list of environment variables.
# Typically used after adding a test, before calling
# `set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${_envvar_list}")`,
# to ensure that it will run with the correct sanitizer settings.
#
# \param envvars_list: A list of extra environment variables to define for that test.
# Note that this does no check for (re-)definition of a same variable.
function(blender_test_set_envvars testname envvars_list)
if(PLATFORM_ENV_INSTALL)
list(APPEND envvar_list "${PLATFORM_ENV_INSTALL}")
endif()
if(NOT CMAKE_BUILD_TYPE MATCHES "Release")
if(WITH_COMPILER_ASAN)
# Don't fail tests on leaks since these often happen in external libraries that we can't fix.
# FIXME This is a 'nuke solution', no xSAN errors will ever fail tests. Needs more refined handling,
# see https://projects.blender.org/blender/blender/pulls/116635 .
set(_lsan_options "LSAN_OPTIONS=exitcode=0")
# FIXME That `allocator_may_return_null=true` ASAN option is only needed for the `guardedalloc` test,
# would be nice to allow tests definition to pass extra envvars better.
# NOTE: This is needed for Mac builds currently, on Linux the `exitcode=0` option passed above to LSAN
# also seems to silence reports from ASAN.
set(_asan_options "ASAN_OPTIONS=allocator_may_return_null=true")
if(DEFINED ENV{LSAN_OPTIONS})
set(_lsan_options "${_lsan_options}:$ENV{LSAN_OPTIONS}")
endif()
if(DEFINED ENV{ASAN_OPTIONS})
set(_asan_options "${_asan_options}:$ENV{ASAN_OPTIONS}")
endif()
list(APPEND envvar_list "${_lsan_options}" "${_asan_options}")
endif()
endif()
# Can only be called once per test to define its custom environment variables.
set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${envvar_list}")
endfunction()
macro(blender_src_gtest_ex)
if(WITH_GTESTS)
set(options)
@ -140,13 +177,8 @@ function(blender_add_ctests)
WORKING_DIRECTORY ${TEST_INSTALL_DIR}
)
endif()
blender_test_set_envvars("${ARGS_SUITE_NAME}" "")
if(WIN32)
set_tests_properties(
${ARGS_SUITE_NAME} PROPERTIES
ENVIRONMENT "PATH=${CMAKE_INSTALL_PREFIX_WITH_CONFIG}/blender.shared/;$ENV{PATH}"
)
endif()
unset(_test_release_dir)
endfunction()

View File

@ -22,28 +22,18 @@ file(MAKE_DIRECTORY ${TEST_OUT_DIR}/blendfile_io)
# endif()
# Run Blender command with parameters.
function(add_blender_test_impl testname exe)
function(add_blender_test_impl testname envvars_list exe)
add_test(
NAME ${testname}
COMMAND ${exe} ${ARGN}
)
# Don't fail tests on leaks since these often happen in external libraries that we can't fix.
set(_lsan_options "exitcode=0")
if(DEFINED ENV{LSAN_OPTIONS})
set(_lsan_options "${_lsan_options}:$ENV{LSAN_OPTIONS}")
endif()
set_tests_properties(${testname} PROPERTIES ENVIRONMENT "LSAN_OPTIONS=${_lsan_options}")
unset(_lsan_options)
if(PLATFORM_ENV_INSTALL)
set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${PLATFORM_ENV_INSTALL}")
endif()
blender_test_set_envvars("${testname}" "${envvars_list}")
endfunction()
function(add_blender_test testname)
add_blender_test_impl(
"${testname}"
""
"${TEST_BLENDER_EXE}"
${TEST_BLENDER_EXE_PARAMS}
${ARGN}
@ -89,6 +79,7 @@ The location of weston, leave blank for the default location."
list(REMOVE_ITEM EXE_PARAMS --background)
add_blender_test_impl(
"${testname}"
"${_blender_headless_env_vars}"
"${TEST_PYTHON_EXE}"
"${CMAKE_SOURCE_DIR}/tests/utils/blender_headless.py"
# NOTE: attempting to maximize the window causes problems with a headless `weston`,
@ -99,7 +90,6 @@ The location of weston, leave blank for the default location."
"${EXE_PARAMS}"
"${ARGN}"
)
set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${_blender_headless_env_vars}")
endfunction()
endif()
@ -114,17 +104,7 @@ function(add_python_test testname testscript)
COMMAND ${TEST_PYTHON_EXE} ${TEST_PYTHON_EXE_EXTRA_ARGS} ${testscript} ${ARGN}
WORKING_DIRECTORY $<TARGET_FILE_DIR:blender>
)
set(_lsan_options "exitcode=0")
if(DEFINED ENV{LSAN_OPTIONS})
set(_lsan_options "${_lsan_options}:$ENV{LSAN_OPTIONS}")
endif()
set_tests_properties(${testname} PROPERTIES ENVIRONMENT "LSAN_OPTIONS=${_lsan_options}")
unset(_lsan_options)
if(PLATFORM_ENV_INSTALL)
set_tests_properties(${testname} PROPERTIES ENVIRONMENT "${PLATFORM_ENV_INSTALL}")
endif()
blender_test_set_envvars("${testname}" "")
endfunction()
# Run Python render test.