Enable coverage information generation for pytest tests with CMake

[christophebedard]

As described in this answers.ros.org question, I was looking to generate coverage information for Python tests in a CMake package. The --pytest-with-coverage colcon option only applies to (pure) Python packages.

I borrowed some code from https://github.com/colcon/colcon-core/blob/af7fa089962a9e9a35926734e17b00cfb4c780a6/colcon_core/task/python/test/pytest.py#L77-L108.

I'm open to suggestions to

• improve/replace/remove the need for AMENT_CMAKE_TEST_PYTEST_WITH_COVERAGE. I couldn't find a way to pass additional arguments/flags from the ctest invocation (done by colcon) through to the run_test.py script other than by using an environment variable.
• improve or remove the need for AMENT_CMAKE_CURRENT_SOURCE_DIR
• extract build_base in a more robust way

[dirk-thomas]

Thanks for contributing this enhancement.

I was wondering about the decision where certain parts of the logic are located in the patch. I few thoughts so far:

• The most natural choice for a CMake package to provide configuration options is to declare them with option(...). The default could be off or some heuristic as in colcon-core.
• The CMake logic in ament_add_pytest_test.cmake is creating the command line to invoke pytest. It seems to more natural location to also add the necessary --cov arguments instead of doing that in ament_cmake_test/ament_cmake_test/__init__.py which than has to do awkward checks if the test is actually not a gtest. This would also avoid needing to pass AMENT_CMAKE_CURRENT_SOURCE_DIR. Also the package ament_cmake_test should have no knowledge about pytest - that should be fully encapsulated in the ament_cmake_pytest package.

[christophebedard]

I was wondering about the decision where certain parts of the logic are located in the patch. I few thoughts so far:

Yeah, like I mentioned above, I wasn't really sure about some of the choices made for the PR, so I really appreciate this feedback!

• The most natural choice for a CMake package to provide configuration options is to declare them with option(...). The default could be off or some heuristic as in colcon-core.

Sounds good. I wanted to make this a colcon test option, but then, as you can see, it requires weird workarounds.

In any way, enabling coverage for CMake packages already requires build time flags (for C and C++).

• The CMake logic in ament_add_pytest_test.cmake is creating the command line to invoke pytest. It seems to more natural location to also add the necessary --cov arguments instead of doing that in ament_cmake_test/ament_cmake_test/__init__.py which than has to do awkward checks if the test is actually not a gtest. This would also avoid needing to pass AMENT_CMAKE_CURRENT_SOURCE_DIR. Also the package ament_cmake_test should have no knowledge about pytest - that should be fully encapsulated in the ament_cmake_pytest package.

Got it. Will address this soon.

[christophebedard]

Got it. Will address this soon.

@dirk-thomas see new commits (to be squashed eventually, or I can just open a new PR if you prefer)

Now with colcon/colcon-ros#101, ros.ament_cmake packages have a colcon build option (--ament-cmake-pytest-with-coverage) which defines AMENT_CMAKE_PYTEST_WITH_COVERAGE=1 to be checked by ament_cmake_pytest. I used an option(), but not every ros.ament_cmake package uses ament_cmake_pytest, therefore those that don't use it print warnings ("Manually-specified variables were not used by the project"). Not sure what the preferred solution is here, especially if I have to keep the option() in ament_cmake_pytest itself. Perhaps there's a hook somewhere?

[dirk-thomas]

• Linux
• Linux-aarch64
• macOS
• Windows

[dirk-thomas]

Thanks for the improvement.