Closed ahans closed 1 year ago
@mvukov Just let me know what you think! Thanks
I think this would be super valuable. And using pytest is somewhat easier / more convenient than unittest. I found this gist very nice: https://gist.github.com/betaboon/c1dd785b5ba468b4df4e382eafff969a (used it in another project and works like a charm; I also ironed it out a bit). How about we adopt this gist? If OK, then please put those two files in e.g. third_party/pytest.
Regarding further integration in ros2_test
macro: I would just add another argument to it, e.g. use_pytest
-- if True we internally set up a test with pytest-like driver. Sounds good?
I think this would be super valuable. And using pytest is somewhat easier / more convenient than unittest. I found this gist very nice: https://gist.github.com/betaboon/c1dd785b5ba468b4df4e382eafff969a (used it in another project and works like a charm; I also ironed it out a bit). How about we adopt this gist? If OK, then please put those two files in e.g. third_party/pytest.
Are you sure we're good to use that from a licensing perspective? Mine would have been much shorter, essentially just the pytest.main()
call. It's nice that it has coverage support already, though!
Regarding further integration in
ros2_test
macro: I would just add another argument to it, e.g.use_pytest
-- if True we internally set up a test with pytest-like driver. Sounds good?
That's actually a pretty neat idea! Will do it like that!
AFAICS, there are no licensing issues with that gist. Do you see any?
Per default, pytests gives you very slim output, and usually devs want something more verbose. After quite a while using it in default mode and calling Bazel test targets often with a bunch of --test_arg args, I am inclined to think one should always call pytest with "-ra", "-vv",
like in the gist.
Also for easier test logs integration with CI, we should invoke pytest in the wrapper with f'--junitxml={os.environ["XML_OUTPUT_FILE"]}'
, where Bazel emits XML_OUTPUT_FILE. The thing in the gist isn't good enough IMO.
AFAICS, there are no licensing issues with that gist. Do you see any?
There isn't any license explicitly mentioned, so it defaults to "all rights reserved". But essentially it's quite trivial, so if the original author wants us to use something else, we can easily come up with something similar ourselves.
Per default, pytests gives you very slim output, and usually devs want something more verbose. After quite a while using it in default mode and calling Bazel test targets often with a bunch of --test_arg args, I am inclined to think one should always call pytest with
"-ra", "-vv",
like in the gist.
Yes, I very much agree. My pytest default was -rP
so far. Not sure what -ra -vv
does differently, but I'm certainly fine with that.
Also for easier test logs integration with CI, we should invoke pytest in the wrapper with
f'--junitxml={os.environ["XML_OUTPUT_FILE"]}'
, where Bazel emits XML_OUTPUT_FILE. The thing in the gist isn't good enough IMO.
Good to know, will adapt that!
I implemented the changes now and it seems to be working. However, I noticed the ament setup part you're doing. For the pytest version, we don't have that yet. To test real nodes that potentially use pluginlib, I think we need it. Easiest solution would probably be generating the wrapper from a template instead of having a static one. Wdyt?
With the exception of the domain ID part I implemented all the changes. That part will follow shortly. Would be great if you could already have a look at the other changes!
Very nice work, I left a small comment.
Now it should be ready!
Many thanks for your work! :rocket:
Here's an initial implementation with support for
launch_pytest
, which seems to me like a nicer alternative tolaunch_testing
. Let me know what you think, then I can finish it. How to integrate it with the rest of the Bazel setup is the part that's somewhat unclear. See my code comments.