From af438bcca1ffb69714c0b225ba0f80aceab04906 Mon Sep 17 00:00:00 2001 From: Jorge Perez Date: Mon, 27 Jul 2020 15:57:45 -0300 Subject: [PATCH] Reformat rmw_impl_id_check to call a testable function (#725) * Reformat rmw_impl_id_check to call a testable function * Reformat to expose the function in the public header * Reformat style return result * Expose macro names to be tested with the function checker * Add test for failing cases of the function * Set error variable and log in the main caller * Use format string for logging * Change name of checker function * Reset rcl error to avoid overwrite Signed-off-by: Jorge Perez --- .../rcl/rmw_implementation_identifier_check.h | 35 ++++++++++ .../rcl/rmw_implementation_identifier_check.c | 53 +++++++------- rcl/test/CMakeLists.txt | 8 +++ rcl/test/rcl/test_rmw_impl_id_check_func.cpp | 70 +++++++++++++++++++ 4 files changed, 142 insertions(+), 24 deletions(-) create mode 100644 rcl/include/rcl/rmw_implementation_identifier_check.h create mode 100644 rcl/test/rcl/test_rmw_impl_id_check_func.cpp diff --git a/rcl/include/rcl/rmw_implementation_identifier_check.h b/rcl/include/rcl/rmw_implementation_identifier_check.h new file mode 100644 index 000000000..2a1ecd35a --- /dev/null +++ b/rcl/include/rcl/rmw_implementation_identifier_check.h @@ -0,0 +1,35 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#ifndef RCL__RMW_IMPLEMENTATION_IDENTIFIER_CHECK_H_ +#define RCL__RMW_IMPLEMENTATION_IDENTIFIER_CHECK_H_ + +#ifdef __cplusplus +extern "C" +{ +#endif + +#include "rcl/visibility_control.h" + +#define RMW_IMPLEMENTATION_ENV_VAR_NAME "RMW_IMPLEMENTATION" +#define RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME "RCL_ASSERT_RMW_ID_MATCHES" + +RCL_PUBLIC +rcl_ret_t rcl_rmw_implementation_identifier_check(void); + +#ifdef __cplusplus +} +#endif + +#endif // RCL__RMW_IMPLEMENTATION_IDENTIFIER_CHECK_H_ diff --git a/rcl/src/rcl/rmw_implementation_identifier_check.c b/rcl/src/rcl/rmw_implementation_identifier_check.c index e66ee2168..9c068a67f 100644 --- a/rcl/src/rcl/rmw_implementation_identifier_check.c +++ b/rcl/src/rcl/rmw_implementation_identifier_check.c @@ -30,8 +30,7 @@ extern "C" #include "rcl/types.h" -#define RMW_IMPLEMENTATION_ENV_VAR_NAME "RMW_IMPLEMENTATION" -#define RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME "RCL_ASSERT_RMW_ID_MATCHES" +#include "rcl/rmw_implementation_identifier_check.h" // Extracted this portable method of doing a "shared library constructor" from SO: // http://stackoverflow.com/a/2390626/671658 @@ -55,7 +54,8 @@ extern "C" static void f(void) #endif -INITIALIZER(initialize) { +rcl_ret_t rcl_rmw_implementation_identifier_check(void) +{ // If the environment variable RMW_IMPLEMENTATION is set, or // the environment variable RCL_ASSERT_RMW_ID_MATCHES is set, // check that the result of `rmw_get_implementation_identifier` matches. @@ -66,18 +66,17 @@ INITIALIZER(initialize) { RMW_IMPLEMENTATION_ENV_VAR_NAME, &expected_rmw_impl_env); if (NULL != get_env_error_str) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( "Error getting env var '" RCUTILS_STRINGIFY(RMW_IMPLEMENTATION_ENV_VAR_NAME) "': %s\n", get_env_error_str); - exit(RCL_RET_ERROR); + return RCL_RET_ERROR; } if (strlen(expected_rmw_impl_env) > 0) { // Copy the environment variable so it doesn't get over-written by the next getenv call. expected_rmw_impl = rcutils_strdup(expected_rmw_impl_env, allocator); if (!expected_rmw_impl) { - RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "allocation failed"); - exit(RCL_RET_BAD_ALLOC); + RCL_SET_ERROR_MSG("allocation failed"); + return RCL_RET_BAD_ALLOC; } } @@ -86,31 +85,29 @@ INITIALIZER(initialize) { get_env_error_str = rcutils_get_env( RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, &asserted_rmw_impl_env); if (NULL != get_env_error_str) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( "Error getting env var '" RCUTILS_STRINGIFY(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME) "': %s\n", get_env_error_str); - exit(RCL_RET_ERROR); + return RCL_RET_ERROR; } if (strlen(asserted_rmw_impl_env) > 0) { // Copy the environment variable so it doesn't get over-written by the next getenv call. asserted_rmw_impl = rcutils_strdup(asserted_rmw_impl_env, allocator); if (!asserted_rmw_impl) { - RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "allocation failed"); - exit(RCL_RET_BAD_ALLOC); + RCL_SET_ERROR_MSG("allocation failed"); + return RCL_RET_BAD_ALLOC; } } // If both environment variables are set, and they do not match, print an error and exit. if (expected_rmw_impl && asserted_rmw_impl && strcmp(expected_rmw_impl, asserted_rmw_impl) != 0) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( "Values of RMW_IMPLEMENTATION ('%s') and RCL_ASSERT_RMW_ID_MATCHES ('%s') environment " "variables do not match, exiting with %d.", expected_rmw_impl, asserted_rmw_impl, RCL_RET_ERROR ); - exit(RCL_RET_ERROR); + return RCL_RET_ERROR; } // Collapse the expected_rmw_impl and asserted_rmw_impl variables so only expected_rmw_impl needs @@ -130,31 +127,39 @@ INITIALIZER(initialize) { // If either environment variable is set, and it does not match, print an error and exit. if (expected_rmw_impl) { const char * actual_rmw_impl_id = rmw_get_implementation_identifier(); + const rcutils_error_string_t rmw_error_msg = rcl_get_error_string(); + rcl_reset_error(); if (!actual_rmw_impl_id) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( "Error getting RMW implementation identifier / RMW implementation not installed " "(expected identifier of '%s'), with error message '%s', exiting with %d.", expected_rmw_impl, - rcl_get_error_string().str, + rmw_error_msg.str, RCL_RET_ERROR ); - rcl_reset_error(); - exit(RCL_RET_ERROR); + return RCL_RET_ERROR; } if (strcmp(actual_rmw_impl_id, expected_rmw_impl) != 0) { - RCUTILS_LOG_ERROR_NAMED( - ROS_PACKAGE_NAME, + RCL_SET_ERROR_MSG_WITH_FORMAT_STRING( "Expected RMW implementation identifier of '%s' but instead found '%s', exiting with %d.", expected_rmw_impl, actual_rmw_impl_id, RCL_RET_MISMATCHED_RMW_ID ); - exit(RCL_RET_MISMATCHED_RMW_ID); + return RCL_RET_MISMATCHED_RMW_ID; } // Free the memory now that all checking has passed. allocator.deallocate((char *)expected_rmw_impl, allocator.state); } + return RCL_RET_OK; +} + +INITIALIZER(initialize) { + rcl_ret_t ret = rcl_rmw_implementation_identifier_check(); + if (ret != RCL_RET_OK) { + RCUTILS_LOG_ERROR_NAMED(ROS_PACKAGE_NAME, "%s\n", rcl_get_error_string().str); + exit(ret); + } } #ifdef __cplusplus diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt index c6f823c11..9e0d85fdd 100644 --- a/rcl/test/CMakeLists.txt +++ b/rcl/test/CMakeLists.txt @@ -281,6 +281,14 @@ function(test_target_function) AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp" "test_msgs" ) + rcl_add_custom_gtest(test_rmw_impl_id_check_func${target_suffix} + SRCS rcl/test_rmw_impl_id_check_func.cpp + ENV ${rmw_implementation_env_var} + APPEND_LIBRARY_DIRS ${extra_lib_dirs} + LIBRARIES ${PROJECT_NAME} + AMENT_DEPENDENCIES ${rmw_implementation} + ) + # Launch tests rcl_add_custom_executable(service_fixture${target_suffix} diff --git a/rcl/test/rcl/test_rmw_impl_id_check_func.cpp b/rcl/test/rcl/test_rmw_impl_id_check_func.cpp new file mode 100644 index 000000000..6398a3878 --- /dev/null +++ b/rcl/test/rcl/test_rmw_impl_id_check_func.cpp @@ -0,0 +1,70 @@ +// Copyright 2020 Open Source Robotics Foundation, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include + +#include "rcutils/env.h" + +#include "rcl/error_handling.h" +#include "rcl/rcl.h" +#include "rcl/rmw_implementation_identifier_check.h" + +TEST(TestRmwCheck, test_rmw_check_id_impl) { + EXPECT_EQ(RCL_RET_OK, rcl_rmw_implementation_identifier_check()); +} + +TEST(TestRmwCheck, test_failing_configuration) { + const char * expected_rmw_impl_env = NULL; + const char * expected_rmw_id_matches = NULL; + + const char * get_env_var_name = rcutils_get_env( + RMW_IMPLEMENTATION_ENV_VAR_NAME, + &expected_rmw_impl_env); + + const char * get_env_id_matches_name = rcutils_get_env( + RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, + &expected_rmw_id_matches); + + // Fail test case, reason: RMW_IMPLEMENTATION_ENV_VAR_NAME set, not matching rmw impl + EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, "some_random_name")); + EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "")); + EXPECT_EQ(RCL_RET_MISMATCHED_RMW_ID, rcl_rmw_implementation_identifier_check()); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + // Fail test case, reason: RMW_IMPLEMENTATION_ENV_VAR_NAME set, not matching rmw impl + EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, "")); + EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "some_random_name")); + EXPECT_EQ(RCL_RET_MISMATCHED_RMW_ID, rcl_rmw_implementation_identifier_check()); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + // Fail test case, reason: env variables not equal + EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, "some_random_name")); + EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "diff_random")); + EXPECT_EQ(RCL_RET_ERROR, rcl_rmw_implementation_identifier_check()); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + // Fail test case, reason: equal env variables do not match rmw impl + EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "some_random_name")); + EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, "some_random_name")); + EXPECT_EQ(RCL_RET_MISMATCHED_RMW_ID, rcl_rmw_implementation_identifier_check()); + EXPECT_TRUE(rcl_error_is_set()); + rcl_reset_error(); + + // Restore env variables set in the test + EXPECT_TRUE(rcutils_set_env(RMW_IMPLEMENTATION_ENV_VAR_NAME, get_env_var_name)); + EXPECT_TRUE(rcutils_set_env(RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME, get_env_id_matches_name)); +}