Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

#

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Set the right path for PODIO_SIOBLOCK_PATH and add an error message #588

Merged
merged 4 commits into from
Apr 19, 2024
Merged

Set the right path for PODIO_SIOBLOCK_PATH and add an error message #588

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
81d0292
Use LD_LIBRARY_PATH instead of DL_PATHS for Catch2 tests
jmcarcell Apr 17, 2024
f18dc50
Add a message when PODIO_SIOBLOCK_PATH is set but no library is found
jmcarcell Apr 18, 2024
bbd8b2d
Set PODIO_SIOBLOCK_PATH to PROJECT_BINARY_DIR/tests where *SioBlocks.…
jmcarcell Apr 18, 2024
852b0e2
Remove the SKIP_CATCH_DISCOVERY option
jmcarcell Apr 18, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions src/SIOBlock.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,11 @@ std::vector<std::tuple<std::string, std::string>> SIOBlockLibraryLoader::getLibN
libs.emplace_back(std::move(filename), dir);
}
}
if (std::getenv("PODIO_SIOBLOCK_PATH") && libs.empty()) {
throw std::runtime_error(
"No SIOBlocks libraries found in PODIO_SIOBLOCK_PATH. Please set PODIO_SIOBLOCK_PATH to the directory "
"containing the SIOBlocks libraries or unset it to fallback to LD_LIBRARY_PATH.");
}
}

return libs;
Expand Down
19 changes: 5 additions & 14 deletions tests/unittests/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,21 +61,11 @@ if (NOT FORCE_RUN_ALL_TESTS)
endif()
endif()

option(SKIP_CATCH_DISCOVERY "Skip the Catch2 test discovery" OFF)

# To work around https://github.com/catchorg/Catch2/issues/2424 we need the
# DL_PATH argument for catch_discoer_tests which requires CMake 3.22 at least
# The whole issue can be avoided if we skip the catch test discovery and set the
# environment on our own
if (CMAKE_VERSION VERSION_LESS 3.22)
set(SKIP_CATCH_DISCOVERY ON)
endif()

if (USE_SANITIZER MATCHES "Memory(WithOrigin)?" OR SKIP_CATCH_DISCOVERY)
if (USE_SANITIZER MATCHES "Memory(WithOrigin)?")
# Automatic test discovery fails with Memory sanitizers due to some issues in
# Catch2. So in that case we skip the discovery step and simply run the thing
# directly in the tests.
if (FORCE_RUN_ALL_TESTS OR SKIP_CATCH_DISCOVERY)
if (FORCE_RUN_ALL_TESTS)
# Unfortunately Memory sanitizer seems to be really unhappy with Catch2 and
# it fails to successfully launch the executable and execute any test. Here
# we just include them in order to have them show up as failing
Expand All @@ -88,9 +78,10 @@ else()
WORKING_DIRECTORY ${CMAKE_CURRENT_LIST_DIR}
TEST_PREFIX "UT_" # make it possible to filter easily with -R ^UT
TEST_SPEC ${filter_tests} # discover only tests that are known to not fail
DL_PATHS ${CMAKE_CURRENT_BINARY_DIR}:${PROJECT_BINARY_DIR}/src:${PROJECT_BINARY_DIR}/tests:$<TARGET_FILE_DIR:ROOT::Tree>:$<$<TARGET_EXISTS:SIO::sio>:$<TARGET_FILE_DIR:SIO::sio>>:$ENV{LD_LIBRARY_PATH}
PROPERTIES
ENVIRONMENT
PODIO_SIOBLOCK_PATH=${CMAKE_CURRENT_BINARY_DIR}
PODIO_SIOBLOCK_PATH=${PROJECT_BINARY_DIR}/tests
ENVIRONMENT
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this second ENVIRONMENT necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Having a look at the comment, I think the SKIP_CATCH_DISCOVERY option can be removed; the point of that part is to check if the cmake version is less than 3.22 because in principle using DL_PATH is needed and this is supported only for 3.22 and above. But, it is actually not needed, because duplicating the ENVIRONMENT keyword works (it's described in the Catch2 issue).

LD_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}:${PROJECT_BINARY_DIR}/src:${PROJECT_BINARY_DIR}/tests:$<TARGET_FILE_DIR:ROOT::Tree>:$<$<TARGET_EXISTS:SIO::sio>:$<TARGET_FILE_DIR:SIO::sio>>:$ENV{LD_LIBRARY_PATH}
)
endif()
Loading