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

Multiple calls to prepare_recipe() within SimpleTestCase fails when recipe uses seq() #132

Open
abbottc opened this issue Nov 24, 2020 · 5 comments
Labels
documentation Improvements or additions to documentation

Comments

@abbottc
Copy link
Contributor

abbottc commented Nov 24, 2020

It doesn't seem possible to create multiple model instances using separate calls to prepare_recipe() within a SimpleTestCase if the recipe uses seq(). This triggers a database call to m.objects.count() in https://github.com/model-bakers/model_bakery/blob/main/model_bakery/recipe.py#L37 which is disallowed inside a SimpleTestCase.

I didn't find this issue documented anywhere, although I may have missed it. It seems at least surprising if not a bug to have a database call in prepare_recipe() since the purpose of that method is to instantiate but not save a model instance. If it turns out this is not fixable, perhaps adding documentation regarding this caveat is appropriate.

Expected behavior

The prepare_recipe() method is always usable within SimpleTestCase.

Actual behavior

The use of prepare_recipe() within SimpleTestCase fails when the recipe contains a seq() and is called more than once.

Reproduction Steps

# models.py
from django.db import models

class MyModel(models.Model):
    my_field = models.IntegerField()


# baker_recipes.py
from model_bakery.recipe import Recipe, seq

from .models import MyModel

my_model = Recipe(MyModel, my_field=seq(1))


# tests.py
from django.test import SimpleTestCase

from model_bakery import baker

class MyTest(SimpleTestCase):
    def test_my_model(self):
        # This test errors due to the db query to m.objects.count() in:
        # https://github.com/model-bakers/model_bakery/blob/main/model_bakery/recipe.py#L37
        # Note that baker.prepare_recipe('myapp.my_model', _quantity=2) would work in this simple example, 
        # but more complex situations may need instances created at separate points in the test
        baker.prepare_recipe('myapp.my_model')
        baker.prepare_recipe('myapp.my_model')

Versions

Python: 3.8.6
Django: 3.0.11
Model Bakery: 1.2.1

@berinhard
Copy link
Member

Hi @abbottc thanks for opening this issue.

Actually, you're right about this not being able to be fixed. There's a hack to "reset" the seq call after the test finishes without creating any extra API for bakery. The hack is this query count you've noticed. The motivation behind this hack was: how to make the recipe to know that the seq should start from the beginning once a second test uses it?

This is being explicitly tested by 2 test methods at test_recipes.py:

The decision behind this was to not add extra parameters to make_recipe or more complex pieces of code such as context managers or custom test runners to handle the iterator reset. Model-bakery always had this goal to be a test utility, not a Django app. That's why people doesn't even need to add it to INSTALLED_APPS.

So, given this context, I totally agree with you that a comment on bakery's limitation to not be used with SimpleTestCase due to internal optimized queries it can perform when using recipes is definitely a good thing. I think the best place to be updated is the recipes docs.

Happy baking =}

@berinhard berinhard added the documentation Improvements or additions to documentation label Jun 16, 2021
@abbottc
Copy link
Contributor Author

abbottc commented Jun 16, 2021

Hi @berinhard,

Thanks for following up on the issue and for the detail in explaining the rationale behind the query that gets executed in this scenario. If you'd like me to submit a PR making the minor doc addition, let me know.

@berinhard
Copy link
Member

Sure, that would be great!

@abbottc
Copy link
Contributor Author

abbottc commented Jun 17, 2021

Thanks, the PR is up.

@abbottc
Copy link
Contributor Author

abbottc commented Jun 17, 2021

@berinhard Also, thinking about what you explained regarding the query led me to consider other issues related to that which could be considered less minor than this issue, and I've documented an example in #209.

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

No branches or pull requests

2 participants