From c62a647d88ffd6be6d7ff5e776216a2cc7c52a12 Mon Sep 17 00:00:00 2001 From: brawner Date: Mon, 31 Aug 2020 12:59:19 -0700 Subject: [PATCH] Add fault injection macros and unit tests to rcl_lifecycle (#731) * Add fault injection macros and unit tests to rcl_lifecycle Signed-off-by: Stephen Brawner * Address feedback Signed-off-by: Stephen Brawner * PR Fixup Signed-off-by: Stephen Brawner --- rcl_lifecycle/CMakeLists.txt | 10 +++ rcl_lifecycle/src/rcl_lifecycle.c | 3 + rcl_lifecycle/src/transition_map.c | 7 ++ .../test/test_default_state_machine.cpp | 17 +++++ rcl_lifecycle/test/test_rcl_lifecycle.cpp | 65 +++++++++++++++++-- 5 files changed, 96 insertions(+), 6 deletions(-) diff --git a/rcl_lifecycle/CMakeLists.txt b/rcl_lifecycle/CMakeLists.txt index ce6e6eaa4..9344ba788 100644 --- a/rcl_lifecycle/CMakeLists.txt +++ b/rcl_lifecycle/CMakeLists.txt @@ -56,6 +56,10 @@ rcl_set_symbol_visibility_hidden(${PROJECT_NAME} LANGUAGE "C") # which is appropriate when building the dll but not consuming it. target_compile_definitions(rcl_lifecycle PRIVATE "RCL_LIFECYCLE_BUILDING_DLL") +if(BUILD_TESTING AND NOT RCUTILS_DISABLE_FAULT_INJECTION) + target_compile_definitions(${PROJECT_NAME} PUBLIC RCUTILS_ENABLE_FAULT_INJECTION) +endif() + install(TARGETS rcl_lifecycle EXPORT rcl_lifecycle ARCHIVE DESTINATION lib LIBRARY DESTINATION lib @@ -78,6 +82,9 @@ if(BUILD_TESTING) "osrf_testing_tools_cpp" ) target_link_libraries(test_default_state_machine ${PROJECT_NAME}) + target_compile_definitions(test_default_state_machine + PUBLIC RCUTILS_ENABLE_FAULT_INJECTION + ) endif() ament_add_gtest(test_multiple_instances test/test_multiple_instances.cpp @@ -98,6 +105,9 @@ if(BUILD_TESTING) "osrf_testing_tools_cpp" ) target_link_libraries(test_rcl_lifecycle ${PROJECT_NAME}) + target_compile_definitions(test_rcl_lifecycle + PUBLIC RCUTILS_ENABLE_FAULT_INJECTION + ) endif() ament_add_gtest(test_transition_map test/test_transition_map.cpp diff --git a/rcl_lifecycle/src/rcl_lifecycle.c b/rcl_lifecycle/src/rcl_lifecycle.c index 80a8ff0f6..ee3d7feff 100644 --- a/rcl_lifecycle/src/rcl_lifecycle.c +++ b/rcl_lifecycle/src/rcl_lifecycle.c @@ -27,6 +27,7 @@ extern "C" #include "rcl/error_handling.h" #include "rcutils/logging_macros.h" +#include "rcutils/macros.h" #include "rcutils/strdup.h" #include "rcl_lifecycle/default_state_machine.h" @@ -80,6 +81,8 @@ rcl_lifecycle_state_fini( rcl_lifecycle_state_t * state, const rcl_allocator_t * allocator) { + RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_ERROR); + if (!allocator) { RCL_SET_ERROR_MSG("can't free state, no allocator given\n"); return RCL_RET_ERROR; diff --git a/rcl_lifecycle/src/transition_map.c b/rcl_lifecycle/src/transition_map.c index 97562dc2b..083e05332 100644 --- a/rcl_lifecycle/src/transition_map.c +++ b/rcl_lifecycle/src/transition_map.c @@ -22,6 +22,7 @@ extern "C" #include #include "rcl/error_handling.h" +#include "rcl/macros.h" #include "rcutils/format_string.h" #include "rcl_lifecycle/transition_map.h" @@ -53,6 +54,7 @@ rcl_lifecycle_transition_map_fini( rcl_lifecycle_transition_map_t * transition_map, const rcutils_allocator_t * allocator) { + RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_ERROR); if (!allocator) { RCL_SET_ERROR_MSG("can't free transition map, no allocator given\n"); return RCL_RET_ERROR; @@ -85,6 +87,8 @@ rcl_lifecycle_register_state( rcl_lifecycle_state_t state, const rcutils_allocator_t * allocator) { + RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_ERROR); + if (rcl_lifecycle_get_state(transition_map, state.id) != NULL) { RCL_SET_ERROR_MSG_WITH_FORMAT_STRING("state %u is already registered\n", state.id); return RCL_RET_ERROR; @@ -116,6 +120,9 @@ rcl_lifecycle_register_transition( rcl_lifecycle_transition_t transition, const rcutils_allocator_t * allocator) { + RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_ERROR); + RCUTILS_CAN_RETURN_WITH_ERROR_OF(RCL_RET_BAD_ALLOC); + RCUTILS_CHECK_ALLOCATOR_WITH_MSG( allocator, "invalid allocator", return RCL_RET_ERROR) diff --git a/rcl_lifecycle/test/test_default_state_machine.cpp b/rcl_lifecycle/test/test_default_state_machine.cpp index afd66c3e0..79a0e77ac 100644 --- a/rcl_lifecycle/test/test_default_state_machine.cpp +++ b/rcl_lifecycle/test/test_default_state_machine.cpp @@ -28,6 +28,7 @@ #include "rcl/rcl.h" #include "rcutils/logging_macros.h" +#include "rcutils/testing/fault_injection.h" #include "rcl_lifecycle/rcl_lifecycle.h" #include "rcl_lifecycle/default_state_machine.h" @@ -833,3 +834,19 @@ TEST_F(TestDefaultStateMachine, default_sequence_error_unresolved) { rcl_lifecycle_state_machine_fini(&state_machine, this->node_ptr, this->allocator)); } } + +TEST_F(TestDefaultStateMachine, init_fini_maybe_fail) { + rcl_lifecycle_state_machine_t sm = rcl_lifecycle_get_zero_initialized_state_machine(); + RCUTILS_FAULT_INJECTION_TEST( + { + rcl_ret_t ret = rcl_lifecycle_init_default_state_machine(&sm, this->allocator); + if (RCL_RET_OK == ret) { + ret = rcl_lifecycle_state_machine_fini(&sm, this->node_ptr, this->allocator); + if (RCL_RET_OK != ret) { + EXPECT_EQ( + RCL_RET_OK, + rcl_lifecycle_state_machine_fini(&sm, this->node_ptr, this->allocator)); + } + } + }); +} diff --git a/rcl_lifecycle/test/test_rcl_lifecycle.cpp b/rcl_lifecycle/test/test_rcl_lifecycle.cpp index 6474d8c20..1571fc518 100644 --- a/rcl_lifecycle/test/test_rcl_lifecycle.cpp +++ b/rcl_lifecycle/test/test_rcl_lifecycle.cpp @@ -19,9 +19,12 @@ #include #include "rcl_lifecycle/rcl_lifecycle.h" + #include "osrf_testing_tools_cpp/memory_tools/memory_tools.hpp" #include "osrf_testing_tools_cpp/scope_exit.hpp" #include "rcl/error_handling.h" +#include "rcutils/testing/fault_injection.h" + #include "lifecycle_msgs/msg/transition_event.h" #include "lifecycle_msgs/srv/change_state.h" #include "lifecycle_msgs/srv/get_available_states.h" @@ -204,9 +207,9 @@ TEST(TestRclLifecycle, state_machine) { OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { - ASSERT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str; - ASSERT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str; - ASSERT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str; }); const rosidl_message_type_support_t * pn = @@ -364,9 +367,9 @@ TEST(TestRclLifecycle, state_transitions) { OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( { - ASSERT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str; - ASSERT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str; - ASSERT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_node_fini(&node)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)) << rcl_get_error_string().str; + EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)) << rcl_get_error_string().str; }); const rosidl_message_type_support_t * pn = @@ -436,3 +439,53 @@ TEST(TestRclLifecycle, state_transitions) { ret = rcl_lifecycle_state_machine_fini(&state_machine, &node, &allocator); EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; } + +TEST(TestRclLifecycle, init_fini_maybe_fail) { + rcl_node_t node = rcl_get_zero_initialized_node(); + rcl_allocator_t allocator = rcl_get_default_allocator(); + rcl_context_t context = rcl_get_zero_initialized_context(); + rcl_node_options_t options = rcl_node_get_default_options(); + rcl_init_options_t init_options = rcl_get_zero_initialized_init_options(); + rcl_ret_t ret = rcl_init_options_init(&init_options, allocator); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + + ret = rcl_init(0, nullptr, &init_options, &context); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + + OSRF_TESTING_TOOLS_CPP_SCOPE_EXIT( + { + EXPECT_EQ(RCL_RET_OK, rcl_shutdown(&context)); + EXPECT_EQ(RCL_RET_OK, rcl_context_fini(&context)); + }); + + ret = rcl_node_init(&node, "node", "namespace", &context, &options); + EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str; + + const rosidl_message_type_support_t * pn = + ROSIDL_GET_MSG_TYPE_SUPPORT(lifecycle_msgs, msg, TransitionEvent); + const rosidl_service_type_support_t * cs = + ROSIDL_GET_SRV_TYPE_SUPPORT(lifecycle_msgs, srv, ChangeState); + const rosidl_service_type_support_t * gs = + ROSIDL_GET_SRV_TYPE_SUPPORT(lifecycle_msgs, srv, GetState); + const rosidl_service_type_support_t * gas = + ROSIDL_GET_SRV_TYPE_SUPPORT(lifecycle_msgs, srv, GetAvailableStates); + const rosidl_service_type_support_t * gat = + ROSIDL_GET_SRV_TYPE_SUPPORT(lifecycle_msgs, srv, GetAvailableTransitions); + const rosidl_service_type_support_t * gtg = + ROSIDL_GET_SRV_TYPE_SUPPORT(lifecycle_msgs, srv, GetAvailableTransitions); + + RCUTILS_FAULT_INJECTION_TEST( + { + // Init segfaults if this is not zero initialized + rcl_lifecycle_state_machine_t sm = rcl_lifecycle_get_zero_initialized_state_machine(); + + ret = rcl_lifecycle_state_machine_init( + &sm, &node, pn, cs, gs, gas, gat, gtg, true, &allocator); + if (RCL_RET_OK == ret) { + ret = rcl_lifecycle_state_machine_fini(&sm, &node, &allocator); + if (RCL_RET_OK != ret) { + EXPECT_EQ(RCL_RET_OK, rcl_lifecycle_state_machine_fini(&sm, &node, &allocator)); + } + } + }); +}