Increase test timeouts of slow running tests with rmw_connext_cpp (#1400

) * Increase test timeouts of slow running tests with rmw_connext_cpp Signed-off-by: Stephen Brawner <brawner@gmail.com> * Fix other issues with connext Signed-off-by: Stephen Brawner <brawner@gmail.com>
ros2 · Oct 19, 2020 · 504e68b · 504e68b
1 parent 5e8fff6
commit 504e68b
Show file tree

Hide file tree

Showing 3 changed files with 28 additions and 7 deletions.
diff --git a/rclcpp/test/CMakeLists.txt b/rclcpp/test/CMakeLists.txt
@@ -207,7 +207,8 @@ if(TARGET test_node_interfaces__node_clock)
   target_link_libraries(test_node_interfaces__node_clock ${PROJECT_NAME})
 endif()
 ament_add_gtest(test_node_interfaces__node_graph
-  rclcpp/node_interfaces/test_node_graph.cpp)
+  rclcpp/node_interfaces/test_node_graph.cpp
+  TIMEOUT 120)
 if(TARGET test_node_interfaces__node_graph)
   ament_target_dependencies(
     test_node_interfaces__node_graph
@@ -322,7 +323,7 @@ ament_add_gtest(test_parameter_map rclcpp/test_parameter_map.cpp)
 if(TARGET test_parameter_map)
   target_link_libraries(test_parameter_map ${PROJECT_NAME})
 endif()
-ament_add_gtest(test_publisher rclcpp/test_publisher.cpp)
+ament_add_gtest(test_publisher rclcpp/test_publisher.cpp TIMEOUT 120)
 if(TARGET test_publisher)
   ament_target_dependencies(test_publisher
     "rcl"
@@ -405,7 +406,8 @@ if(TARGET test_service)
   )
   target_link_libraries(test_service ${PROJECT_NAME} mimick)
 endif()
-ament_add_gtest(test_subscription rclcpp/test_subscription.cpp)
+# Creating and destroying nodes is slow with Connext, so this needs larger timeout.
+ament_add_gtest(test_subscription rclcpp/test_subscription.cpp TIMEOUT 120)
 if(TARGET test_subscription)
   ament_target_dependencies(test_subscription
     "rcl_interfaces"
@@ -556,7 +558,7 @@ if(TARGET test_multi_threaded_executor)
 endif()
 
 ament_add_gtest(test_static_executor_entities_collector rclcpp/executors/test_static_executor_entities_collector.cpp
-  APPEND_LIBRARY_DIRS "${append_library_dirs}")
+  APPEND_LIBRARY_DIRS "${append_library_dirs}" TIMEOUT 120)
 if(TARGET test_static_executor_entities_collector)
   ament_target_dependencies(test_static_executor_entities_collector
     "rcl"
@@ -632,7 +634,7 @@ endif()
 
 ament_add_gtest(test_executor rclcpp/test_executor.cpp
   APPEND_LIBRARY_DIRS "${append_library_dirs}"
-)
+  TIMEOUT 120)
 if(TARGET test_executor)
   ament_target_dependencies(test_executor "rcl")
   target_link_libraries(test_executor ${PROJECT_NAME} mimick)

diff --git a/rclcpp/test/rclcpp/test_publisher.cpp b/rclcpp/test/rclcpp/test_publisher.cpp
@@ -29,6 +29,10 @@
 
 #include "test_msgs/msg/empty.hpp"
 
+// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check
+// that this test can complete fully, or adjust the timeout as necessary.
+// See https://github.com/ros2/rmw_connext/issues/325 for resolution
+
 class TestPublisher : public ::testing::Test
 {
 public:
@@ -274,6 +278,9 @@ TEST_F(TestPublisher, serialized_message_publish) {
   auto publisher = node->create_publisher<test_msgs::msg::Empty>("topic", 10, options);
 
   rclcpp::SerializedMessage serialized_msg;
+  // Mock successful rcl publish because the serialized_msg above is poorly formed
+  auto mock = mocking_utils::patch_and_return(
+    "self", rcl_publish_serialized_message, RCL_RET_OK);
   EXPECT_NO_THROW(publisher->publish(serialized_msg));
 
   EXPECT_NO_THROW(publisher->publish(serialized_msg.get_rcl_serialized_message()));
@@ -411,14 +418,22 @@ TEST_F(TestPublisher, inter_process_publish_failures) {
     EXPECT_THROW(publisher->publish(msg), rclcpp::exceptions::RCLError);
   }
 
-  rclcpp::SerializedMessage serialized_msg;
-  EXPECT_NO_THROW(publisher->publish(serialized_msg));
+  {
+    // Using 'self' instead of 'lib:rclcpp' because `rcl_publish_serialized_message` is entirely
+    // defined in a header. Also, this one requires mocking because the serialized_msg is poorly
+    // formed and this just tests rclcpp functionality.
+    auto mock = mocking_utils::patch_and_return(
+      "self", rcl_publish_serialized_message, RCL_RET_OK);
+    rclcpp::SerializedMessage serialized_msg;
+    EXPECT_NO_THROW(publisher->publish(serialized_msg));
+  }
 
   {
     // Using 'self' instead of 'lib:rclcpp' because `rcl_publish_serialized_message` is entirely
     // defined in a header
     auto mock = mocking_utils::patch_and_return(
       "self", rcl_publish_serialized_message, RCL_RET_ERROR);
+    rclcpp::SerializedMessage serialized_msg;
     EXPECT_THROW(publisher->publish(serialized_msg), rclcpp::exceptions::RCLError);
   }
 

diff --git a/rclcpp/test/rclcpp/test_subscription.cpp b/rclcpp/test/rclcpp/test_subscription.cpp
@@ -29,6 +29,10 @@
 
 #include "test_msgs/msg/empty.hpp"
 
+// Note: This is a long running test with rmw_connext_cpp, if you change this file, please check
+// that this test can complete fully, or adjust the timeout as necessary.
+// See https://github.com/ros2/rmw_connext/issues/325 for resolution
+
 using namespace std::chrono_literals;
 
 class TestSubscription : public ::testing::Test