🧪 Rewire pytest fixtures avoiding import loops

This patch also refactors and reduces the duplication of the previously
existing fixtures for retrieving different multidict module
implementations and makes the c-extension testing controllable by a CLI
option on the pytest level.

Fixes aio-libs#837

.github/workflows/ci-cd.yml

@@ -261,12 +261,11 @@ jobs:
     - name: Self-install
       run: python -Im pip install '${{ steps.wheel-file.outputs.path }}'
     - name: Run unittests
-      env:
-        MULTIDICT_NO_EXTENSIONS: ${{ matrix.no-extensions }}
       run: >-
-        python -m pytest tests -vv
+        python -Im pytest tests -vv
         --cov-report xml
         --junitxml=.test-results/pytest/test.xml
+        --${{ matrix.no-extensions == 'Y' && 'no-' || '' }}c-extensions
     - name: Produce markdown test summary from JUnit
       if: >-
         !cancelled()
@@ -284,11 +283,10 @@ jobs:
       if: >-
         !cancelled()
         && failure()
-      env:
-        MULTIDICT_NO_EXTENSIONS: ${{ matrix.no-extensions }}
       run: >-  # `exit 1` makes sure that the job remains red with flaky runs
         python -Im
         pytest --no-cov -vvvvv --lf -rA
+        --${{ matrix.no-extensions == 'Y' && 'no-' || '' }}c-extensions
         && exit 1
       shell: bash
     - name: Prepare coverage artifact

.mypy.ini

@@ -25,6 +25,5 @@ warn_unreachable = True
 warn_unused_ignores = True
 warn_return_any = True
 
-[mypy-tests.*]
-disallow_any_decorated = False
-disallow_untyped_calls = False
+[mypy-test_multidict]
+disable_error_code = call-arg

.pre-commit-config.yaml

@@ -31,6 +31,21 @@ repos:
 
 - repo: local
   hooks:
+  - id: top-level-tests-init-py
+    name: changelog filenames
+    language: fail
+    entry: >-
+      The `tests/__init__.py` module must not exist so `pytest` doesn't add the
+      project root to `sys.path` / `$PYTHONPATH`
+    files: >-
+      (?x)
+      ^
+        tests/__init__\.py
+      $
+    types: []
+    types_or:
+    - file
+    - symlink
   - id: changelogs-rst
     name: changelog filenames
     language: fail

tests/conftest.py

@@ -1,25 +1,159 @@
+from __future__ import annotations
+
+import argparse
 import pickle
+from dataclasses import dataclass
+from importlib import import_module
+from sys import version_info as _version_info
+from types import ModuleType
+from typing import Callable, Type
+
+try:
+    from functools import cached_property  # Python 3.8+
+except ImportError:
+    from functools import lru_cache as _lru_cache
+
+    def cached_property(func):
+        return property(_lru_cache()(func))
+
 
 import pytest
 
-from multidict._compat import USE_EXTENSIONS
+from multidict import MultiMapping, MutableMultiMapping
+
+PY_38_AND_BELOW = _version_info < (3, 9)
+
+
+@dataclass(frozen=True)
+class MultidictImplementation:
+    is_pure_python: bool
+
+    @cached_property
+    def tag(self) -> str:
+        return "pure-python" if self.is_pure_python else "c-extension"
+
+    @cached_property
+    def imported_module(self) -> ModuleType:
+        importable_module = "_multidict_py" if self.is_pure_python else "_multidict"
+        return import_module(f"multidict.{importable_module}")
+
+    def __str__(self):
+        return f"{self.tag}-module"
 
