-
-
Notifications
You must be signed in to change notification settings - Fork 3k
Use pytest Node.from_parent if available #9263
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
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.
Thanks, I think I had seen these deprecation warnings a few times.
A thought: perhaps we could simplify things and just require pytest >= 5.4 in the test reps?
3eb8071
to
5ac89ea
Compare
I have no objection to that, if everyone's on board with requiring a recent pytest for static analysis tests. Would certainly keep this cleaner. Also I force-pushed with a minor fix to shift a variable annotation to a type comment, since it looks like 3.5 is still part of the CI suite. |
Okay, can you make it so? Hopefully pytest 5.4 still supports 3.5... |
Done. Pushed it up as a separate commit, but we can squash before merging to keep the diff cleaner (I unwound some other misc changes to cancel out when we squash). |
Thanks. Please don't squash anything, we do that on merging. Squashing earlier confuses the code review. |
Since we're already bumping test requirements from pytest 5.3.x to 5.4+, I figured I would try bumping up to the latest pytest 6.x.x (which is where I ran into the PytestDeprecationWarning becoming an error in the first place). Most notably, this release adds types to the pytest package, which allows us to remove some type ignores in the mypy test code. Waiting to see what CI looks like but if that's green and there are no objections, I think we can just bump this up for the additional type safety. |
Awesome! I am anxiously awaiting the test results. :) |
The pytest Node construction API has changed to prefer a
Node.from_parent
factory over the raw constructor [1]. This PR updates a few points to conform to the recommended API to avoid thePytestDeprecationWarning
that arises when tests are being collected with pytest 6+.[1] https://docs.pytest.org/en/stable/deprecations.html#node-construction-changed-to-node-from-parent