model-bakers/model_bakery

`_save_kwargs` inside of a recipe definition overridden

jeremy-engel opened this issue · 6 comments

As a result of #292, when certain attributes are passed as arguments to Recipe, they are overridden by defaults before the recipe is used to create the object.

Expected behavior

When using a recipe such as my_recipe = Recipe("MyModel", _save_kwargs={"foo": "bar"}), by running baker.make_recipe(my_recipe), the kwargs for the model's save should include the key foo with value bar.

Actual behavior

The kwargs are empty using this recipe. This contrasts with my_recipe_2 = Recipe("MyModel"), where by running baker.make_recipe(my_recipe_2, _save_kwargs={"foo": "bar"}), the correct kwargs are used.

Reproduction Steps

Below are updates to the project test files demonstrating the issue.

# tests.generic.models

class ModelWithSaveKwargs(Dog):
    def save(self, *args, **kwargs):
        print(kwargs)
        self.breed = kwargs.pop("breed")
        return super(ModelWithSaveKwargs, self).save(*args, **kwargs)

# tests.generic.baker_recipes

with_save_kwargs = Recipe("generic.ModelWithSaveKwargs", _save_kwargs={"breed": "updated_breed"})

# tests.test_recipes.TestExecutingRecipes

    def test_pass_save_kwargs_in_recipe_definition(self):
        dog = baker.make_recipe("tests.generic.with_save_kwargs")
        assert dog.breed == "updated_breed"

And as a patch:

diff --git a/tests/generic/baker_recipes.py b/tests/generic/baker_recipes.py
index f4e27c7..a8aebc8 100644
--- a/tests/generic/baker_recipes.py
+++ b/tests/generic/baker_recipes.py
@@ -113,6 +113,8 @@ movie_with_cast = Recipe(
 
 overrided_save = Recipe("generic.ModelWithOverridedSave")
 
+with_save_kwargs = Recipe("generic.ModelWithSaveKwargs", _save_kwargs={"breed": "updated_breed"})
+
 ip_fields = Recipe(
     "generic.DummyGenericIPAddressFieldModel",
     ipv4_field=seq("127.0.0.", increment_by=2),
diff --git a/tests/generic/models.py b/tests/generic/models.py
index 0acf214..c3248cb 100755
--- a/tests/generic/models.py
+++ b/tests/generic/models.py
@@ -226,6 +226,12 @@ class ModelWithOverridedSave(Dog):
         return super(ModelWithOverridedSave, self).save(*args, **kwargs)
 
 
+class ModelWithSaveKwargs(Dog):
+    def save(self, *args, **kwargs):
+        self.breed = kwargs.pop("breed")
+        return super(ModelWithSaveKwargs, self).save(*args, **kwargs)
+
+
 class Classroom(models.Model):
     students = models.ManyToManyField(Person, null=True)
     active = models.BooleanField(null=True)
diff --git a/tests/test_recipes.py b/tests/test_recipes.py
index 45d8780..ab5163c 100644
--- a/tests/test_recipes.py
+++ b/tests/test_recipes.py
@@ -332,6 +332,10 @@ class TestExecutingRecipes:
         )
         assert owner == dog.owner
 
+    def test_pass_save_kwargs_in_recipe_definition(self):
+        dog = baker.make_recipe("tests.generic.with_save_kwargs")
+        assert dog.breed == "updated_breed"
+
     def test_ip_fields_with_start(self):
         first, second = baker.make_recipe("tests.generic.ip_fields", _quantity=2)

Versions

Python: 3.10.4
Django: 4.0.3
Model Bakery: 1.5.0

I'm seeing the same with _create_files=True as well, bisected to #292 as well (to be more exact, this commit: Better type hints for model_bakery.recipe by @hoefling · Pull Request #292 · model-bakers/model_bakery).

With this recipe, I now get:

...
  File "/srv/www/studentenportal/apps/documents/models.py", line 159, in exists
    return os.path.exists(self.document.path)
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/files.py", line 58, in path
    self._require_file()
  File "/usr/local/lib/python3.7/site-packages/django/db/models/fields/files.py", line 40, in _require_file
    raise ValueError("The '%s' attribute has no file associated with it." % self.field.name)
ValueError: The 'document' attribute has no file associated with it.

Hey @jeremy-engel,

Thanks for the great issue report with the test patch! That helped a lot during the investigation. And I am sorry it took us so long to process the report.

@The-Compiler and thank you for sharing another failing case!

I came up with a draft solution in #330, which is not quite elegant (given my lack of experience with typed things).

I'd love to hear your opinions here.

@model-bakers/core please, take a look as well, maybe you have some ideas on improving this piece.

Best,
Rust

@jeremy-engel @The-Compiler hey! It took us a while, but we released a new version that is addressing this issue.

Please, at your convenience, take a look at 1.7.0 and check if this solves everything for you!

This release takes care of it for me. Thanks!

I'm a bit late to the party as well, but indeed I just upgraded from 1.4.0 to 1.7.0 without any issues - thanks, @amureki!

Fantastic, thanks everyone, for the cooperation! 💯