Populating PostGenerationMethodCall via Traits

[flaeppe]

Consider the following approach (with 2.11.1):

from django.test import TestCase

import factory


class UserFactory(factory.django.DjangoModelFactory):
    username = factory.Sequence(lambda n: 'USER_%03d' % n)
    password = factory.PostGenerationMethodCall('set_password', 'supersecret')

    class Meta:
        model = 'auth.User'

    class Params:
        weak_password = factory.Trait(password='notsecretatall')


class PostGenerationMethodAndTraits(TestCase):

    def test_populate_post_generation_through_trait(self):
        user = UserFactory(weak_password=True)
        self.assertTrue(user.check_password('notsecretatall'))

Which gives the following results:

Traceback (most recent call last):
  File "/app/src/post_and_traits/tests.py", line 21, in test_populate_post_generation_through_trait
    self.assertTrue(user.check_password('notsecretatall'))
AssertionError: False is not true

I'm not sure if this is intended behaviour or unintended, as it works with <=2.8.1.

However, with >2.8.1 one has to redeclare the PostGenerationMethodCall in the Trait in order to make it work: e.g.

weak_password = factory.Trait(password=factory.PostGenerationMethodCall('set_password', 'notsecretatall'))

This could fall as far to being a design decision; whether the generation approach should have to be redeclared completely or if the generation approach should be "static", hence the traits would only make the argument(and keywords) differ.

I'm not sure what is wanted here, although I've personally run in to very few cases where the method would differ rather than only the argument(and keywords) and thus would see it fitting if one only had to write weak_password = factory.Trait(password='notsecretatall') for changing argument for PostGenerationMethodCall. And instead maybe use PostGeneration for the eventual case of changing both method and argument.

[rbarrois]

I agree, this group of features is not intuitive at all...

The provided API is not consistent with usual Python and factory_boy behaviour:

1. You define a factory, with a PostGenerationMethodCall or a @post_generation decoration
2. If you pass (when calling the factory, or through a SubFactory) another PostGeneration declaration, it replaces the previous one — OK so far
3. If you pass (when calling or with a SubFactory) a scalar value or a non-PostGeneration declaration, it gets passed as an argument to your method

The more I look at it, the more I feel step 3 to be counter-intuitive...

I'd like to find a cleaner API for that feature. How, as a user, would you expect to perform those functions? Maybe with more options to the decorator?

[flaeppe]

I'm on your side and would say that intuition is the key here and regardless if one would branch on behaviour at the API:s top-level (i.e. two separate declarations -> PostGenerationMethodCall and PostGenerationX) or "under the hood" configurable through options to PostGenerationMethodCall/@post_generation, it would have to become clear what usage would fit what situation.

At the top of my head, I'm imagining the following use cases, where we can pan out differently:

(1.) When the intention is to mutate the method/function being called post generation we might have the (generated) object acting with a function based API where the object is passed as argument. We can say that we're working with n callback methods/functions, on post generation the object is to go through 1 of them (to make it a simpler case).

(2.) When the intention is to mutate the argument(s)/keywords to a set method/function being called post generation we might have an API acting on an input argument together with the (generated) object. We can say that we're working with 1 callback method/function, on post generation it can be called with an argument of n (different) values.

