Does this change actually pass building and testing of downstream packages which was a concern raised before (ros2/ros2cli#540 (comment))?

I assume so because I've only created new interfaces in this PR; I haven't modified any existing interfaces.

I can check to make sure. What would be the best way to check in CI? On, say, just ci_linux for now, build everything and then run tests on all packages above test_msgs?

What would be the best way to check in CI? On, say, just ci_linux for now, build everything and then run tests on all packages above test_msgs?

https://colcon.readthedocs.io/en/released/user/how-to.html#test-selected-packages-as-well-as-their-dependents

These changes cause tests in downstream packages to fail:

I can close this PR without merging and make a new package in ros2cli with these interface definitions to avoid breaking tests downstream of test_msgs. Or is there a better way to proceed?

Could you use the existing message definitions? Potentially adding comments to them where you want to test for those?

I could add comments to existing message files. It depends on if we think the following are good trade-offs.

The tests will not be very concise. For example, to test multiple layers of nesting, test_msgs/msg/MultiNested seems to be the only existing option in test_msgs and its output is 667 lines with all comments included. This is the most extreme example, but test_msgs/srv/Arrays, which is probably the best choice to test services, is 141 lines.

Also, the tests won't be as thorough in testing services and actions. For services, test_msgs/srv/Arrays seems to be the best available choice, but it won't support me testing multiple nestings. For actions, the only interface definition that doesn't use builtin_interfaces is test_msgs/action/Fibonacci which won't allow me to test nesting or constants. Perhaps, we can assume that because these are done correctly on messages definitions that they will be done on actions.

I would think either of the two options (use existing interfaces with added comments or creating a separate package for concise interfaces) is preferred over adding interfaces to test_msgs and having to work around them in downstream packages.

I agree. I'll take one of the other approaches. I'm favoring making a separate package with concise interfaces as the tests can be more thorough.

I created a new package in ros2cli with the concise interface definitions and updated the tests here: ros2/ros2cli#547.

These changes cause tests in downstream packages to fail

I can close this PR without merging and make a new package in ros2cli with these interface definitions to avoid breaking tests downstream of test_msgs. Or is there a better way to proceed?

I would think either of the two options (use existing interfaces with added comments or creating a separate package for concise interfaces) is preferred over adding interfaces to test_msgs and having to work around them in downstream packages.

If I understand correctly it looks like it's just test_communication tests that are failing. It looks like test_communication is creating tests for all interface files in test_msgs, but the executable it's using to do those tests has a hard coded list of interface files it supports. A quick fix would be to hard code the interface files in test_communication's CMakeLists.txt to match the hard coding in the executable, but I would recommend adding support for the new interface files in the test executables in test_communication instead.

I like doing this instead of creating a separate package because we might not want to release a separate package. Depending on the repo it's added to (it looks like ros2/ros2cli#547 adds it to ros2/ros2cli) that means having to remember to update the package blacklist in the release repo when a new ROS distro is created.

Reopening to continue the discussion.

This set of PRs seems to pass CI without breaking downstream packages. I'll run the rest of CI once these PRs are reviewed.

One downside of adding new messages to test in test_communication is that is can drastically increase CI duration (at least it did historically) since it adds another message to test pub/sub with for each RMW.

Originally, I had thought we'd create a separate package with test messages for ros2interface specifically. Since it's primary purpose is to work with generated interfaces, it seems reasonable to me to have it's own test message package. A separate package would give us an isolated place to add/modify test messages without interfering with the more general test_msgs. I think there is a high likelihood of adding new messages beyond this PR, for testing future features and bug fixes in ros2interface.

Closed in favor of ros2/ros2cli#579.