-
Notifications
You must be signed in to change notification settings - Fork 163
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
Changes from 9 commits
4306e2d
4c836b6
4c5c4e3
9000f99
c111d2a
6c2938c
2ac1b42
bb3cdca
4074093
ba06cb9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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 | ||
|
@@ -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(); | ||
|
||
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( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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(); | ||
} | ||
}); | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?There was a problem hiding this comment.
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.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agree, moved with ba06cb9