Skip to content

Commit

Permalink
Increase coverage of node_interfaces, including with mocking rcl erro…
Browse files Browse the repository at this point in the history 
…rs (#1322)

* Increase coverage of node_interfaces, including with mocking rcl errors

Signed-off-by: Stephen Brawner <brawner@gmail.com>

* PR Fixup

Signed-off-by: Stephen Brawner <brawner@gmail.com>
  • Loading branch information
@brawner
brawner committed Oct 7, 2020
1 parent db37a32 commit 44b0244
Show file tree
Hide file tree
Showing 8 changed files with 689 additions and 138 deletions.
14 changes: 7 additions & 7 deletions rclcpp/test/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,7 +173,7 @@ endif()
ament_add_gtest(test_node_interfaces__node_base
rclcpp/node_interfaces/test_node_base.cpp)
if(TARGET test_node_interfaces__node_base)
target_link_libraries(test_node_interfaces__node_base ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_base ${PROJECT_NAME} mimick)
endif()
ament_add_gtest(test_node_interfaces__node_clock
rclcpp/node_interfaces/test_node_clock.cpp)
Expand All @@ -186,35 +186,35 @@ if(TARGET test_node_interfaces__node_graph)
ament_target_dependencies(
test_node_interfaces__node_graph
"test_msgs")
target_link_libraries(test_node_interfaces__node_graph ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_graph ${PROJECT_NAME} mimick)
endif()
ament_add_gtest(test_node_interfaces__node_parameters
rclcpp/node_interfaces/test_node_parameters.cpp)
if(TARGET test_node_interfaces__node_parameters)
target_link_libraries(test_node_interfaces__node_parameters ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_parameters ${PROJECT_NAME} mimick)
endif()
ament_add_gtest(test_node_interfaces__node_services
rclcpp/node_interfaces/test_node_services.cpp)
if(TARGET test_node_interfaces__node_services)
target_link_libraries(test_node_interfaces__node_services ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_services ${PROJECT_NAME} mimick)
endif()
ament_add_gtest(test_node_interfaces__node_timers
rclcpp/node_interfaces/test_node_timers.cpp)
if(TARGET test_node_interfaces__node_timers)
target_link_libraries(test_node_interfaces__node_timers ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_timers ${PROJECT_NAME} mimick)
endif()
ament_add_gtest(test_node_interfaces__node_topics
rclcpp/node_interfaces/test_node_topics.cpp)
if(TARGET test_node_interfaces__node_topics)
ament_target_dependencies(
test_node_interfaces__node_topics
"test_msgs")
target_link_libraries(test_node_interfaces__node_topics ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_topics ${PROJECT_NAME} mimick)
endif()
ament_add_gtest(test_node_interfaces__node_waitables
rclcpp/node_interfaces/test_node_waitables.cpp)
if(TARGET test_node_interfaces__node_waitables)
target_link_libraries(test_node_interfaces__node_waitables ${PROJECT_NAME})
target_link_libraries(test_node_interfaces__node_waitables ${PROJECT_NAME} mimick)
endif()

# TODO(wjwwood): reenable these build failure tests when I can get Jenkins to ignore their output
Expand Down
133 changes: 133 additions & 0 deletions rclcpp/test/rclcpp/node_interfaces/test_node_base.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@
#include "rclcpp/node.hpp"
#include "rclcpp/node_interfaces/node_base.hpp"
#include "rclcpp/rclcpp.hpp"
#include "rmw/validate_namespace.h"
#include "rmw/validate_node_name.h"

#include "../../mocking_utils/patch.hpp"
#include "../../utils/rclcpp_gtest_macros.hpp"

class TestNodeBase : public ::testing::Test
{
Expand All @@ -36,6 +41,12 @@ class TestNodeBase : public ::testing::Test
}
};

// Required for mocking_utils below
MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, ==)
MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, !=)
MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, <)
MOCKING_UTILS_BOOL_OPERATOR_RETURNS_FALSE(rcl_guard_condition_options_t, >)

TEST_F(TestNodeBase, construct_from_node)
{
std::shared_ptr<rclcpp::Node> node = std::make_shared<rclcpp::Node>("node", "ns");
Expand All @@ -58,3 +69,125 @@ TEST_F(TestNodeBase, construct_from_node)
EXPECT_NE(nullptr, const_node_base->get_rcl_node_handle());
EXPECT_NE(nullptr, const_node_base->get_shared_rcl_node_handle());
}

