Merge pull request #167 from pytest-dev/named_model

Add `named_model` function and raise warning for factories using generic models

CHANGES.rst

@@ -3,6 +3,7 @@ Changelog
 
 Unreleased
 ----------
+- Using a generic class container like ``dict``, ``list``, ``set``, etc. will raise a warning suggesting you to wrap your model using ``named_model(...)``. Doing this will make sure that the fixture name is correctly chosen, otherwise SubFactory and RelatedFactory aren't able to determine the name of the model. See `Generic Container Classes as models <https://pytest-factoryboy.readthedocs.io/en/latest/#generic-container-classes-as-models>`_ `#167 <https://github.com/pytest-dev/pytest-factoryboy/pull/167>`_
 - Fix ``Factory._after_postgeneration`` being invoked twice. `#164 <https://github.com/pytest-dev/pytest-factoryboy/pull/164>`_ `#156 <https://github.com/pytest-dev/pytest-factoryboy/issues/156>`_
 
 2.4.0

README.rst

@@ -281,6 +281,33 @@ LazyFixture constructor accepts either existing fixture name or callable with de
     register(BookFactory, "another_book", author=LazyFixture("another_author"))
 
 
+Generic container classes as models
+-----------------------------------
+It's often useful to create factories for ``dict`` or other common generic container classes.
+In that case, you should wrap the container class around ``named_model(...)``, so that pytest-factoryboy can correctly determine the model name when using it in a SubFactory or RelatedFactory.
+
+Pytest-factoryboy will otherwise raise a warning.
+
+For example:
+
+.. code-block:: python
+
+    import factory
+    from pytest_factoryboy import named_model, register
+
+    @register
+    class JSONPayload(factory.Factory):
+        class Meta:
+            model = named_model("JSONPayload", dict)
+
+        name = "foo"
+
+
+    def test_foo(json_payload):
+        assert json_payload.name == "foo"
+
+As a bonus, factory is automatically registering the ``json_payload`` fixture (rather than ``dict``), so there is no need to override ``@register(_name="json_payload"))``.
+
 Post-generation dependencies
 ============================
 

pyproject.toml

@@ -40,6 +40,8 @@ typing_extensions = "*"
 [tool.poetry.dev-dependencies]
 mypy = "^0.960"
 tox = "^3.25.0"
+packaging = "^21.3"
+importlib-metadata = "^4.11.4"
 
 [build-system]
 requires = ["poetry-core>=1.0.0"]

pytest_factoryboy/__init__.py

@@ -1,4 +1,4 @@
 """pytest-factoryboy public API."""
-from .fixture import LazyFixture, register
+from .fixture import LazyFixture, named_model, register
 
-__all__ = ("register", "LazyFixture")
+__all__ = ("register", "named_model", "LazyFixture")

pytest_factoryboy/fixture.py

@@ -4,6 +4,7 @@
 import contextlib
 import functools
 import sys
+import warnings
 from dataclasses import dataclass
 from inspect import signature
 from types import MethodType
@@ -43,6 +44,7 @@
 P = ParamSpec("P")
 
 SEPARATOR = "__"
+WARN_FOR_MODEL_TYPES = frozenset({dict, list, set, tuple, frozenset})
 
 
 @dataclass(eq=False)
@@ -56,6 +58,11 @@ def __call__(self, request: SubRequest) -> Any:
         return self.function(request)
 
 
+def named_model(model_cls: type[T], name: str) -> type[T]:
+    """Return a model class with a given name."""
+    return type(name, (model_cls,), {})
+
+
 # register(AuthorFactory, ...)
 #
 # @register
