Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add coverage tests wait module #769

Merged
merged 10 commits into from
Sep 1, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion rcl/test/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,7 @@ function(test_target_function)
SRCS rcl/test_wait.cpp
ENV ${rmw_implementation_env_var}
APPEND_LIBRARY_DIRS ${extra_lib_dirs}
LIBRARIES ${PROJECT_NAME}
LIBRARIES ${PROJECT_NAME} mimick
AMENT_DEPENDENCIES ${rmw_implementation} "osrf_testing_tools_cpp"
)

Expand Down
93 changes: 93 additions & 0 deletions rcl/test/rcl/test_wait.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
#include "rcutils/logging_macros.h"

#include "./allocator_testing_utils.h"
#include "../mocking_utils/patch.hpp"

#ifdef RMW_IMPLEMENTATION
# define CLASSNAME_(NAME, SUFFIX) NAME ## __ ## SUFFIX
Expand Down Expand Up @@ -684,3 +685,95 @@ TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_get_allocator
ret = rcl_wait_set_fini(&wait_set);
EXPECT_EQ(RCL_RET_OK, ret) << rcl_get_error_string().str;
}

// Test wait set init failure cases using mocks
TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_failed_init) {
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
// nullptr failures
rcl_ret_t ret =
rcl_wait_set_init(nullptr, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
EXPECT_TRUE(rcl_error_is_set());
rcl_reset_error();

ret =
rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, nullptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
EXPECT_TRUE(rcl_error_is_set());
rcl_reset_error();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Blast545 shoud this two go to the test case immediately above? Also, should we test using an invalid (zero initialized) allocator?

Copy link
Contributor Author

@Blast545 Blast545 Aug 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You mean making them part of wait_set_get_allocator test?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, no, the one above that one, wait_set_valid_arguments. I misread the diff.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Agree, moved with ba06cb9


rcl_allocator_t zero_init_allocator =
static_cast<rcl_allocator_t>(rcutils_get_zero_initialized_allocator());
ret =
rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, zero_init_allocator);
EXPECT_EQ(RCL_RET_INVALID_ARGUMENT, ret);
EXPECT_TRUE(rcl_error_is_set());
rcl_reset_error();

{
// Mock rmw implementation to fail init
auto mock = mocking_utils::patch_and_return(
"lib:rcl", rmw_create_wait_set, nullptr);
ret =
rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator());
EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, ret);
EXPECT_TRUE(rcl_error_is_set());
rcl_reset_error();
}
}

// Test wait set fini failure cases using mocks
TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_failed_fini) {
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret =
rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 0, context_ptr, rcl_get_default_allocator());
ASSERT_EQ(RCL_RET_OK, ret);
{
// Mock rmw implementation to fail fini
auto mock = mocking_utils::inject_on_return(
"lib:rcl", rmw_destroy_wait_set, RMW_RET_ERROR);
EXPECT_EQ(RCL_RET_WAIT_SET_INVALID, rcl_wait_set_fini(&wait_set));
EXPECT_TRUE(rcl_error_is_set());
rcl_reset_error();
}
}

// Test proper error handling with a fault injection test
TEST_F(CLASSNAME(WaitSetTestFixture, RMW_IMPLEMENTATION), wait_set_test_maybe_init_fail) {
rcl_wait_set_t wait_set = rcl_get_zero_initialized_wait_set();
rcl_ret_t ret = RCL_RET_OK;
rcl_allocator_t alloc = rcl_get_default_allocator();

RCUTILS_FAULT_INJECTION_TEST(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think these two fault injection blocks could use some comments describing what each one tests. Besides the initial capacities, they look the same to me.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I used the two main coverage paths, I added an explanation with 4074093, let me know if you think it's clear

{
// Test init in the case where guard_conditions_size + timers_size = 0
// (used for guard condition size)
ret = rcl_wait_set_init(&wait_set, 1, 0, 0, 1, 1, 0, context_ptr, alloc);
if (RCL_RET_OK == ret) {
ret = rcl_wait_set_fini(&wait_set);
if (ret != RCL_RET_OK) {
rcl_reset_error();
}
} else {
EXPECT_TRUE(RCL_RET_WAIT_SET_INVALID == ret || RCL_RET_BAD_ALLOC == ret);
rcl_reset_error();
}
});

RCUTILS_FAULT_INJECTION_TEST(
{
// Test init wait_set using at least one of each of the possible elements that receives
// (subs, guard conditions, timers, clients, services, events)
ret = rcl_wait_set_init(&wait_set, 1, 1, 1, 1, 1, 1, context_ptr, alloc);
if (RCL_RET_OK == ret) {
ret = rcl_wait_set_fini(&wait_set);
if (ret != RCL_RET_OK) {
rcl_reset_error();
}
EXPECT_EQ(RCL_RET_OK, rcl_wait_set_fini(&wait_set));
Blast545 marked this conversation as resolved.
Show resolved Hide resolved
} else {
EXPECT_TRUE(RCL_RET_WAIT_SET_INVALID == ret || RCL_RET_BAD_ALLOC == ret);
rcl_reset_error();
}
});
}