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 test with multiple service calls #57

Merged
merged 1 commit into from
Oct 15, 2015

Conversation

dirk-thomas
Copy link
Member

This new test implement exactly the same test for services as #20 does for pub/sub. It covers what has been tracked in ros2/rclcpp#52 for a while.

Without any patches the test fails since most rmw implementations currently don't wake up a second time: http://ci.ros2.org/job/ros2_batch_ci_osx/349/

With the two patches (ros2/rmw_connext#66 ros2/rmw_opensplice#60) the test passes since the rmw implementations check the read correctly (as suggested in ros2/rclcpp#52 (comment)): http://ci.ros2.org/job/ros2_batch_ci_osx/350/

Connect to ros2/rclcpp#52

@dirk-thomas dirk-thomas added the in review Waiting for review (Kanban column) label Oct 15, 2015
@dirk-thomas dirk-thomas self-assigned this Oct 15, 2015
@jacquelinekay
Copy link
Contributor

Nice. Just for my own sanity, I'm going to build and the fix_service_samples branches locally and run the multiple services test to make sure it's not flaky.

@esteve
Copy link
Member

esteve commented Oct 15, 2015

+1

@dirk-thomas dirk-thomas mentioned this pull request Oct 15, 2015
@jacquelinekay
Copy link
Contributor

The test passed 50/50 times on my machine. I feel good about that. +1

@esteve
Copy link
Member

esteve commented Oct 15, 2015

I've merged the two PRs that fix the issue, this PR should be ready for merging.

dirk-thomas added a commit that referenced this pull request Oct 15, 2015
add test with multiple service calls
@dirk-thomas dirk-thomas merged commit b791232 into master Oct 15, 2015
@dirk-thomas dirk-thomas removed the in review Waiting for review (Kanban column) label Oct 15, 2015
@dirk-thomas dirk-thomas deleted the test_multiple_service_calls branch October 15, 2015 18:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants