-
-
Notifications
You must be signed in to change notification settings - Fork 31.4k
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
gh-116789: Add more tests for inspect.getmembers
#116802
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.
All these tests are just copies of each other. They do not contain anything specific to the specific type, so why add them? A test for arbitrary user object should be enough. They only depend on the result of dir(), if it returns an empty list, all tests will automatically pass.
Sorry, I lost this PR in the notifications at some point. I addressed @serhiy-storchaka's feedback:
Thanks a lot for the feedback and notification! 👍 |
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.
LGTM.
@serhiy-storchaka: Do you want to review the updated new tests?
It was up for 2 weeks, so I am going to merge this, we can add more tests later. Thanks a lot for the reviews, they were very helpful. |
Thanks @sobolevn for the PR 🌮🎉.. I'm working now to backport this PR to: 3.12, 3.13. |
GH-123129 is a backport of this pull request to the 3.13 branch. |
GH-123130 is a backport of this pull request to the 3.12 branch. |
It basically checks that
getmembers
is equaldir(...) + getattr
for these types.I've covered all types that were mentioned in the docs.
inspect.getmembers
is not fully tested #116789