Skip to content
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 13 commits into from
Jun 11, 2022

Conversation

youtux
Copy link
Contributor

@youtux youtux commented Jun 6, 2022

Implementing the idea previously described in #163 (comment).

We add a helper function, named_model, that makes sure the user uses a model with a specific name, rather than a generic dict, list, etc.

This way we avoid problems that arises when SubFactory and RelatedFactory are used with factories for generic models. See the tests for examples.

It will be helpful when using SubFactory and RelatedFactory for generic types like dict, str, etc.
tests/test_model_name.py Outdated Show resolved Hide resolved
pytest_factoryboy/fixture.py Outdated Show resolved Hide resolved

def test_generic_model_with_custom_name_no_warning(testdir):
testdir.makepyfile(
"""
Copy link
Contributor

Choose a reason for hiding this comment

The 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

@youtux youtux marked this pull request as ready for review June 11, 2022 08:22
@youtux youtux enabled auto-merge June 11, 2022 08:28
@youtux youtux disabled auto-merge June 11, 2022 08:37
@youtux youtux merged commit 0f0c7c1 into master Jun 11, 2022
@youtux youtux deleted the named_model branch June 11, 2022 08:37
Comment on lines +40 to +42
with warnings.catch_warnings():
warnings.simplefilter("error")
assert get_model_name(ModelFactory) == "foo"
Copy link
Contributor

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:

    with pytest.warns(None) as recorded_warnings:
        assert get_model_name(ModelFactory) == "foo"
        assert not recorded_warnings

We can also pass the exact warning class (UserWarning in our case) instead of None.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants