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

Fix pytest-cov version detection with pytest >=7.0.0 #436

Conversation

christophebedard
Copy link
Contributor

@christophebedard christophebedard commented Mar 24, 2023

I noticed that pytest-cov wasn't detected properly, even if python3-pytest-cov is installed. See for example test_tracetools here: https://build.ros2.org/job/Rpr__ros2_tracing__ubuntu_jammy_amd64/63/consoleFull#console-section-56

17:37:13 CMake Warning at /opt/ros/rolling/share/ament_cmake_pytest/cmake/ament_add_pytest_test.cmake:128 (message):
17:37:13   The Python module 'pytest-cov' was not found, test coverage will not be
17:37:13   produced.  On Linux, install the 'python3-pytest-cov' package.  On other
17:37:13   platforms, install 'pytest-cov' using pip.
17:37:13 Call Stack (most recent call first):
17:37:13   CMakeLists.txt:149 (ament_add_pytest_test)

I tested it locally and I got the same warning, but I know this used to work just fine.

Turns out that, while the plugin information printed by pytest used to be printed to stderr (which was a bit weird, I admit), it's now printed to stdout since pytest version 7.0.0: pytest-dev/pytest#8247. See the difference between pytest 7.2.2 (from pip) and pytest 6.2.5 (from system packages):

$ python3 -m pytest --version --version
This is pytest version 7.2.2, imported from /home/chris/.local/lib/python3.10/site-packages/pytest/__init__.py
setuptools registered plugins:
  pytest-cov-4.0.0 at /home/chris/.local/lib/python3.10/site-packages/pytest_cov/plugin.py
  pytest-repeat-0.9.1 at /usr/lib/python3/dist-packages/pytest_repeat.py
  pytest-timeout-2.1.0 at /usr/lib/python3/dist-packages/pytest_timeout.py
  pytest-mock-3.6.1 at /usr/lib/python3/dist-packages/pytest_mock/__init__.py
  colcon-core-0.12.1 at /usr/lib/python3/dist-packages/colcon_core/pytest/hooks.py
  pytest-rerunfailures-10.2 at /usr/lib/python3/dist-packages/pytest_rerunfailures.py
$ sudo python3 -m pytest --version --version
This is pytest version 6.2.5, imported from /usr/lib/python3/dist-packages/pytest/__init__.py
setuptools registered plugins:
  pytest-repeat-0.9.1 at /usr/lib/python3/dist-packages/pytest_repeat.py
  pytest-timeout-2.1.0 at /usr/lib/python3/dist-packages/pytest_timeout.py
  pytest-mock-3.6.1 at /usr/lib/python3/dist-packages/pytest_mock/__init__.py
  colcon-core-0.12.1 at /usr/lib/python3/dist-packages/colcon_core/pytest/hooks.py
  pytest-rerunfailures-10.2 at /usr/lib/python3/dist-packages/pytest_rerunfailures.py
  pytest-cov-3.0.0 at /usr/lib/python3/dist-packages/pytest_cov/plugin.py
$ python3 -m pytest --version --version > /dev/null
$ sudo python3 -m pytest --version --version > /dev/null
This is pytest version 6.2.5, imported from /usr/lib/python3/dist-packages/pytest/__init__.py
setuptools registered plugins:
  pytest-repeat-0.9.1 at /usr/lib/python3/dist-packages/pytest_repeat.py
  pytest-timeout-2.1.0 at /usr/lib/python3/dist-packages/pytest_timeout.py
  pytest-mock-3.6.1 at /usr/lib/python3/dist-packages/pytest_mock/__init__.py
  colcon-core-0.12.1 at /usr/lib/python3/dist-packages/colcon_core/pytest/hooks.py
  pytest-rerunfailures-10.2 at /usr/lib/python3/dist-packages/pytest_rerunfailures.py
  pytest-cov-3.0.0 at /usr/lib/python3/dist-packages/pytest_cov/plugin.py

Therefore, just search for the pytest-cov version in both stdout and stderr. This works (i.e., gets rid of the warning) locally.

Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
@clalancette
Copy link
Contributor

CI:

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Windows Build Status

@christophebedard
Copy link
Contributor Author

christophebedard commented Mar 24, 2023

CI:

Note that, while this checks for build failures, it will not validate this fix. ament_add_pytest_test() only looks for pytest-cov if:

  1. The AMENT_CMAKE_PYTEST_WITH_COVERAGE CMake option is enabled, and I assume it isn't enabled in normal CI runs; or
  2. The package that calls ament_add_pytest_test() declares a <test_depend>python3-pytest-cov</test_depend>.
    • test_tracetools is the only core package that does this, but it only runs its tracing-related Python tests (i.e., only calls ament_add_pytest_test()) if LTTng is installed, which it isn't, currently

However, it did work locally for me, so perhaps that's enough, functionality-wise.

Ideally, a test would've caught this regression (because it is one), but this isn't really straightforward to test.

@clalancette
Copy link
Contributor

  1. The AMENT_CMAKE_PYTEST_WITH_COVERAGE CMake option is enabled, and I assume it isn't enabled in normal CI runs; or

I'll note that you can specify arbitrary options to those CI jobs, so you could enable it as a colcon --cmake-args option. It might be worthwhile to run another job with that enabled.

@christophebedard
Copy link
Contributor Author

I'll note that you can specify arbitrary options to those CI jobs, so you could enable it as a colcon --cmake-args option. It might be worthwhile to run another job with that enabled.

Here:

  • Linux Build Status

Here's one without this branch to make sure that the issue is indeed present:

  • Linux Build Status

@christophebedard
Copy link
Contributor Author

christophebedard commented Mar 24, 2023

Looks good:

@clalancette clalancette merged commit 62208eb into ament:rolling Mar 26, 2023
@christophebedard christophebedard deleted the christophebedard/fix-pytest-cov-version-detection branch March 26, 2023 17:05
@haudren-woven
Copy link

@clalancette : I just hit this particular issue on Humble, is there any chance to backport this?

@christophebedard
Copy link
Contributor Author

This should be backportable to Humble.

Let's see if the bot will let me do it:

@christophebedard
Copy link
Contributor Author

@Mergifyio backport humble

@mergify
Copy link

mergify bot commented Jun 1, 2023

backport humble

❌ Command disallowed due to command restrictions in the Mergify configuration.

  • sender-permission>=write

@christophebedard
Copy link
Contributor Author

No :(

@haudren-woven you can always just cherry-pick this onto humble and open a PR.

@clalancette
Copy link
Contributor

@Mergifyio backport humble

@mergify
Copy link

mergify bot commented Jun 6, 2023

backport humble

✅ Backports have been created

mergify bot pushed a commit that referenced this pull request Jun 6, 2023
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
(cherry picked from commit 62208eb)
clalancette pushed a commit that referenced this pull request Jun 6, 2023
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
(cherry picked from commit 62208eb)

Co-authored-by: Christophe Bedard <christophe.bedard@apex.ai>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants