Skip to content

Commit

Permalink
Change some EXPECT_EQ to ASSERT_EQ in test_action_server. (#759)
Browse files Browse the repository at this point in the history 
There are several places where we run a function, check the
result code, and then look at some of the output variables.
In these tests, we should always use ASSERT_EQ for checking
the return code; if that fails, we shouldn't go on and do
the rest of the tests, as we will be testing uninitialized
data at that point.  Go through test_action_server and make
these changes as appropriate.

Signed-off-by: Chris Lalancette <clalancette@openrobotics.org>
  • Loading branch information
@clalancette
clalancette authored Aug 21, 2020
1 parent a674160 commit 0980f88
Showing 1 changed file with 13 additions and 13 deletions.
26 changes: 13 additions & 13 deletions rcl_action/test/rcl_action/test_action_server.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -597,7 +597,7 @@ TEST_F(TestActionServer, test_action_clear_expired_goals)

// Clear with valid arguments
ret = rcl_action_expire_goals(&this->action_server, expired_goals, capacity, &num_expired);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_EQ(num_expired, 1u);
EXPECT_TRUE(uuidcmp(expired_goals[0].goal_id.uuid, goal_info_in.goal_id.uuid));

Expand Down Expand Up @@ -643,7 +643,7 @@ TEST_F(TestActionServer, test_action_process_cancel_request)
// Process cancel request with valid arguments (but no goals to cancel)
ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_EQ(cancel_response.msg.goals_canceling.data, nullptr);
EXPECT_EQ(cancel_response.msg.goals_canceling.size, 0u);
// A zero request means "cancel all goals", which succeeds if there's nothing to cancel
Expand Down Expand Up @@ -707,11 +707,11 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)

// Get with valid arguments (but not goals being tracked)
ret = rcl_action_get_goal_status_array(&this->action_server, &status_array);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_EQ(status_array.msg.status_list.data, nullptr);
EXPECT_EQ(status_array.msg.status_list.size, 0u);
ret = rcl_action_goal_status_array_fini(&status_array);
ASSERT_EQ(ret, RCL_RET_OK);
EXPECT_EQ(ret, RCL_RET_OK);

std::vector<rcl_action_goal_handle_t> handles;

Expand All @@ -723,13 +723,13 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)
ASSERT_NE(goal_handle, nullptr) << rcl_get_error_string().str;
handles.push_back(*goal_handle);
ret = rcl_action_get_goal_status_array(&this->action_server, &status_array);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(status_array.msg.status_list.data, nullptr);
EXPECT_EQ(status_array.msg.status_list.size, 1u);
rcl_action_goal_info_t * goal_info_out = &status_array.msg.status_list.data[0].goal_info;
EXPECT_TRUE(uuidcmp(goal_info_out->goal_id.uuid, goal_info_in.goal_id.uuid));
ret = rcl_action_goal_status_array_fini(&status_array);
ASSERT_EQ(ret, RCL_RET_OK);
EXPECT_EQ(ret, RCL_RET_OK);

// Add nine more goals
for (int i = 1; i < 10; ++i) {
Expand All @@ -741,7 +741,7 @@ TEST_F(TestActionServer, test_action_server_get_goal_status_array)
handles.push_back(*goal_handle);
}
ret = rcl_action_get_goal_status_array(&this->action_server, &status_array);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(status_array.msg.status_list.data, nullptr);
ASSERT_EQ(status_array.msg.status_list.size, 10u);
for (int i = 0; i < 10; ++i) {
Expand Down Expand Up @@ -841,7 +841,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_all_goal
rcl_action_cancel_response_t cancel_response = rcl_action_get_zero_initialized_cancel_response();
rcl_ret_t ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(cancel_response.msg.goals_canceling.data, nullptr);
ASSERT_EQ(cancel_response.msg.goals_canceling.size, (size_t)NUM_GOALS);
EXPECT_EQ(cancel_response.msg.return_code, action_msgs__srv__CancelGoal_Response__ERROR_NONE);
Expand All @@ -865,7 +865,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_g
rcl_action_get_zero_initialized_cancel_response();
rcl_ret_t ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(cancel_response.msg.goals_canceling.data, nullptr);
ASSERT_EQ(cancel_response.msg.goals_canceling.size, 1u);
EXPECT_EQ(cancel_response.msg.return_code, action_msgs__srv__CancelGoal_Response__ERROR_NONE);
Expand All @@ -881,7 +881,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_g
rcl_action_get_zero_initialized_cancel_response();
rcl_ret_t ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_EQ(cancel_response.msg.goals_canceling.data, nullptr);
EXPECT_EQ(cancel_response.msg.goals_canceling.size, 0u);
EXPECT_EQ(
Expand All @@ -903,7 +903,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_single_g
rcl_action_get_zero_initialized_cancel_response();
ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_EQ(cancel_response.msg.goals_canceling.data, nullptr);
EXPECT_EQ(cancel_response.msg.goals_canceling.size, 0u);
EXPECT_EQ(
Expand All @@ -922,7 +922,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time)
rcl_action_cancel_response_t cancel_response = rcl_action_get_zero_initialized_cancel_response();
rcl_ret_t ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(cancel_response.msg.goals_canceling.data, nullptr);
ASSERT_EQ(cancel_response.msg.goals_canceling.size, time_index + 1); // goals at indices [0, 7]
EXPECT_EQ(cancel_response.msg.return_code, action_msgs__srv__CancelGoal_Response__ERROR_NONE);
Expand All @@ -949,7 +949,7 @@ TEST_F(TestActionServerCancelPolicy, test_action_process_cancel_request_by_time_
rcl_action_cancel_response_t cancel_response = rcl_action_get_zero_initialized_cancel_response();
rcl_ret_t ret = rcl_action_process_cancel_request(
&this->action_server, &cancel_request, &cancel_response);
EXPECT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
ASSERT_EQ(ret, RCL_RET_OK) << rcl_get_error_string().str;
EXPECT_NE(cancel_response.msg.goals_canceling.data, nullptr);
EXPECT_EQ(cancel_response.msg.return_code, action_msgs__srv__CancelGoal_Response__ERROR_NONE);
const size_t num_goals_canceling = cancel_response.msg.goals_canceling.size;
Expand Down

0 comments on commit 0980f88

Please sign in to comment.