use ament_cmake_pytest instead of ament_cmake_nose #188
Conversation
| @@ -186,7 +186,7 @@ if(BUILD_TESTING) | |||
| file(GENERATE | |||
| OUTPUT test_composition${target_suffix}_$<CONFIG>.py | |||
| INPUT test_composition${target_suffix}.py.genexp) | |||
| ament_add_nose_test(test_composition${target_suffix} | |||
| ament_add_pytest_test(test_composition${target_suffix} | |||
There was a problem hiding this comment.
I know it's not related to this change but it seems weird that this uses macros from package that were not find_package'd. same for the other PRs. (ament_lint_auto_find_test_dependencies is the one finding them but I think it's confusing that the ament_lint_XX macro finds non linter test dependencies for you)
There was a problem hiding this comment.
Well the function is called find_test_dependencies (with the prefix of the package it is contained in). So I think it is very reasonable to expect that it finds all test dependencies.
Since the package was relying on exactly the same behavior before with nose I would suggest to address it in a separate PR if you think it should be done explicitly.
There was a problem hiding this comment.
I remember us having the same conversation (#181 (comment)) when explicit call to ament_flake8 was added a few lines below. So I assumed you wanted to do the same here as it sees to be a very similar scenario
There was a problem hiding this comment.
Ok, I added the explicit calls to make it REQUIRED.
The same on all the other PRs with this comment.
dirk-thomas
force-pushed
the
ament_cmake_pytest
branch
from
a2ecfaf to
480cf16
Compare
Connect to ament/ament_cmake#115.