Merge branch 'bugfixes/tests-import-loop'

PR #915

.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

@@ -26,4 +26,4 @@ warn_unused_ignores = True
 warn_return_any = True
 
 [mypy-test_multidict]
-disable_error_code = misc
+disable_error_code = call-arg

CHANGES/837.contrib.rst

@@ -0,0 +1 @@
+915.contrib.rst

CHANGES/915.contrib.rst

@@ -0,0 +1,12 @@
+The test framework has been refactored. In the previous state, the circular
+imports reported in :issue:`837` caused the C-extension tests to be skipped.
+
+Now, there is a set of the ``pytest`` fixtures that is set up in a parametrized
+manner allowing to have a consistent way of accessing mirrored ``multidict``
+implementations across all the tests.
+
+This change also implemented a pair of CLI flags (``--c-extensions`` /
+``--no-c-extensions``) that allow to explicitly request deselecting the tests
+running against the C-extension.
+
+-- by :user:`webknjaz`.

tests/conftest.py

@@ -1,25 +1,222 @@
+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
+
+C_EXT_MARK = pytest.mark.c_extension
+PY_38_AND_BELOW = _version_info < (3, 9)
+
+
+@dataclass(frozen=True)
+class MultidictImplementation:
+    """A facade for accessing importable multidict module variants.
+
+    An instance essentially represents a c-extension or a pure-python module.
+    The actual underlying module is accessed dynamically through a property and
+    is cached.
+
+    It also has a text tag depending on what variant it is, and a string
+    representation suitable for use in Pytest's test IDs via parametrization.
+    """
+
+    is_pure_python: bool
+    """A flag showing whether this is a pure-python module or a C-extension."""
+
+    @cached_property
+    def tag(self) -> str:
+        """Return a text representation of the pure-python attribute."""
+        return "pure-python" if self.is_pure_python else "c-extension"
+
+    @cached_property
+    def imported_module(self) -> ModuleType:
+        """Return a loaded importable containing a multidict variant."""
+        importable_module = "_multidict_py" if self.is_pure_python else "_multidict"
+        return import_module(f"multidict.{importable_module}")
+
+    def __str__(self):
+        """Render the implementation facade instance as a string."""
+        return f"{self.tag}-module"
 
-OPTIONAL_CYTHON = (
-    ()
-    if USE_EXTENSIONS
-    else pytest.mark.skip(reason="No extensions available")
+
+@pytest.fixture(
+    scope="session",
+    params=(
+        pytest.param(
+            MultidictImplementation(is_pure_python=False),
+            marks=C_EXT_MARK,
+        ),
+        MultidictImplementation(is_pure_python=True),
+    ),
+    ids=str,
 )
+def multidict_implementation(request) -> MultidictImplementation:
+    """Return a multidict variant facade."""
+    return request.param
+
 
+@pytest.fixture(scope="session")
+def multidict_module(
+    multidict_implementation: MultidictImplementation,
+) -> ModuleType:
+    """Return a pre-imported module containing a multidict variant."""
+    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 a class name of a mutable multidict implementation."""
+    return request.param
+
+
+@pytest.fixture(scope="session")
+def any_multidict_class(
+    any_multidict_class_name: str,
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    """Return a class object of a mutable multidict implementation."""
+    return getattr(multidict_module, any_multidict_class_name)
+
+
+@pytest.fixture(scope="session")
+def case_sensitive_multidict_class(
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    """Return a case-sensitive mutable multidict class."""
+    return multidict_module.MultiDict
+
+
+@pytest.fixture(scope="session")
+def case_insensitive_multidict_class(
+    multidict_module: ModuleType,
+) -> Type[MutableMultiMapping[str]]:
+    """Return a case-insensitive mutable multidict class."""
+    return multidict_module.CIMultiDict
+
+
+@pytest.fixture(scope="session")
+def case_insensitive_str_class(multidict_module: ModuleType) -> Type[str]:
+    """Return a case-insensitive string class."""
+    return multidict_module.istr
+
+
+@pytest.fixture(scope="session")
+def any_multidict_proxy_class_name(any_multidict_class_name: str) -> str:
+    """Return a class name of an immutable multidict implementation."""
+    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 an immutable multidict implementation class object."""
+    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 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]]:
+    """Return a case-insensitive immutable multidict class."""
+    return multidict_module.CIMultiDictProxy
+
+
+@pytest.fixture(scope="session")
+def multidict_getversion_callable(multidict_module: ModuleType) -> Callable:
+    """Return a ``getversion()`` function for current implementation."""
+    return multidict_module.getversion
+
+
+def pytest_addoption(
+    parser: pytest.Parser,
+    pluginmanager: pytest.PytestPluginManager,
+) -> None:
+    """Define a new ``--c-extensions`` flag.
+
+    This lets the callers deselect tests executed against the C-extension
+    version of the ``multidict`` implementation.
+    """
+    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_collection_modifyitems(
+    session: pytest.Session,
+    config: pytest.Config,
+    items: list[pytest.Item],
+) -> None:
+    """Deselect tests against C-extensions when requested via CLI."""
+    test_c_extensions = config.getoption("--c-extensions") is True
+
+    if test_c_extensions:
+        return
+
+    selected_tests = []
+    deselected_tests = []
+
+    for item in items:
+        c_ext = item.get_closest_marker(C_EXT_MARK.name) is not None
+
+        target_items_list = deselected_tests if c_ext else selected_tests
+        target_items_list.append(item)
+
+    config.hook.pytest_deselected(items=deselected_tests)
+    items[:] = selected_tests
+
+
+def pytest_configure(config: pytest.Config) -> None:
+    """Declare the C-extension marker in config."""
+    config.addinivalue_line(
+        "markers",
+        f"{C_EXT_MARK.name}: "
+        "tests running against the C-extension implementation.",
+    )
 
 
 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():