Some example code: (note that I'm trying to give 2 separate implementation approaches supporting/enabling identical functionality)

# Example Case 1.

class MyObjWithState(models.Model):
    def preprocess(self, *args, **kwargs):
        # Do stuff

    def accept(self, *args, **kwargs):
        # Do stuff

    def decline(self, *args, **kwargs):
        # Do stuff


class MyObjWithStateFactory(factory.django.DjangoModelFactory):
    state = factory.PostGenerationMethodCall('preprocess')

    class Meta:
        model = MyObjWithState

    class Params:
        accepted = factory.Trait(state=factory.PostGenerationMethodCall('accept'))
        declined = factory.Trait(state=factory.PostGenerationMethodCall('decline'))

# Example Case 2.

class MyObjWithState(models.Model):
    def set_state(self, *args, **kwargs):
        # Do stuff


class MyObjWithStateFactory(factory.django.DjangoModelFactory):
    state = factory.PostGenerationMethodCall('set_state', 'preprocessed')

    class Meta:
        model = MyObjWithState

    class Params:
        accepted = factory.Trait(state=factory.PostGenerationMethodCall('set_state', 'accepted'))
        declined = factory.Trait(state=factory.PostGenerationMethodCall('set_state', 'declined'))

Unfortunately, as we've concluded in previous comments, a user with approach Case 2 would have to suffer a bit with his implementation, although it looks completely fine for Case 1.

I personally don't have any opinion if one would have to write(apart from that this could clash badly with kwargs to set_state) e.g.:

state = factory.PostGenerationMethodCall('set_state', 'preprocessed', options={'x': x})

It could look fine with @post_generation:

@factory.post_generation(options={'x': x})
def state(*args, **kwargs):
    pass

As long as this would allow:

3. If you pass (when calling or with a SubFactory) a scalar value or a non-PostGeneration declaration, it gets passed as an argument to your method

e.g.

class Params:
    accepted = factory.Trait(state='accepted')
    declined = factory.Trait(state='declined')

The best scenario might even be, for intuition, to add a separate declaration for when passing a non-PostGeneration declaration:

factory.PostGenerationMethodInput(method_name='set_state', *args, **kwargs)

(I find the naming here to be quite difficult, PostGenerationMethodInput is just an example)

What do you think, are we abusing e.g. PostGenerationMethodCall when trying to reuse it for multiple approaches like this?

[dlobue]

@rbarrois

Actually, there is a bug here in that post generation isn't being called when a trait defines an argument that should be passed to the post-generate method. Here's a simple example:

import factory


class Target(object):
    def __init__(self, m2m=None):
        self.m2m = m2m or []


class TargetFactory(factory.Factory):
    class Meta:
        model = Target

    class Params:
        empty = factory.Trait(
            m2m=None,
            m2m__num=1,
        )

    m2m__num = 3

    @factory.post_generation
    def m2m(self, create, extracted, num=0, **kwargs):
        print('In m2m post_generation: %r, %r' % ((create, extracted, num), kwargs))
        for i in range(num):
            self.m2m.append(extracted)


def main():
    print("no args")
    value = TargetFactory()
    print('value of m2m: %r\n' % value.m2m)

    print("m2m given argument")
    value = TargetFactory(m2m='blarg')
    print('value of m2m: %r\n' % value.m2m)

    print("m2m given argument and empty trait active")
    value = TargetFactory(empty=True, m2m='blarg')
    print('value of m2m: %r\n' % value.m2m)


if __name__ == '__main__':
    main()

Here's the output of this script with version 2.11.1:

no args
In m2m post_generation: (True, None, 3), {}
value of m2m: [None, None, None]

m2m given argument
In m2m post_generation: (True, 'blarg', 3), {}
value of m2m: ['blarg', 'blarg', 'blarg']

m2m given argument and empty trait active
value of m2m: []

And here's the output of this script with version 2.8.1:

no args
In m2m post_generation: (True, None, 3), {}
value of m2m: [None, None, None]

m2m given argument
In m2m post_generation: (True, 'blarg', 3), {}
value of m2m: ['blarg', 'blarg', 'blarg']

m2m given argument and empty trait active
In m2m post_generation: (True, 'blarg', 1), {}
value of m2m: ['blarg']

IMO the functionality and output of 2.8.1 is correct: the post-generate method is run, and parameters passed to the factory constructor override parameters defined in the trait.

Thanks, and I hope this helps!

[timc13]

i can confirm the bug @dlobue reported. Seems like a very nasty side effect of using Traits - essentially rendering any PostGeneration declarations useless if they are used in a trait.

[BenVosper]

I've also just noticed the bug mentioned above. Kwargs defined inside traits don't seem to be getting passed to any post-generation methods as they would if they were used to instantiate the factory.

Here's an even more stripped-back example:

import factory as factory_boy


class Foo:
    def __init__(self, bar):
        self.bar = bar


class FooFactory(factory_boy.Factory):

    class Meta:
        model = Foo

    bar = 1

    class Params:
        bar_trait = factory_boy.Trait(
            bar_post_generation=2
        )

    @factory_boy.post_generation
    def bar_post_generation(self, create, extracted, **kwargs):
        if create and extracted:
            self.bar = extracted


# Using default set in factory definition
foo = FooFactory()
assert foo.bar == 1

# Overriding value in factory definition
foo = FooFactory(bar=2)
assert foo.bar == 2

# Passing value directly to post-generation
foo = FooFactory(bar_post_generation=2)
assert foo.bar == 2

# Passing value to post-generation via trait
# Fails on 2.12.0
foo = FooFactory(bar_trait=True)
assert foo.bar == 2

Everything works up until that last assertion.

Other than this (admittedly niche) issue, I love Factory Boy! I use it whenever I can. Thanks for your hard work maintainers :)

[stevelacey]

Yep just stumbled over this problem myself, I expected traits to be an extension of passing params, and for post generation to work same as if I'd supplied all of the params in the test rather than via the trait.

I also tried to passing LazyAttribute and LazyFunction through the trait params based on the assumption that they'd be resolved then passed to the post generation, which might be asking a lot of the whole thing but that's what I tried.