Fix the lifecycle tests on RHEL-9. (#2583)

* Fix the lifecycle tests on RHEL-9. The full explanation is in the comment, but basically since RHEL doesn't support mocking_utils::inject_on_return, we have to split out certain tests to make sure resources within a process don't collide. Signed-off-by: Chris Lalancette <[email protected]> Co-authored-by: Alejandro Hernández Cordero <[email protected]>
ros2 · Jul 12, 2024 · 304b51c · 304b51c
1 parent 069a001
commit 304b51c
Show file tree

Hide file tree

Showing 3 changed files with 76 additions and 16 deletions.
diff --git a/rclcpp_lifecycle/CMakeLists.txt b/rclcpp_lifecycle/CMakeLists.txt
@@ -99,6 +99,13 @@ if(BUILD_TESTING)
   if(TARGET test_lifecycle_node)
     target_link_libraries(test_lifecycle_node ${PROJECT_NAME} mimick rcl_lifecycle::rcl_lifecycle rclcpp::rclcpp rcutils::rcutils)
   endif()
+
+  ament_add_gtest(test_lifecycle_node_errors test/test_lifecycle_node_errors.cpp)
+  ament_add_test_label(test_lifecycle_node_errors mimick)
+  if(TARGET test_lifecycle_node_errors)
+    target_link_libraries(test_lifecycle_node_errors ${PROJECT_NAME} mimick rcl_lifecycle::rcl_lifecycle)
+  endif()
+
   ament_add_gtest(test_lifecycle_publisher test/test_lifecycle_publisher.cpp)
   if(TARGET test_lifecycle_publisher)
     target_link_libraries(test_lifecycle_publisher ${PROJECT_NAME} rcl_lifecycle::rcl_lifecycle rclcpp::rclcpp ${test_msgs_TARGETS})

diff --git a/rclcpp_lifecycle/test/test_lifecycle_node.cpp b/rclcpp_lifecycle/test/test_lifecycle_node.cpp
@@ -237,22 +237,6 @@ TEST_F(TestDefaultStateMachine, empty_initializer) {
   EXPECT_EQ(State::PRIMARY_STATE_UNCONFIGURED, test_node->get_current_state().id());
 }
 
-TEST_F(TestDefaultStateMachine, empty_initializer_rcl_errors) {
-  {
-    auto patch = mocking_utils::inject_on_return(
-      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_init, RCL_RET_ERROR);
-    EXPECT_THROW(
-      std::make_shared<EmptyLifecycleNode>("testnode").reset(),
-      std::runtime_error);
-  }
-  {
-    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
-    auto patch = mocking_utils::inject_on_return(
-      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_fini, RCL_RET_ERROR);
-    EXPECT_NO_THROW(test_node.reset());
-  }
-}
-
 TEST_F(TestDefaultStateMachine, check_logger_services_exist) {
   // Logger level services are disabled
   {

diff --git a/rclcpp_lifecycle/test/test_lifecycle_node_errors.cpp b/rclcpp_lifecycle/test/test_lifecycle_node_errors.cpp
@@ -0,0 +1,69 @@
+// Copyright 2024 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 <memory>
+#include <stdexcept>
+#include <string>
+
+#include "rcl_lifecycle/rcl_lifecycle.h"
+
+#include "rclcpp_lifecycle/lifecycle_node.hpp"
+
+#include "./mocking_utils/patch.hpp"
+
+class TestDefaultStateMachine : public ::testing::Test
+{
+protected:
+  static void SetUpTestCase()
+  {
+    rclcpp::init(0, nullptr);
+  }
+  static void TearDownTestCase()
+  {
+    rclcpp::shutdown();
+  }
+};
+
+class EmptyLifecycleNode : public rclcpp_lifecycle::LifecycleNode
+{
+public:
+  explicit EmptyLifecycleNode(const std::string & node_name)
+  : rclcpp_lifecycle::LifecycleNode(node_name)
+  {}
+};
+
+// This test is split out of test_lifecycle_node.cpp for an esoteric reason.  When running on
+// RedHat-based distributions (like Fedora or RHEL), the way that glibc is compiled does not
+// allow mocking_utils::inject_on_return to work.  Thus the test has to patch_and_return().
+// Unfortunately, this means that the resources are not actually cleaned up, and thus other tests
+// may return incorrect results.  By having it in a separate process we ensure that the resources
+// will at least be cleaned up by the process dying.
+TEST_F(TestDefaultStateMachine, empty_initializer_rcl_errors)
+{
+  {
+    auto patch = mocking_utils::patch_and_return(
+      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_init, RCL_RET_ERROR);
+    EXPECT_THROW(
+      std::make_shared<EmptyLifecycleNode>("testnode").reset(),
+      std::runtime_error);
+  }
+  {
+    auto test_node = std::make_shared<EmptyLifecycleNode>("testnode");
+    auto patch = mocking_utils::patch_and_return(
+      "lib:rclcpp_lifecycle", rcl_lifecycle_state_machine_fini, RCL_RET_ERROR);
+    EXPECT_NO_THROW(test_node.reset());
+  }
+}