FactoryBoy / factory_boy

A test fixtures replacement for Python
https://factoryboy.readthedocs.io/
MIT License
3.49k stars 392 forks source link

Take default values into account in DjangoModelFactory.build() #330

Open aaugustin opened 7 years ago

aaugustin commented 7 years ago

Let's assume I have a model with an autogenerated field:

    token = models.CharField(
        verbose_name=_("token"),
        help_text=_("Auto-generated token"),
        max_length=12,
        default=crypto.get_random_string,
        editable=False,
    )

and I leave out this field in the definition of the corresponding factory.

If I use DjangoModelFactory.create() then Django sets the default value before saving the instance to the database, and everything is fine.

If I use DjangoModelFactory.build() (the database is hot lava, etc.) then the default value isn't set.

I can understand why factory_boy has this behavior. However I find it frustrating. My best option for making DjangoModelFactory.build() usable right now is to declare:

    token = factory.LazyFunction(crypto.get_random_string)

or:

    token = factory.LazyFunction(MyModel._meta.get_field('token').default)

The former duplicates logic — if I change the default value in the model, my factory will go out of sync. The latter is boilerplate-ish.

It's especially frustrating in cases like this one where the value is autogenerated on creation and never set in any other way...

The obvious solution is factory.FieldDefaultOnlyForBuild but that seems too specific to the use case described here.

Is this a problem you've seen before? Are you interested in investigating solutions?

aaugustin commented 7 years ago

In fact, this seems to do the job.

import factory

class DjangoModelFactoryWithDefaults(factory.DjangoModelFactory):

    @classmethod
    def _build(cls, model_class, *args, **kwargs):
        """Use default values for fields that have them.

        Since the model hasn't been saved to the database, Django hasn't had
        the opportunity to set defaults.

        See https://github.com/FactoryBoy/factory_boy/issues/330 for details.

        """
        # Add default values to models fields that don't already have a value
        # and for which the field definition provides a default. We have to
        # call the private _get_fields API in order to exclude reverse fields
        # which don't have an `has_default` property.
        kwargs.update({
            field.name: field.get_default()
            for field in model_class._meta._get_fields(reverse=False)
            if field.name not in kwargs and field.has_default()
        })
        return super()._build(model_class, *args, **kwargs)

If you think there's a better approach or would like to make changes to factory_boy (perhaps just documenting this pitfall of .build()) let's keep this ticket open, otherwise it can be closed safely.

jeffwidman commented 7 years ago

The problem makes sense, and I understand why it's frustrating. It's been a little while, but I think I hit similar problems last spring with SQLAlchemy + FactoryBoy. So yes, we are open to solutions.

I'd like @rbarrois to take a look at this one as he may have thoughts re: using the private API.

Whatever the end result is, I would like to see it applied to both Django and SQLAlchemy (and I'm happy to help implement the latter).

stephane commented 7 years ago

It reminds me the _force_instantdefaults() of SQLAlchemy Utils http://sqlalchemy-utils.readthedocs.io/en/latest/_modules/sqlalchemy_utils/listeners.html#force_instant_defaults