[TODO] Replace `tests/gen_pickles.py` with a pytest plugin

[webknjaz]

The idea is simple:

• Make a session-scoped fixture in tests/conftest.py
• Move the logic from gen_pickles.py there and generate the pickles in a tmppath-provided dir
• Integrate that into the pickling tests
• Delete tests/gen_pickles.py and the pickle files in the tests/ 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

[vrdn-23]

I've raised a PR here (#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!

[webknjaz]

I'll have a number of comments in the review there.

[vrdn-23]

Sounds good. Let me know!

[Jamim]

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.

[webknjaz]

Feel free to submit an alternative PR.

[webknjaz]

I concluded that we should integrate one of pytest-harvest, pytest-datadir, python-pytest-cases or pytest-regressions instead. This needs some research.

[asvetlov]

Sorry, I object to removing pickle files stored on the disk.

These files were added intentionally to ensure that newer multidict versions still could read older pickles.
All this code was added during the bugfix for an accident with broken backward compatibility.

@webknjaz please feel free to reword the issue to keep pickle files and the generator. Honestly, I prefer keeping everything as is but moving tests/gen_pickles.py file to tools folder.