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

Add compatitibility with pytest 7 #592

Merged
merged 1 commit into from Feb 10, 2022
Merged

Conversation

sloretz
Copy link
Contributor

@sloretz sloretz commented Feb 9, 2022

fixes #590

This adds a new function _pytest_version_ge to make the hooks compatible with multiple versions of pytest. I tested that rclpy and launch_testing's tests passed on my system with pytest versions 4.6.9, 5.4, 6.0, and 7.0.

In short, pytest deprecated the uses of their LocalPath class in favor of pathlib.Path.

Tested using commands like:

pip3 install pytest==5.4 && colcon test --event-handlers console_direct+ --packages-select rclpy

Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Copy link
Contributor

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like a good solution to me. Approved with green CI on Focal Linux, Jammy Linux, and Windows.

Copy link
Contributor

@Blast545 Blast545 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@sloretz
Copy link
Contributor Author

sloretz commented Feb 9, 2022

CI (build: --packages-above-and-dependencies launch_testing test: --packages-above launch_testing)

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

@sloretz
Copy link
Contributor Author

sloretz commented Feb 10, 2022

@clalancette I see 16 warnings and 1 test failure in sros2 on Jammy, but none of them seem related to this PR. Do you know of a job with the current state of Jammy that I can compare this CI run to?

@clalancette
Copy link
Contributor

@clalancette I see 16 warnings and 1 test failure in sros2 on Jammy, but none of them seem related to this PR. Do you know of a job with the current state of Jammy that I can compare this CI run to?

Those are all expected as of right now. The 16 warnings are from use of older OpenSSL APIs in Fast-DDS (I need to open an issue for that). The failing mypy test should be fixed by ros2/ci#617 (I still need to do a bit more testing there).

So this looks good to merge to me!

@sloretz sloretz merged commit 13d0c28 into master Feb 10, 2022
@delete-merged-branch delete-merged-branch bot deleted the sloretz__launch__pytest7 branch February 10, 2022 17:00
@sloretz
Copy link
Contributor Author

sloretz commented Jul 14, 2022

@Mergifyio backport foxy galactic

mergify bot pushed a commit that referenced this pull request Jul 14, 2022
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
(cherry picked from commit 13d0c28)
mergify bot pushed a commit that referenced this pull request Jul 14, 2022
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
(cherry picked from commit 13d0c28)
@mergify
Copy link

mergify bot commented Jul 14, 2022

backport foxy galactic

✅ Backports have been created

1 similar comment
@mergify
Copy link

mergify bot commented Jul 14, 2022

backport foxy galactic

✅ Backports have been created

sloretz added a commit that referenced this pull request Jul 18, 2022
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
(cherry picked from commit 13d0c28)

Co-authored-by: Shane Loretz <sloretz@osrfoundation.org>
sloretz added a commit that referenced this pull request Jul 18, 2022
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
(cherry picked from commit 13d0c28)

Co-authored-by: Shane Loretz <sloretz@osrfoundation.org>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants