Skip to content

Commit

Permalink
♻️ Refactor pickle tests
Browse files Browse the repository at this point in the history
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
  • Loading branch information
Jamim committed Feb 1, 2024
1 parent 770445a commit 3eb82a4
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 64 deletions.
6 changes: 0 additions & 6 deletions .mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,6 @@ disable_error_code =
type-arg,
var-annotated,

[mypy-gen_pickles]
disable_error_code =
attr-defined,
no-untyped-call,
no-untyped-def,

[mypy-test_abc]
disable_error_code =
no-untyped-call,
Expand Down
3 changes: 3 additions & 0 deletions CHANGES/938.contrib.rst
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
Pickle tests have been refactored in order
to make ``test_pickle.py`` more readable
and eliminate ``get_pickles.py``.
3 changes: 2 additions & 1 deletion docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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

Expand Down
36 changes: 33 additions & 3 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -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+
Expand All @@ -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)
Expand Down Expand Up @@ -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

Expand All @@ -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,
Expand Down
28 changes: 0 additions & 28 deletions tests/gen_pickles.py

This file was deleted.

5 changes: 3 additions & 2 deletions tests/test_multidict.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
from collections.abc import Mapping
from types import ModuleType
from typing import (
Any,
Callable,
Dict,
Iterable,
Expand Down Expand Up @@ -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:
Expand Down
60 changes: 36 additions & 24 deletions tests/test_pickle.py
Original file line number Diff line number Diff line change
@@ -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)

Check warning on line 50 in tests/test_pickle.py

View check run for this annotation

Codecov / codecov/patch

tests/test_pickle.py#L50

Added line #L50 was not covered by tests

0 comments on commit 3eb82a4

Please sign in to comment.