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

Fix random generation of ContentType values when there is no database access #290

Merged

Conversation

ento
Copy link
Contributor

@ento ento commented Feb 7, 2022

Context

I wrote a test in pytest that does:

baker.prepare(SomeModel, _fill_optional=True)

and it failed with:

RuntimeError: Database access not allowed, use the "django_db" mark, or the "db" or "transactional_db" fixtures to enable it.

model_bakery was trying to generate a ContentType value to fill in with and the first thing it does is to fetch a random value from the database:

path/to/venv/lib/python3.7/site-packages/model_bakery/random_gen.py:208: in gen_content_type
    return ContentType.objects.get_for_model(choice(apps.get_models()))

The gen_content_type() function tries to remediate when database access is not available:
https://github.com/model-bakers/model_bakery/blob/1.4.0/model_bakery/random_gen.py#L207-L211

    try:
        return ContentType.objects.get_for_model(choice(apps.get_models()))
    except AssertionError:
        warnings.warn("Database access disabled, returning ContentType raw instance")
        return ContentType()

But in the case of pytest-django, a RuntimeError is raised, not an AssertionError: https://github.com/pytest-dev/pytest-django/blob/v4.5.2/pytest_django/plugin.py#L709-L713

What this PR does

This PR adds RuntimeError to the exception classes that are caught by the try-except block in gen_content_type().

Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

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

Hi @ento! Thanks for opening your PR and contributing with model-bakery!

The code looks fine and it's good for me. Thanks for adding tests as well. Although I liked it, we can't merge your PR yet because there are some conflicts which are preventing the CI to run. Can you update your local main branch with the most up to date version and merge it into yours fix branch? That way the CI actions will be triggered and we'll feel more confident about merging your code.

We'd love to make it into the next 1.5.0 release =)

…ithout-db

Modified tests/test_baker.py in order to fix this pre-commit error from
pydocstyle:

> tests/test_baker.py:574 in public method `test_ensure_reverse_fk_for_many_to_one_is_working`:
>         D400: First line should end with a period (not 'd')
@ento
Copy link
Contributor Author

ento commented Apr 1, 2022

Can you update your local main branch with the most up to date version and merge it into yours fix branch? That way the CI actions will be triggered and we'll feel more confident about merging your code.

👍 Done. Although it looks like we need a manual approval to run the CI checks, as this is my first PR against this repo.

image

@berinhard
Copy link
Member

berinhard commented Apr 1, 2022

Hi @ento! The tests are running now, but it seems one of them is failing:

  tests/test_baker.py:622: in test_create_model_with_contenttype_field
      dummy = baker.prepare(models.DummyGenericForeignKeyModel)
  E   Failed: DID NOT WARN. No warnings of type (<class 'UserWarning'>,) were emitted. The list of emitted warnings is: [].

@ento
Copy link
Contributor Author

ento commented Apr 2, 2022

@berinhard Pushed a fix. Good thing it failed in that run, as the failure was a flaky one in nature.

@berinhard berinhard merged commit b5afc92 into model-bakers:main Apr 3, 2022
@ento ento deleted the fix-gen-contenttype-without-db branch April 6, 2022 05:17
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.

None yet

3 participants