-
Notifications
You must be signed in to change notification settings - Fork 223
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 version check #669
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR Type: Refactoring
PR Summary: The pull request updates the version checking logic in the pytest_bdd compatibility module to use a more efficient tuple comparison rather than creating a new Version object.
Decision: Comment
📝 Type: 'Refactoring' - not supported yet.
- Sourcery currently only approves 'Typo fix' PRs.
✅ Issue addressed: this change correctly addresses the issue or implements the desired feature.
No details provided.
✅ Small diff: the diff is small enough to approve with confidence.
No details provided.
General suggestions:
- Confirm that the
release
attribute of theparse_version
object will consistently provide a tuple for comparison and will not result in anyAttributeError
orTypeError
. - Consider whether patch versions of pytest could affect the compatibility and if so, ensure that the version checking logic accounts for this.
- Review the broader impact of this change to ensure that it aligns with compatibility requirements across different pytest versions.
Thanks for using Sourcery. We offer it for free for open source projects and would be very grateful if you could help us grow. If you like it, would you consider sharing Sourcery on your favourite social media? ✨
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## master #669 +/- ##
==========================================
- Coverage 95.18% 95.17% -0.01%
==========================================
Files 50 50
Lines 1826 1825 -1
Branches 200 200
==========================================
- Hits 1738 1737 -1
Misses 57 57
Partials 31 31 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the radio silence - this seems to fix things from what I can see!
If you'd be willing to lift the minimum pytest version to 7 (released almost two years ago), you could also use pytest.version_tuple
instead. That'd mean not introducing the new dependency on packaging
.
Or given that you're only interested in major/minor version, you'd probably get away with tuple(int(c) for c in pytest.__version__.split(".")[:2])
on pytest 6.x (oh, the irony of having a version-dependent version check...).
yes I could update to pytest 7, but I don't have much time so I'll just merge this |
No description provided.