Add more test cases for graph API + fix bug (#404)

* Add more test cases for graph API * Guard against invalid allocator in rcl_get_service_names_and_types. * Increase timeout for Connext tests * Refactor tests * Isolate unknown node name tests * Add invalid node name tests * Add zero allocator tests * Fix a segfault when passing a zero allocator to rcl_names_and_types_init. * Add TODOs for buggy tests Signed-off-by: Jacob Perron <[email protected]>
ros2 · Apr 10, 2019 · d810f12 · d810f12
1 parent c3ea0df
commit d810f12
Show file tree

Hide file tree

Showing 3 changed files with 318 additions and 8 deletions.
diff --git a/rcl/src/rcl/graph.c b/rcl/src/rcl/graph.c
@@ -42,7 +42,7 @@ rcl_get_publisher_names_and_types_by_node(
   if (!rcl_node_is_valid(node)) {
     return RCL_RET_NODE_INVALID;
   }
-  RCL_CHECK_ARGUMENT_FOR_NULL(allocator, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(node_name, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(node_namespace, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(topic_names_and_types, RCL_RET_INVALID_ARGUMENT);
@@ -80,7 +80,7 @@ rcl_get_subscriber_names_and_types_by_node(
   if (!rcl_node_is_valid(node)) {
     return RCL_RET_NODE_INVALID;
   }
-  RCL_CHECK_ARGUMENT_FOR_NULL(allocator, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(node_name, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(node_namespace, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(topic_names_and_types, RCL_RET_INVALID_ARGUMENT);
@@ -117,7 +117,7 @@ rcl_get_service_names_and_types_by_node(
   if (!rcl_node_is_valid(node)) {
     return RCL_RET_NODE_INVALID;
   }
-  RCL_CHECK_ARGUMENT_FOR_NULL(allocator, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(node_name, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(node_namespace, RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(service_names_and_types, RCL_RET_INVALID_ARGUMENT);
@@ -152,7 +152,7 @@ rcl_get_topic_names_and_types(
   if (!rcl_node_is_valid(node)) {
     return RCL_RET_NODE_INVALID;  // error already set
   }
-  RCL_CHECK_ARGUMENT_FOR_NULL(allocator, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(topic_names_and_types, RCL_RET_INVALID_ARGUMENT);
   rmw_ret_t rmw_ret;
   rmw_ret = rmw_names_and_types_check_zero(topic_names_and_types);
@@ -178,6 +178,7 @@ rcl_get_service_names_and_types(
   if (!rcl_node_is_valid(node)) {
     return RCL_RET_NODE_INVALID;  // error already set
   }
+  RCL_CHECK_ALLOCATOR_WITH_MSG(allocator, "invalid allocator", return RCL_RET_INVALID_ARGUMENT);
   RCL_CHECK_ARGUMENT_FOR_NULL(service_names_and_types, RCL_RET_INVALID_ARGUMENT);
   rmw_ret_t rmw_ret;
   rmw_ret = rmw_names_and_types_check_zero(service_names_and_types);
@@ -200,6 +201,7 @@ rcl_names_and_types_init(
   rcl_allocator_t * allocator)
 {
   RCL_CHECK_ARGUMENT_FOR_NULL(names_and_types, RCL_RET_INVALID_ARGUMENT);
+  RCL_CHECK_ALLOCATOR(allocator, return RCL_RET_INVALID_ARGUMENT);
   rmw_ret_t rmw_ret = rmw_names_and_types_init(names_and_types, size, allocator);
   return rcl_convert_rmw_ret_to_rcl_ret(rmw_ret);
 }

diff --git a/rcl/test/CMakeLists.txt b/rcl/test/CMakeLists.txt
@@ -100,7 +100,7 @@ function(test_target_function)
   # TODO(mm318): why rmw_connext tests run much slower than rmw_fastrtps and rmw_opensplice tests
   elseif(rmw_implementation STREQUAL "rmw_connext_cpp")
     message(STATUS "Increasing test_graph${target_suffix} test timeout.")
-    set(AMENT_GTEST_ARGS TIMEOUT 90)
+    set(AMENT_GTEST_ARGS TIMEOUT 180)
   endif()
   rcl_add_custom_gtest(test_graph${target_suffix}
     SRCS rcl/test_graph.cpp