Fix pickling classes and functions defined in a faulty module #136
Conversation
Codecov Report
@@ Coverage Diff @@
## master #136 +/- ##
==========================================
+ Coverage 83.76% 83.85% +0.09%
==========================================
Files 2 2
Lines 536 539 +3
Branches 98 98
==========================================
+ Hits 449 452 +3
Misses 64 64
Partials 23 23
Continue to review full report at Codecov.
|
|
I probably need to add a new test for classes and functions that have |
|
I do not understand how codecov computes its diff hit percentage. The diff coverage is actually increasing as per https://codecov.io/gh/cloudpipe/cloudpickle/compare/ce7f1b5110ed1b16332d4cabe6c530ef6b8c06e4...c821d972c79f01bd66c0d7eb6bb110b438aa0b47/diff . |
| # pickle.whichimodule. | ||
| raise Exception() | ||
| def test_faulty_module(self): | ||
| for module_name in ['_faulty_module', '_missing_module', None]: |
There was a problem hiding this comment.
I cannot use @pytest.mark.parametrize because we derive from unittest.TestCase.
There was a problem hiding this comment.
Maybe new tests could be added in a pytest style? (i.e.: as separate functions instead of unittest.TestCase methods) This would result in a bit less boilerplate, less indentation and would allow the use of assert instead of assertEqual().
Eventually we could rewrite all tests with this style if you all like the idea. Should not be too hard. Willing to help with that. 😊
There was a problem hiding this comment.
We still need to use a base class for the various protocols (but not necessarily inheriting the unittest.TestCase class).
Otherwise one would have to parametrize all the test with the 2 protocols.
Anyway I think it's fine like this for now.
|
@rgbkrk let me merge this one as well. I would like to release 0.5.2 out soon. I will issue other PR for that. |
|
Thank you for fixing this. |
test_ignoring_whichmodule_exceptiontotest_faulty_module.save_functionwhich I fixed to make the new version of the test pass.