multidict
multidict copied to clipboard
[TODO] Replace `tests/gen_pickles.py` with a pytest plugin
The idea is simple:
- [ ] Make a session-scoped fixture in
tests/conftest.py - [ ] Move the logic from
gen_pickles.pythere and generate the pickles in atmppath-provided dir - [ ] Integrate that into the pickling tests
- [ ] Delete
tests/gen_pickles.pyand the pickle files in thetests/dir - [ ] Bonus points: keep pickles in-memory, so they don't hit the disk
- [ ] Bonus 2: check if a session fixture is needed and switch to the normal function scope, if not
I've raised a PR here (https://github.com/aio-libs/multidict/pull/924). I think this addresses the ideas mentioned here. let me know if there's a better way to do this or if I'm barking up the wrong tree completely! Thanks for all your help so far!
I'll have a number of comments in the review there.
Sounds good. Let me know!
Hello @vrdn-23 and @webknjaz,
I tried to fix #924 locally and after a few iterations of a simplification I realized that the test_load_from_file test doesn't increase coverage since there's already the test_pickle test that does exactly the same, but in a more elegant and easy to understand way. If I didn't miss something, it's implicitly parametrized enough to cover all the cases. The only difference is that the initial implementation of test_load_from_file checks whether data pickled with some previous version of the multidict library still can be unpickled and matched with the current version data, but if you are going to eliminate the corresponding files, it seems reasonable to drop the test_load_from_file completely and rely on the test_pickle instead.
def test_pickle(any_multidict_class, pickle_protocol):
d = any_multidict_class([("a", 1), ("a", 2)])
pbytes = pickle.dumps(d, pickle_protocol)
obj = pickle.loads(pbytes)
assert d == obj
assert isinstance(obj, any_multidict_class)
@webknjaz what do you think?
@vrdn-23, in case @webknjaz decide to keep this test, it would be better to use io.BytesIO in order to keep pickle.dumps/pickle.loads and not switch to pickle.dump/pickle.load, so the test_load_from_file would verify what its name suggests.
Feel free to submit an alternative PR.