Conversation
Signed-off-by: William Woodall <william@osrfoundation.org>
a1e02ff to
013f1fe
Compare
|
This is ready for review. |
hidmic
left a comment
There was a problem hiding this comment.
Two small comments. Will the test generators output more than one element eventually? If not, why an std::vector?
|
|
||
| { | ||
| ActionClientTest<test_msgs::action::Fibonacci> test; | ||
| size_t order = 10; |
There was a problem hiding this comment.
Yeah, that was intentional, the idea being each scoped section would make their own order, but I guess I could use different names here.
| } | ||
| return true; | ||
| }; | ||
| result.push_back(test); |
That's the plan. I based this on the tests from services, which have a "add two int" test and a more complete one that tests all IDL types in a single service call. I didn't do that yet for actions because of the time and because it's somewhat redundant given that the tests for topics and services already test a complex message like that and actions derive from services and topics. |
sloretz
left a comment
There was a problem hiding this comment.
LGTM with green CI and a small cmake fix
test_communication/CMakeLists.txt
Outdated
| set_tests_properties( | ||
| test_action_client_server${test_suffix} | ||
| PROPERTIES DEPENDS | ||
| "test_requester_cpp__${rmw_implementation1};test_replier_cpp__${rmw_implementation2}" |
There was a problem hiding this comment.
"test_action_client_cpp__${rmw_implementation1};test_action_server_cpp__${rmw_implementation2}" I think
| // on feedback, check the feedback is valid | ||
| auto feedback_callback = | ||
| [&](auto, const auto & feedback) { | ||
| RCLCPP_INFO(logger, "received feedback"); |
There was a problem hiding this comment.
Will the test pass if no feedback is received? I guess it has to, no way to make sure feedback is received before the result.
There was a problem hiding this comment.
Yeah, I wasn't asserting that at all. I just wanted to make sure that if feedback is sent it is valid. I could keep a counter, but I'm not sure if all the feedback would be received before the result or not, and if not how I could wait for all the feedback even after the result.
I'd consider this a good improvement on the test if we could find a good way to do it.
| [logger](auto /* goal_handle */) { | ||
| RCLCPP_ERROR(logger, "received unexpected request to cancel goal"); | ||
| return rclcpp_action::CancelResponse::ACCEPT; | ||
| }; |
There was a problem hiding this comment.
should_fail = true if cancel request received?
|
|
CI with fix for test failure when using connext
|
|
CI post ros2/rosidl#338
|
|
Rebuilding just windows to check if warning is gone |
Requires ros2/rclcpp#593
Requires ros2/rclcpp#594