-
Notifications
You must be signed in to change notification settings - Fork 42
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add named_model
function and raise warning for factories using generic models
#167
Merged
Merged
Changes from 8 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
7f31cb2
Add named_model to explicitly define the model name.
youtux c8b759b
Add compatibility function
youtux 4a009f4
Don't tell the user how to shoot their feet.
youtux 0e5ad34
Update pytest_factoryboy/fixture.py
youtux 8c97b68
Fix indentation
youtux 9e23c7f
Fix closing quotes
youtux 51330f5
Add unit tests
youtux 6b02718
Add section to the readme
youtux 2ac0b06
Fix markup
youtux 72c3e0c
Add side effect explaination
youtux 23ef366
Improve explaination
youtux 73109d5
Add changelog
youtux 9970cda
Fix changelog
youtux File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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") |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
pytest_plugins = "pytester" |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -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( | ||
""" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIP: let's indent whole string content or just this line to make code more readable |
||
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*" | ||
) |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
NIP: to make this test more explicit and obvious we can check emitted warnings in an assert statement explicitly:
We can also pass the exact warning class (
UserWarning
in our case) instead ofNone
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is the way that pytest suggests: https://docs.pytest.org/en/stable/how-to/capture-warnings.html#:~:text=To%20ensure%20that%20no%20warnings%20are%20emitted%2C%20use%3A
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh nice, somehow I missed it in their docs, thanks!