TEST_F(TestNodeBase, construct_destruct_rcl_guard_condition_init_error) {
auto mock = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_guard_condition_init, RCL_RET_ERROR);
EXPECT_THROW(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
rclcpp::exceptions::RCLError);
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_error) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_ERROR);

// This function is called only if rcl_node_init fails, so both mocked functions are required
// This just logs an error, so behavior shouldn't change
auto mock_guard_condition_fini = mocking_utils::inject_on_return(
"lib:rclcpp", rcl_guard_condition_fini, RCL_RET_ERROR);

EXPECT_THROW(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
rclcpp::exceptions::RCLError);
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_rcl_invalid_node_name) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_NODE_INVALID_NAME);

// `rmw_validate_node_name` is only called if `rcl_node_init` returns INVALID_NAME
auto mock_validate_node_name = mocking_utils::patch_and_return(
"lib:rclcpp", rmw_validate_node_name, RMW_RET_ERROR);

EXPECT_THROW(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
rclcpp::exceptions::RCLError);
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_rcl_invalid_node_name_invalid_argument) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_NODE_INVALID_NAME);

// `rmw_validate_node_name` is only called if `rcl_node_init` returns INVALID_NAME
auto mock_validate_node_name = mocking_utils::patch_and_return(
"lib:rclcpp", rmw_validate_node_name, RMW_RET_INVALID_ARGUMENT);

EXPECT_THROW(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
rclcpp::exceptions::RCLInvalidArgument);
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_rcl_invalid_node_name_valid_rmw_node_name) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_NODE_INVALID_NAME);

// `rmw_validate_node_name` is only called if `rcl_node_init` returns INVALID_NAME
auto mock = mocking_utils::patch(
"lib:rclcpp", rmw_validate_node_name, [](const char *, int * validation_result, size_t *)
{
*validation_result = RMW_NODE_NAME_VALID;
return RMW_RET_OK;
});

RCLCPP_EXPECT_THROW_EQ(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
std::runtime_error("valid rmw node name but invalid rcl node name"));
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_rcl_invalid_namespace) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_NODE_INVALID_NAMESPACE);

// `rmw_validate_namespace` is only called if `rcl_node_init` returns INVALID_NAMESPACE
auto mock_validate_namespace = mocking_utils::patch_and_return(
"lib:rclcpp", rmw_validate_namespace, RMW_RET_ERROR);

EXPECT_THROW(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
rclcpp::exceptions::RCLError);
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_rcl_invalid_namespace_rmw_invalid_argument) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_NODE_INVALID_NAMESPACE);

// `rmw_validate_namespace` is only called if `rcl_node_init` returns INVALID_NAMESPACE
auto mock_validate_namespace = mocking_utils::patch_and_return(
"lib:rclcpp", rmw_validate_namespace, RMW_RET_INVALID_ARGUMENT);

EXPECT_THROW(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
rclcpp::exceptions::RCLInvalidArgument);
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_rcl_invalid_namespace_valid_rmw_namespace) {
auto mock_node_init = mocking_utils::patch_and_return(
"lib:rclcpp", rcl_node_init, RCL_RET_NODE_INVALID_NAMESPACE);

// `rmw_validate_namespace` is only called if `rcl_node_init` returns INVALID_NAMESPACE
auto mock = mocking_utils::patch(
"lib:rclcpp", rmw_validate_namespace, [](const char *, int * validation_result, size_t *)
{
*validation_result = RMW_NAMESPACE_VALID;
return RMW_RET_OK;
});

RCLCPP_EXPECT_THROW_EQ(
std::make_shared<rclcpp::Node>("node", "ns").reset(),
std::runtime_error("valid rmw node namespace but invalid rcl node namespace"));
}

TEST_F(TestNodeBase, construct_destruct_rcl_node_init_fini_error) {
auto mock_node_fini = mocking_utils::inject_on_return(
"lib:rclcpp", rcl_node_fini, RCL_RET_ERROR);

EXPECT_NO_THROW(std::make_shared<rclcpp::Node>("node", "ns").reset());
}

TEST_F(TestNodeBase, construct_destruct_rcl_guard_condition_fini_error) {
auto mock_node_fini = mocking_utils::inject_on_return(
"lib:rclcpp", rcl_guard_condition_fini, RCL_RET_ERROR);

EXPECT_NO_THROW(std::make_shared<rclcpp::Node>("node", "ns").reset());
}
Loading

0 comments on commit 44b0244

Please sign in to comment.