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

ros2cli tests don't verify output stream #484

Open
rotu opened this issue Apr 13, 2020 · 4 comments
Open

ros2cli tests don't verify output stream #484

rotu opened this issue Apr 13, 2020 · 4 comments
Labels
backlog enhancement New feature or request help wanted Extra attention is needed

Comments

@rotu
Copy link

rotu commented Apr 13, 2020

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
Copy link
Member

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.

@dirk-thomas dirk-thomas added backlog enhancement New feature or request help wanted Extra attention is needed labels Apr 13, 2020
@rotu
Copy link
Author

rotu commented Apr 13, 2020

There is a great example here

elif result:
print('Transitioning successful')
else:
print('Transitioning failed', file=sys.stderr)

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
Copy link
Member

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

@rotu
Copy link
Author

rotu commented Apr 14, 2020

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backlog enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

2 participants