From 0bae7b6612a6f63fbd06d1895628df65db61c334 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Thu, 19 Nov 2020 20:46:11 +0000 Subject: [PATCH 1/4] Fix another failing test on CentOS 7. Replace the use of inject_on_return with patch_and_return. Signed-off-by: Chris Lalancette --- rcl/test/rcl/test_node.cpp | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index c21576126..9e93d9175 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -511,15 +511,21 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_init_with_i EXPECT_EQ(RCL_RET_ERROR, ret); rcl_reset_error(); } + // Try normal init but force an internal error on fini. { ret = rcl_node_init(&node, name, namespace_, &context, &options); EXPECT_EQ(RCL_RET_OK, ret); - auto mock = mocking_utils::inject_on_return("lib:rcl", rmw_destroy_node, RMW_RET_ERROR); - ret = rcl_node_fini(&node); - EXPECT_EQ(RCL_RET_ERROR, ret); + auto mock = mocking_utils::patch_and_return( + "lib:rcl", rcl_logging_rosout_fini_publisher_for_node, RCL_RET_ERROR); + EXPECT_EQ(RCL_RET_ERROR, rcl_node_fini(&node)); rcl_reset_error(); } + // This takes advantage of the fact that the very first call in + // `rcl_node_fini` (`rcl_logging_rosout_fini_publisher_for_node`) was patched + // for failure. Because of this, we can stop the patching, and just call + // `rcl_node_fini` to really cleanup. + EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node)); // Battle test node init. RCUTILS_FAULT_INJECTION_TEST( From 6775df3c1d4cde248faf6ebbaac9c174e02a52fb Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 30 Nov 2020 20:22:25 +0000 Subject: [PATCH 2/4] Revert "Fix another failing test on CentOS 7." This reverts commit 0bae7b6612a6f63fbd06d1895628df65db61c334. Signed-off-by: Chris Lalancette --- rcl/test/rcl/test_node.cpp | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/rcl/test/rcl/test_node.cpp b/rcl/test/rcl/test_node.cpp index 9e93d9175..c21576126 100644 --- a/rcl/test/rcl/test_node.cpp +++ b/rcl/test/rcl/test_node.cpp @@ -511,21 +511,15 @@ TEST_F(CLASSNAME(TestNodeFixture, RMW_IMPLEMENTATION), test_rcl_node_init_with_i EXPECT_EQ(RCL_RET_ERROR, ret); rcl_reset_error(); } - // Try normal init but force an internal error on fini. { ret = rcl_node_init(&node, name, namespace_, &context, &options); EXPECT_EQ(RCL_RET_OK, ret); - auto mock = mocking_utils::patch_and_return( - "lib:rcl", rcl_logging_rosout_fini_publisher_for_node, RCL_RET_ERROR); - EXPECT_EQ(RCL_RET_ERROR, rcl_node_fini(&node)); + auto mock = mocking_utils::inject_on_return("lib:rcl", rmw_destroy_node, RMW_RET_ERROR); + ret = rcl_node_fini(&node); + EXPECT_EQ(RCL_RET_ERROR, ret); rcl_reset_error(); } - // This takes advantage of the fact that the very first call in - // `rcl_node_fini` (`rcl_logging_rosout_fini_publisher_for_node`) was patched - // for failure. Because of this, we can stop the patching, and just call - // `rcl_node_fini` to really cleanup. - EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node)); // Battle test node init. RCUTILS_FAULT_INJECTION_TEST( From f7577f5be7f9cf08fdcb1f1861b60ee61c69dee2 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Mon, 30 Nov 2020 20:58:02 +0000 Subject: [PATCH 3/4] Skip the test on CentOS. Instead of trying to fix the test on CentOS, just skip it. This relies on a file called /etc/os-release being available (which it is on CentOS and Ubuntu). If we see that file, we parse it and find the ID. If we see that the ID is "centos" (including the quotes), we skip compiling and running the test_node tests. Signed-off-by: Chris Lalancette --- rcl/test/CMakeLists.txt | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index ea98bb804..1b45c4758 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -23,6 +23,27 @@ set(extra_lib_dirs "${rcl_lib_dir}") set(test_resources_dir_name "resources") add_definitions(-DTEST_RESOURCES_DIRECTORY="${CMAKE_CURRENT_BINARY_DIR}/${test_resources_dir_name}") +set(DISTRIBUTION "Unknown") +if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") + # If we are on Linux, look for /etc/os-release, which has "key=value" items. + # Parse those items, looking for a key named "id" (case-insensitive), and + # if we find it, set DISTRIBUTION to that value. That gives us some idea + # of which Linux distribution we are on. + if(EXISTS "/etc/os-release") + file(STRINGS "/etc/os-release" OS_RELEASE) + foreach(line ${OS_RELEASE}) + string(REGEX REPLACE "^(.*)=.*" "\\1" key "${line}") + string(TOLOWER "${key}" key) + if("${key}" STREQUAL "id") + string(REGEX REPLACE "^.*=(.*)" "\\1" DISTRIBUTION "${line}") + string(TOLOWER "${DISTRIBUTION}" DISTRIBUTION) + message(STATUS "CHRIS: ${DISTRIBUTION}") + break() + endif() + endforeach() + endif() +endif() + # finding gtest once in the highest scope # prevents finding it repeatedly in each local scope ament_find_gtest() @@ -162,14 +183,16 @@ function(test_target_function) AMENT_DEPENDENCIES ${rmw_implementation} ) - rcl_add_custom_gtest(test_node${target_suffix} - SRCS rcl/test_node.cpp - ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var} - APPEND_LIBRARY_DIRS ${extra_lib_dirs} - LIBRARIES ${PROJECT_NAME} mimick osrf_testing_tools_cpp::memory_tools - AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" - TIMEOUT 240 # Large timeout to wait for fault injection tests - ) + if(NOT "${DISTRIBUTION}" STREQUAL "\"centos\"") + rcl_add_custom_gtest(test_node${target_suffix} + SRCS rcl/test_node.cpp + ENV ${rmw_implementation_env_var} ${memory_tools_ld_preload_env_var} + APPEND_LIBRARY_DIRS ${extra_lib_dirs} + LIBRARIES ${PROJECT_NAME} mimick osrf_testing_tools_cpp::memory_tools + AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" + TIMEOUT 240 # Large timeout to wait for fault injection tests + ) + endif() rcl_add_custom_gtest(test_arguments${target_suffix} SRCS rcl/test_arguments.cpp From 111cb412aaee235fac83f5369157df9750f427c6 Mon Sep 17 00:00:00 2001 From: Chris Lalancette Date: Tue, 1 Dec 2020 20:08:32 +0000 Subject: [PATCH 4/4] Fixes from review. Signed-off-by: Chris Lalancette --- rcl/test/CMakeLists.txt | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index 1b45c4758..3a11666a1 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -37,7 +37,6 @@ if("${CMAKE_SYSTEM_NAME}" STREQUAL "Linux") if("${key}" STREQUAL "id") string(REGEX REPLACE "^.*=(.*)" "\\1" DISTRIBUTION "${line}") string(TOLOWER "${DISTRIBUTION}" DISTRIBUTION) - message(STATUS "CHRIS: ${DISTRIBUTION}") break() endif() endforeach() @@ -183,6 +182,9 @@ function(test_target_function) AMENT_DEPENDENCIES ${rmw_implementation} ) + # CentOS does not support mimick's inject_on_return functionality, which + # causes test_node to segfault when it runs. Since CentOS is not a Tier 1 + # platform, just disable the test since we are covering it elsewhere. if(NOT "${DISTRIBUTION}" STREQUAL "\"centos\"") rcl_add_custom_gtest(test_node${target_suffix} SRCS rcl/test_node.cpp