-
Notifications
You must be signed in to change notification settings - Fork 532
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
Port unit tests to ROS 2 + Fix CBOR conversion and PNG compression #882
Port unit tests to ROS 2 + Fix CBOR conversion and PNG compression #882
Conversation
@achim-k This is ready for review. I've run the unit tests numerous times to try see if anything is flaky, and seems to be in good shape. |
e30e52d
to
ca84599
Compare
rosbridge_library/src/rosbridge_library/capabilities/advertise_service.py
Outdated
Show resolved
Hide resolved
rosbridge_library/src/rosbridge_library/capabilities/service_response.py
Outdated
Show resolved
Hide resolved
43c733b
to
84f7461
Compare
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.
Amazing, ran tests in my ws (humble) and everything seems to pass. I can't speak much to the API changes so just throwing in some nit: suggestions where I saw them.
rosbridge_library/test/capabilities/test_service_capabilities.py
Outdated
Show resolved
Hide resolved
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.
LGTM. Having functional tests is good 😆
This PR seeks to bring all
rosbridge_library
unit tests up to date with ROS 2 so that CI is actually testing most of the functionality.There is one test that raises an exception when unadvertising a service, and I think it's this issue: ros2/rclpy#1098 -- so just left a TODO for now.
Closes #856