♻️ Refactor pickle tests

The main goal of these changes is to make test_pickle.py
more readable and eliminate get_pickles.py.

These changes:
  - add dict_data, pickled_data, and pickle_file_path fixtures
  - rename test_pickle to test_unpickle
  - add test_pickle_format_stability
  - replace test_load_from_file with test_pickle_backward_compatibility
  - replace get_pickles.py with test_write_pickle_file

Author: Jamim

CHANGES/938.contrib.rst

@@ -0,0 +1,3 @@
+Pickle tests have been refactored in order
+to make ``test_pickle.py`` more readable
+and eliminate ``get_pickles.py``.

docs/conf.py

@@ -19,6 +19,7 @@
 import re
 from contextlib import suppress
 from pathlib import Path
+from typing import Optional
 
 import alabaster
 from sphinx.addnodes import pending_xref
@@ -384,7 +385,7 @@ def _replace_missing_aiohttp_hdrs_reference(
     env: BuildEnvironment,
     node: pending_xref,
     contnode: literal,
-) -> reference:
+) -> Optional[reference]:
     if (node.get('refdomain'), node.get('reftype')) != ("py", "mod"):
         return None
 

tests/conftest.py

@@ -4,9 +4,10 @@
 import pickle
 from dataclasses import dataclass
 from importlib import import_module
+from pathlib import Path
 from sys import version_info as _version_info
 from types import ModuleType
-from typing import Callable, Type
+from typing import Any, Callable, Type
 
 try:
     from functools import cached_property  # Python 3.8+
@@ -23,6 +24,7 @@ def cached_property(func):
 
 C_EXT_MARK = pytest.mark.c_extension
 PY_38_AND_BELOW = _version_info < (3, 9)
+TESTS_DIR = Path(__file__).parent.resolve()
 
 
 @dataclass(frozen=True)
@@ -139,15 +141,15 @@ def any_multidict_proxy_class(
 @pytest.fixture(scope="session")
 def case_sensitive_multidict_proxy_class(
     multidict_module: ModuleType,
-) -> Type[MutableMultiMapping[str]]:
+) -> Type[MultiMapping[str]]:
     """Return a case-sensitive immutable multidict class."""
     return multidict_module.MultiDictProxy
 
 
 @pytest.fixture(scope="session")
 def case_insensitive_multidict_proxy_class(
     multidict_module: ModuleType,
-) -> Type[MutableMultiMapping[str]]:
+) -> Type[MultiMapping[str]]:
     """Return a case-insensitive immutable multidict class."""
     return multidict_module.CIMultiDictProxy
 
@@ -158,6 +160,34 @@ def multidict_getversion_callable(multidict_module: ModuleType) -> Callable:
     return multidict_module.getversion
 
 
+@pytest.fixture
+def dict_data() -> Any:
+    return [("a", 1), ("a", 2)]
+
+
+@pytest.fixture
+def pickled_data(
+    any_multidict_class,
+    pickle_protocol: int,
+    dict_data: Any,
+) -> bytes:
+    """Generates a pickled representation of the test data"""
+    d = any_multidict_class(dict_data)
+    return pickle.dumps(d, pickle_protocol)
+
+
+@pytest.fixture
+def pickle_file_path(
+    any_multidict_class_name: str,
+    multidict_implementation: MultidictImplementation,
+    pickle_protocol: int,
+) -> Path:
+    return TESTS_DIR / (
+        f"{any_multidict_class_name.lower()}-{multidict_implementation.tag}"
+        f".pickle.{pickle_protocol}"
+    )
+
+
 def pytest_addoption(
     parser: pytest.Parser,
     pluginmanager: pytest.PytestPluginManager,

tests/gen_pickles.py

@@ -8,6 +8,7 @@
 from collections.abc import Mapping
 from types import ModuleType
 from typing import (
+    Any,
     Callable,
     Dict,
     Iterable,
@@ -574,8 +575,8 @@ def test_preserve_stable_ordering(
 
         assert s == "a=1&b=2&a=3"
 
-    def test_get(self, cls: Type[MultiDict[int]]) -> None:
-        d = cls([("a", 1), ("a", 2)])
+    def test_get(self, cls: Type[MultiDict[int]], dict_data: Any) -> None:
+        d = cls(dict_data)
         assert d["a"] == 1
 
     def test_items__repr__(self, cls: Type[MultiDict[str]]) -> None:

tests/test_multidict.py

@@ -1,38 +1,50 @@
+import os
 import pickle
 from pathlib import Path
 
 import pytest
 
-here = Path(__file__).resolve().parent
+WRITE_PICKLE_FILES = bool(os.environ.get("WRITE_PICKLE_FILES"))
 
 
-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)
+def test_unpickle(any_multidict_class, dict_data, pickled_data):
+    expected = any_multidict_class(dict_data)
+    actual = pickle.loads(pickled_data)
+    assert actual == expected
+    assert isinstance(actual, any_multidict_class)
 
 
-def test_pickle_proxy(any_multidict_class, any_multidict_proxy_class):
-    d = any_multidict_class([("a", 1), ("a", 2)])
+def test_pickle_proxy(any_multidict_class, any_multidict_proxy_class, dict_data):
+    d = any_multidict_class(dict_data)
     proxy = any_multidict_proxy_class(d)
     with pytest.raises(TypeError):
         pickle.dumps(proxy)
 
 
-def test_load_from_file(any_multidict_class, multidict_implementation, pickle_protocol):
-    multidict_class_name = any_multidict_class.__name__
-    pickle_file_basename = "-".join(
-        (
-            multidict_class_name.lower(),
-            multidict_implementation.tag,
-        )
-    )
-    d = any_multidict_class([("a", 1), ("a", 2)])
-    fname = f"{pickle_file_basename}.pickle.{pickle_protocol}"
-    p = here / fname
-    with p.open("rb") as f:
-        obj = pickle.load(f)
-    assert d == obj
-    assert isinstance(obj, any_multidict_class)
+def test_pickle_format_stability(pickled_data, pickle_file_path, pickle_protocol):
+    if pickle_protocol == 0:
+        # TODO: consider updating pickle files
+        pytest.skip(reason="Format for pickle protocol 0 is changed, it's a known fact")
+    expected = pickle_file_path.read_bytes()
+    assert pickled_data == expected
+
+
+def test_pickle_backward_compatibility(
+    any_multidict_class,
+    dict_data,
+    pickle_file_path,
+):
+    expected = any_multidict_class(dict_data)
+    with pickle_file_path.open("rb") as f:
+        actual = pickle.load(f)
+
+    assert actual == expected
+    assert isinstance(actual, any_multidict_class)
+
+
+@pytest.mark.skipif(
+    not WRITE_PICKLE_FILES,
+    reason="This is a helper that writes pickle test files",
+)
+def test_write_pickle_file(pickled_data: bytes, pickle_file_path: Path) -> None:
+    pickle_file_path.write_bytes(pickled_data)