-OPTIONAL_CYTHON = (
-    ()
-    if USE_EXTENSIONS
-    else pytest.mark.skip(reason="No extensions available")
+
+@pytest.fixture(
+    scope="session",
+    params=(
+        MultidictImplementation(is_pure_python=False),
+        MultidictImplementation(is_pure_python=True),
+    ),
+    ids=str,
 )
+def multidict_implementation(request) -> MultidictImplementation:
+    multidict_implementation = request.param
+    test_c_extensions = request.config.getoption("--c-extensions") is True
+
+    if not test_c_extensions and not multidict_implementation.is_pure_python:
+        pytest.skip("C-extension testing not requested")
+
+    return multidict_implementation
+
 
+@pytest.fixture(scope="session")
+def multidict_module(
+    multidict_implementation: MultidictImplementation,
+) -> ModuleType:
+    return multidict_implementation.imported_module
 
-@pytest.fixture(  # type: ignore[call-overload]
+
+@pytest.fixture(
     scope="session",
-    params=[
-        pytest.param("multidict._multidict", marks=OPTIONAL_CYTHON),  # type: ignore
-        "multidict._multidict_py",
-    ],
+    params=("MultiDict", "CIMultiDict"),
+    ids=("case-sensitive", "case-insensitive"),
 )
-def _multidict(request):
-    return pytest.importorskip(request.param)
+def any_multidict_class_name(request: pytest.FixtureRequest) -> str:
+    return request.param
+
+
+@pytest.fixture(scope="session")
+def any_multidict_class(
+    any_multidict_class_name: str,
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    return getattr(multidict_module, any_multidict_class_name)
+
+
+@pytest.fixture(scope="session")
+def case_sensitive_multidict_class(
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    return multidict_module.MultiDict
+
+
+@pytest.fixture(scope="session")
+def case_insensitive_multidict_class(
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    return multidict_module.CIMultiDict
+
+
+@pytest.fixture(scope="session")
+def case_insensitive_str_class(multidict_module: ModuleType) -> Type[str]:
+    return multidict_module.istr
+
+
+@pytest.fixture(scope="session")
+def any_multidict_proxy_class_name(any_multidict_class_name: str) -> str:
+    return f"{any_multidict_class_name}Proxy"
+
+
+@pytest.fixture(scope="session")
+def any_multidict_proxy_class(
+    any_multidict_proxy_class_name: str,
+    multidict_module: ModuleType,
+) -> Type[MultiMapping[str]]:
+    return getattr(multidict_module, any_multidict_proxy_class_name)
+
+
+@pytest.fixture(scope="session")
+def case_sensitive_multidict_proxy_class(
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    return multidict_module.MultiDictProxy
+
+
+@pytest.fixture(scope="session")
+def case_insensitive_multidict_proxy_class(
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    return multidict_module.CIMultiDictProxy
+
+
+@pytest.fixture(scope="session")
+def multidict_getversion_callable(multidict_module: ModuleType) -> Callable:
+    return multidict_module.getversion
+
+
+def pytest_addoption(
+    parser: pytest.Parser,
+    pluginmanager: pytest.PytestPluginManager,
+) -> None:
+    del pluginmanager
+    parser.addoption(
+        "--c-extensions",  # disabled with `--no-c-extensions`
+        action="store_true" if PY_38_AND_BELOW else argparse.BooleanOptionalAction,
+        default=True,
+        dest="c_extensions",
+        help="Test C-extensions (on by default)",
+    )
+
+    if PY_38_AND_BELOW:
+        parser.addoption(
+            "--no-c-extensions",
+            action="store_false",
+            dest="c_extensions",
+            help="Skip testing C-extensions (on by default)",
+        )
 
 
 def pytest_generate_tests(metafunc):

tests/gen_pickles.py

@@ -1,31 +1,24 @@
 import pickle
 
-from multidict._compat import USE_EXTENSIONS
-from multidict._multidict_py import CIMultiDict as PyCIMultiDict  # noqa
-from multidict._multidict_py import MultiDict as PyMultiDict  # noqa
+import multidict
 
-try:
-    from multidict._multidict import (  # type: ignore # noqa
-        CIMultiDict,
-        MultiDict,
-    )
-except ImportError:
-    pass
 
-
-def write(name, proto):
-    cls = globals()[name]
+def write(tag, cls, proto):
     d = cls([("a", 1), ("a", 2)])
-    with open("{}.pickle.{}".format(name.lower(), proto), "wb") as f:
+    file_basename = f"{cls.__name__.lower()}-{tag}"
+    with open(f"{file_basename}.pickle.{proto}", "wb") as f:
         pickle.dump(d, f, proto)
 
 
 def generate():
-    if not USE_EXTENSIONS:
-        raise RuntimeError("C Extension is required")
+    _impl_map = {
+        "c-extension": multidict._multidict,
+        "pure-python": multidict._multidict_py,
+    }
     for proto in range(pickle.HIGHEST_PROTOCOL + 1):
-        for name in ("MultiDict", "CIMultiDict", "PyMultiDict", "PyCIMultiDict"):
-            write(name, proto)
+        for tag, impl in _impl_map.items():
+            for cls in impl.CIMultiDict, impl.MultiDict:
+                write(tag, cls, proto)
 
 
 if __name__ == "__main__":

tests/test_abc.py

@@ -3,43 +3,6 @@
 import pytest
 
 from multidict import MultiMapping, MutableMultiMapping
-from multidict._compat import USE_EXTENSIONS
-from multidict._multidict_py import CIMultiDict as PyCIMultiDict
-from multidict._multidict_py import CIMultiDictProxy as PyCIMultiDictProxy
-from multidict._multidict_py import MultiDict as PyMultiDict  # noqa: E402
-from multidict._multidict_py import MultiDictProxy as PyMultiDictProxy
-
-if USE_EXTENSIONS:
-    from multidict._multidict import (  # type: ignore
-        CIMultiDict,
-        CIMultiDictProxy,
-        MultiDict,
-        MultiDictProxy,
-    )
-
-
-@pytest.fixture(
-    params=([MultiDict, CIMultiDict] if USE_EXTENSIONS else [])
-    + [PyMultiDict, PyCIMultiDict],
-    ids=(["MultiDict", "CIMultiDict"] if USE_EXTENSIONS else [])
-    + ["PyMultiDict", "PyCIMultiDict"],
-)
-def cls(request):
-    return request.param
-
-
-@pytest.fixture(
-    params=(
-        [(MultiDictProxy, MultiDict), (CIMultiDictProxy, CIMultiDict)]
-        if USE_EXTENSIONS
-        else []
-    )
-    + [(PyMultiDictProxy, PyMultiDict), (PyCIMultiDictProxy, PyCIMultiDict)],
-    ids=(["MultiDictProxy", "CIMultiDictProxy"] if USE_EXTENSIONS else [])
-    + ["PyMultiDictProxy", "PyCIMultiDictProxy"],
-)
-def proxy_classes(request):
-    return request.param
 
 
 def test_abc_inheritance():
@@ -116,15 +79,14 @@ def test_abc_popall():
         B().popall("key")
 
 
-def test_multidict_inheritance(cls):
-    assert issubclass(cls, MultiMapping)
-    assert issubclass(cls, MutableMultiMapping)
+def test_multidict_inheritance(any_multidict_class):
+    assert issubclass(any_multidict_class, MultiMapping)
+    assert issubclass(any_multidict_class, MutableMultiMapping)
 
 
-def test_proxy_inheritance(proxy_classes):
-    proxy, _ = proxy_classes
-    assert issubclass(proxy, MultiMapping)
-    assert not issubclass(proxy, MutableMultiMapping)
+def test_proxy_inheritance(any_multidict_proxy_class):
+    assert issubclass(any_multidict_proxy_class, MultiMapping)
+    assert not issubclass(any_multidict_proxy_class, MutableMultiMapping)
 
 
 def test_generic_type_in_runtime():