gh-142624: Make test_inspect's _pickle requirement optional#142625
gh-142624: Make test_inspect's _pickle requirement optional#142625youknowone wants to merge 2 commits intopython:mainfrom
Conversation
| try: | ||
| import _pickle | ||
| MISSING_C_PICKLE = False | ||
| except ImportError: | ||
| MISSING_C_PICKLE = True |
There was a problem hiding this comment.
| try: | |
| import _pickle | |
| MISSING_C_PICKLE = False | |
| except ImportError: | |
| MISSING_C_PICKLE = True | |
| try: | |
| import _pickle | |
| except ImportError: | |
| _pickle = None |
This pattern is used more often in tests, as shown in the line below, it removes the need for a variable and makes the codebase more consistent.
There was a problem hiding this comment.
With this, use @unittest.skipUnless(_pickle, "requires _pickle module") as the test decorator.
Or to pick a different pattern in use in this module, use _pickle = import_helper.import_module('_pickle') in the test (see some examples that use _testcapi).
As another option, is there a particular reason to use _pickle? Can/should something from _testcapi be used instead?
serhiy-storchaka
left a comment
There was a problem hiding this comment.
You can simply use other builtin, e.g. list.append. But you need to find also a method with flag METH_METHOD.
|
I applied the review about |
|
|
||
| @unittest.skipIf(MISSING_C_DOCSTRINGS, | ||
| "Signature information for builtins requires docstrings") | ||
| @unittest.skipUnless(_pickle, "requires _pickle module") |
There was a problem hiding this comment.
Only skip the _pickle related tests.
serhiy-storchaka
left a comment
There was a problem hiding this comment.
In some tests we could use other builtin class, not Pickler.
Uh oh!
There was an error while loading. Please reload this page.