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

Jump to bottom

Improve test_pickling in test_ast #100882

Closed
sobolevn opened this issue Jan 9, 2023 · 1 comment
Closed

Improve test_pickling in test_ast #100882

sobolevn opened this issue Jan 9, 2023 · 1 comment
Assignees
@sobolevn
Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error

Comments

@sobolevn
Copy link
Member

sobolevn commented Jan 9, 2023 •
edited by bedevere-bot

Here's how it is defined right now:

cpython/Lib/test/test_ast.py

Lines 640 to 655 in 837ba05

def test_pickling(self):
import pickle
mods = [pickle]
try:
import cPickle
mods.append(cPickle)
except ImportError:
pass
protocols = [0, 1, 2]
for mod in mods:
for protocol in protocols:
for ast in (compile(i, "?", "exec", 0x400) for i in exec_tests):
ast2 = mod.loads(mod.dumps(ast, protocol))
self.assertEqual(to_tuple(ast2), to_tuple(ast))
def test_invalid_sum(self):

I see two major problems here:

  1. cPickle does not exist anymore (since 3.0), so it always fails with ImportError. If we want to be sure that both Python and C versions behave the same - we can surely do that with import_helper.import_fresh_module or we can drop this part and only keep pickle module direct usage (as other modules do). I prefer the second option, because of how other things are tested. But, in the end it is up to maintainers to decide :)
  2. Not all pickle protocols are tested: we need to use pickle.HIGHEST_PROTOCOL instead of just three items

PR is incoming!

Linked PRs
@sobolevn sobolevn added type-bug An unexpected behavior, bug, or error tests Tests in the Lib/test dir labels Jan 9, 2023
@sobolevn sobolevn self-assigned this Jan 9, 2023
sobolevn added a commit to sobolevn/cpython that referenced this issue Jan 9, 2023
@sobolevn
pythongh-100882: Improve test_pickling case in test_ast.py
93f5bac
@bedevere-bot bedevere-bot mentioned this issue Jan 9, 2023
Merged
pablogsal pushed a commit that referenced this issue Jan 9, 2023
@sobolevn
gh-100882: Improve test_pickling case in test_ast.py (#100883)
2e80c2a
@sobolevn
Copy link
Member Author

sobolevn commented Jan 9, 2023

Solved by #100883

@sobolevn sobolevn closed this as completed Jan 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@sobolevn sobolevn

Labels
tests Tests in the Lib/test dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
1 participant
@sobolevn