-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
Test pickling a simple callable (does not work). #3906
Conversation
serialized = pickle.dumps(m.simple_callable) | ||
deserialized = pickle.loads(serialized) | ||
assert deserialized() == 20220426 | ||
else: |
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.
Do we really want to test the broken behavior of CPython or just wrap this test with a non-strict xfail? At the very least, we should add a comment that this is a CPython bug not a PyPy bug and that PyPY is doing the preferred behavior for once.
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.
I added a comment to be explicit.
The test as-is gives unambiguous information about the where & what, in 3 lines.
strict xfail would lose where & what exactly the failure is.
non-strict xfail would lose almost all information about the where & what.
unless it's tracked in a comment, which is about as many characters, only not executable code, therefore leaving room for doubts and misunderstandings.
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.
@rwgk You can add a raises arg to xfail, but I see what you mean.
Currently only documents that it does not work. Starting point for future fix.
Thanks Aaron! Merging this now, to get this into the smart_holder update. |
Description
Currently only documents that it does not work for C Python (it only works for PyPy). Starting point for future fix.
Suggested changelog entry: