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 <jjperez@ekumenlabs.com>

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_

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"
+#include "rcl/rmw_implementation_identifier_check.h"
-#define RCL_ASSERT_RMW_ID_MATCHES_ENV_VAR_NAME "RCL_ASSERT_RMW_ID_MATCHES"
 
 // 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(
+    RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-      ROS_PACKAGE_NAME,
       "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");
+      RCL_SET_ERROR_MSG("allocation failed");
-      exit(RCL_RET_BAD_ALLOC);
+      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(
+    RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-      ROS_PACKAGE_NAME,
       "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");
+      RCL_SET_ERROR_MSG("allocation failed");
-      exit(RCL_RET_BAD_ALLOC);
+      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(
+    RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-      ROS_PACKAGE_NAME,
       "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(
+      RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-        ROS_PACKAGE_NAME,
         "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();
+      return RCL_RET_ERROR;
-      exit(RCL_RET_ERROR);
     }
     if (strcmp(actual_rmw_impl_id, expected_rmw_impl) != 0) {
-      RCUTILS_LOG_ERROR_NAMED(
+      RCL_SET_ERROR_MSG_WITH_FORMAT_STRING(
-        ROS_PACKAGE_NAME,
         "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

rcl/test/CMakeLists.txt

@@ -279,6 +279,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}

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 <gtest/gtest.h>
+
+#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));
+}