-
Notifications
You must be signed in to change notification settings - Fork 47
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_rmw_implementation package. #106
Conversation
Includes dummy init/shutdown test. Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Interestingly enough, this test as-is fails for [rmw_cyclonedds_cpp]: rmw_create_node: domain id out of range It appears we didn't notice before because Shall we change |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
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, with @brawner's point addressed.
Shall we change RMW_DEFAULT_DOMAIN_ID to reflect that? Or should it be honored?
I think having a way to let the middleware pick the default is useful, so maybe we should change it so it is honored.
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
just my 2cts, but why do we need a dedicated package for this? Can't this be part of system tests? |
This package is not meant to include system tests, but (ideally) unit tests for every portion of the documented While much of |
About this. Seems like neither |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
I was missing one last bit! See 7fde494. |
@hidmic please follow up with releases as necessary to make this job pass again: http://build.ros2.org/job/Rdev__rmw_implementation__ubuntu_focal_amd64/2/testReport/ |
Looks like this PR caused two CI jobs to fail last night. I think the fix is just to add a |
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Signed-off-by: Michel Hidalgo <michel@ekumenlabs.com>
Precisely what the title says. To serve as a basis for upcoming generic RMW API tests. Includes dummy init/shutdown test.