search

ros2 / ros2cli

ROS 2 command line interface tools
Apache License 2.0
181 stars 162 forks source link

ros2cli tests don't verify output stream #484

Open rotu opened 4 years ago

rotu commented 4 years ago

ros2cli tests don't verify which stream output comes out on (stdout vs stderr).

These tests should confirm that the expected output does indeed come out via the expected stream, instead of munging together stdout and stderr.

See related #482, which may be wholly or partially attributed to this issue.

dirk-thomas commented 4 years ago

I would think it is very unlikely that the combined output is actually equal to the expected output but it was printed via the "wrong" stream. At least I have never seen a false positive like this.

Do you have any specific use case where this is actually problematic?

Assuming there is none I will mark this as an enhancement for the backlog. Please consider to contribute a pull request for this.

rotu commented 4 years ago

There is a great example here https://github.com/ros2/ros2cli/blob/bf4941afd06064d20914c19687a8ca98186687e6/ros2lifecycle/ros2lifecycle/verb/set.py#L86-L89

This utility logs a successful transition to stdout but an unsuccessful one to stderr. So if you're piping the output of the utility, you'll likely get an incomplete view of what's happening, and you may get log messages confusingly out of order.

dirk-thomas commented 4 years ago

I would suggest to contribute a pull request to update that specific test then.

rotu commented 4 years ago

And the tests as well to verify tool sanity! I'm sure there are other examples as well across this repo, and it would be a great use case for TDD.

That said, my bosses have requested I go on fewer development tangents, so I'm not likely to write the PR myself.