@@ -239,11 +246,22 @@ def inject_into_caller(name: str, function: Callable[..., Any], locals_: dict[st
 
 def get_model_name(factory_class: FactoryType) -> str:
     """Get model fixture name by factory."""
-    return (
-        inflection.underscore(factory_class._meta.model.__name__)
-        if not isinstance(factory_class._meta.model, str)
-        else factory_class._meta.model
-    )
+    model_cls = factory_class._meta.model
+
+    if isinstance(model_cls, str):
+        return model_cls
+
+    model_name = inflection.underscore(model_cls.__name__)
+    if model_cls in WARN_FOR_MODEL_TYPES:
+        warnings.warn(
+            f"Using a {model_cls} as model type for {factory_class} is discouraged by pytest-factoryboy, "
+            f"as it assumes that the model name is {model_name!r} when using it as SubFactory or RelatedFactory, "
+            "which is too generic and probably not what you want.\n"
+            "You can giving an explicit name to the model by using:\n"
+            f'model = named_model({model_cls.__name__}, "Foo")',
+        )
+
+    return model_name
 
 
 def get_factory_name(factory_class: FactoryType) -> str:

tests/compat.py

@@ -0,0 +1,55 @@
+from __future__ import annotations
+
+import sys
+
+from _pytest.pytester import RunResult
+from packaging.version import Version
+
+if sys.version_info >= (3, 8):
+    from importlib import metadata
+else:
+    import importlib_metadata as metadata
+
+PYTEST_VERSION = Version(metadata.version("pytest"))
+
+if PYTEST_VERSION >= Version("6.0.0"):
+
+    def assert_outcomes(
+        result: RunResult,
+        passed: int = 0,
+        skipped: int = 0,
+        failed: int = 0,
+        errors: int = 0,
+        xpassed: int = 0,
+        xfailed: int = 0,
+    ) -> None:
+        """Compatibility function for result.assert_outcomes"""
+        result.assert_outcomes(
+            errors=errors,
+            passed=passed,
+            skipped=skipped,
+            failed=failed,
+            xpassed=xpassed,
+            xfailed=xfailed,
+        )
+
+else:
+
+    def assert_outcomes(
+        result: RunResult,
+        passed: int = 0,
+        skipped: int = 0,
+        failed: int = 0,
+        errors: int = 0,
+        xpassed: int = 0,
+        xfailed: int = 0,
+    ) -> None:
+        """Compatibility function for result.assert_outcomes"""
+        result.assert_outcomes(
+            error=errors,  # Pytest < 6 uses the singular form
+            passed=passed,
+            skipped=skipped,
+            failed=failed,
+            xpassed=xpassed,
+            xfailed=xfailed,
+        )  # type: ignore[call-arg]

tests/conftest.py

@@ -0,0 +1 @@
+pytest_plugins = "pytester"

tests/test_model_name.py

@@ -0,0 +1,176 @@
+import warnings
+
+import factory
+import pytest
+
+from pytest_factoryboy.fixture import get_model_name, named_model
+from tests.compat import assert_outcomes
+
+
+def make_class(name: str):
+    """Create a class with the given name."""
+    return type(name, (object,), {})
+
+
+@pytest.mark.parametrize("model_cls", [dict, set, list, frozenset, tuple])
+def test_get_model_name_warns_for_common_containers(model_cls):
+    """Test that a warning is raised when common containers are used as models."""
+
+    class ModelFactory(factory.Factory):
+        class Meta:
+            model = model_cls
+
+    with pytest.warns(
+        UserWarning,
+        match=rf"Using a .*{model_cls.__name__}.* as model type for .*ModelFactory.* is discouraged",
+    ):
+        assert get_model_name(ModelFactory)
+
+
+def test_get_model_name_does_not_warn_for_user_defined_models():
+    """Test that no warning is raised for when using user-defined models"""
+
+    class Foo:
+        pass
+
+    class ModelFactory(factory.Factory):
+        class Meta:
+            model = Foo
+
+    with warnings.catch_warnings():
+        warnings.simplefilter("error")
+        assert get_model_name(ModelFactory) == "foo"
+
+
+@pytest.mark.parametrize(
+    ["model_cls", "expected"],
+    [
+        (make_class("Foo"), "foo"),
+        (make_class("TwoWords"), "two_words"),
+        (make_class("HTTPHeader"), "http_header"),
+        (make_class("C3PO"), "c3_po"),
+    ],
+)
+def test_get_model_name(model_cls, expected):
+    """Test normal cases for ``get_model_name``."""
+
+    class ModelFactory(factory.Factory):
+        class Meta:
+            model = model_cls
+
+    assert get_model_name(ModelFactory) == expected
+
+
+def test_named_model():
+    """Assert behaviour of ``named_model``."""
+    cls = named_model(dict, "Foo")
+
+    assert cls.__name__ == "Foo"
+    assert issubclass(cls, dict)
+
+
+def test_generic_model_with_custom_name_no_warning(testdir):
+    testdir.makepyfile(
+        """
+        from factory import Factory
+        from pytest_factoryboy import named_model, register
+
+        @register
+        class JSONPayloadFactory(Factory):
+            class Meta:
+                model = named_model(dict, "JSONPayload")
+            foo = "bar"
+
+
+        def test_payload(json_payload: dict):
+            assert isinstance(json_payload, dict)
+            assert json_payload["foo"] == "bar"
+        """
+    )
+    result = testdir.runpytest("-Werror")  # Warnings become errors
+    assert_outcomes(result, passed=1)
+
+
+def test_generic_model_name_raises_warning(testdir):
+    testdir.makepyfile(
+        """
+        import builtins
+        from factory import Factory
+        from pytest_factoryboy import register
+
+        @register
+        class JSONPayloadFactory(Factory):
+            class Meta:
+                model = dict
+            foo = "bar"
+
+
+        def test_payload(dict):
+            assert isinstance(dict, builtins.dict)
+            assert dict["foo"] == "bar"
+    """
+    )
+    result = testdir.runpytest()
+    assert_outcomes(result, passed=1)
+    result.stdout.fnmatch_lines(
+        "*UserWarning: Using a *class*dict* as model type for *JSONPayloadFactory* is discouraged*"
+    )
+
+
+def test_generic_model_with_register_override_no_warning(testdir):
+    testdir.makepyfile(
+        """
+        from factory import Factory
+        from pytest_factoryboy import named_model, register
+
+        @register(_name="json_payload")
+        class JSONPayloadFactory(Factory):
+            class Meta:
+                model = dict
+            foo = "bar"
+
+
+        def test_payload(json_payload: dict):
+            assert isinstance(json_payload, dict)
+            assert json_payload["foo"] == "bar"
+
+        """
+    )
+    result = testdir.runpytest("-Werror")  # Warnings become errors
+    assert_outcomes(result, passed=1)
+
+
+def test_using_generic_model_name_for_subfactory_raises_warning(testdir):
+    testdir.makepyfile(
+        """
+        import builtins
+        from factory import Factory, SubFactory
+        from pytest_factoryboy import register
+
+        @register(_name="JSONPayload")
+        class JSONPayloadFactory(Factory):
+            class Meta:
+                model = dict  # no warning raised here, since we override the name at the @register(...)
+            foo = "bar"
+
+        class HTTPRequest:
+            def __init__(self, json: dict):
+                self.json = json
+
+        @register
+        class HTTPRequestFactory(Factory):
+            class Meta:
+                model = HTTPRequest
+
+            json = SubFactory(JSONPayloadFactory)  # this will raise a warning
+
+        def test_payload(http_request):
+            assert http_request.json["foo"] == "bar"
+        """
+    )
+
+    result = testdir.runpytest()
+    assert_outcomes(result, errors=1)
+    result.stdout.fnmatch_lines(
+        "*UserWarning: Using *class*dict* as model type for *JSONPayloadFactory* is discouraged*"
+    )