Skip to content

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

Merged
clalancette merged 1 commit intoament:rollingfrom
christophebedard:christophebedard/fix-pytest-cov-version-detection
Mar 26, 2023
Merged

Fix pytest-cov version detection with pytest >=7.0.0#436
clalancette merged 1 commit intoament:rollingfrom
christophebedard:christophebedard/fix-pytest-cov-version-detection

Conversation

@christophebedard
Copy link
Copy Markdown
Member

@christophebedard christophebedard commented Mar 24, 2023 •
edited
Loading

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.

@christophebedard
Fix pytest-cov version detection with pytest >=7.0.0
bbcc6f6 
Signed-off-by: Christophe Bedard <christophe.bedard@apex.ai>
@christophebedard christophebedard mentioned this pull request Mar 24, 2023
Merged
@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Mar 24, 2023

CI:

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

@christophebedard
Copy link
Copy Markdown
Member Author

christophebedard commented Mar 24, 2023 •
edited
Loading

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
Copy Markdown
Contributor

clalancette commented Mar 24, 2023

  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
Copy Markdown
Member Author

christophebedard commented Mar 24, 2023

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
Copy Markdown
Member Author

christophebedard commented Mar 24, 2023 •
edited
Loading

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
Copy Markdown

haudren-woven commented May 30, 2023

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

@christophebedard
Copy link
Copy Markdown
Member Author

christophebedard commented Jun 1, 2023

This should be backportable to Humble.

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

@christophebedard
Copy link
Copy Markdown
Member Author

christophebedard commented Jun 1, 2023

@Mergifyio backport humble

@mergify
Copy link
Copy Markdown

mergify bot commented Jun 1, 2023

backport humble

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

Details
  • sender-permission>=write

@christophebedard
Copy link
Copy Markdown
Member Author

christophebedard commented Jun 1, 2023

No :(

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

@clalancette
Copy link
Copy Markdown
Contributor

clalancette commented Jun 6, 2023

@Mergifyio backport humble

@mergify
Copy link
Copy Markdown

mergify bot commented Jun 6, 2023 •
edited
Loading

backport humble

✅ Backports have been created

Details

@mergify mergify bot mentioned this pull request Jun 6, 2023
Merged
mergify bot pushed a commit that referenced this pull request Jun 6, 2023
@christophebedard @mergify
Fix pytest-cov version detection with pytest >=7.0.0 (#436)
f52bfc1 
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
@mergify @christophebedard
Fix pytest-cov version detection with pytest >=7.0.0 (#436) (#459)
c9e3e03 
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

Reviewers

@clalancette clalancette clalancette approved these changes

+1 more reviewer

@mjeronimo mjeronimo mjeronimo approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@christophebedard @clalancette @haudren-woven @mjeronimo