FactoryBoy / factory_boy

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

Set params with trait #550

Open timdiels opened 5 years ago

timdiels commented 5 years ago

The problem

Setting params with a trait seems to be unsupported currently. Not sure whether to consider this a bug but the docs on traits refer to 'fields', not 'params', so I opted with not-a-bug.

E.g.

from django.db import models
from factory import (
    SubFactory, SelfAttribute, LazyAttribute, DjangoModelFactory, Trait
)

class LegacyFoo(models.Model):
    bar_id = models.IntegerField(null=True)

class Bar(models.Model):
    pass

class BarFactory(DjangoModelFactory):
    class Meta:
        model = Bar

class LegacyFooFactory(DjangoModelFactory):
    class Meta:
        model = LegacyFoo

    class Params:
        bar = None
        has_bar = Trait(
            bar = SubFactory(BarFactory),
        )

    bar_id = LazyAttribute(lambda o: o.bar.id if o.bar else None)

def test_one():
    foo = LegacyFooFactory.build()
    assert foo.bar_id is None
    foo = LegacyFooFactory.build(has_bar=True)
    assert foo.bar_id is not None

Fails the second assert.

And when changing it to:

class LegacyFooFactory(DjangoModelFactory):
    class Meta:
        model = LegacyFoo

    class Params:
        bar = None
        has_bar = Trait(
            bar = SubFactory(BarFactory),
            bar_id = SelfAttribute('bar.id'),
        )

    bar_id = None

This results in:

test_various.py:44: in test_one
    foo = LegacyFooFactory.build(has_bar=True)
python3.6/site-packages/factory/base.py:546: in build
    return cls._generate(enums.BUILD_STRATEGY, kwargs)
python3.6/site-packages/factory/base.py:500: in _generate
    return step.build()
python3.6/site-packages/factory/builder.py:272: in build
    step.resolve(pre)
python3.6/site-packages/factory/builder.py:221: in resolve
    self.attributes[field_name] = getattr(self.stub, field_name)
python3.6/site-packages/factory/builder.py:375: in __getattr__
    extra=context,
python3.6/site-packages/factory/declarations.py:502: in evaluate
    extra=extra,
python3.6/site-packages/factory/declarations.py:159: in evaluate
    return deepgetattr(target, self.attribute_name, self.default)
python3.6/site-packages/factory/declarations.py:119: in deepgetattr
    return deepgetattr(getattr(obj, attr), subname, default)
python3.6/site-packages/factory/declarations.py:121: in deepgetattr
    return getattr(obj, name)
E   AttributeError: 'NoneType' object has no attribute 'id'

This can be worked around with an exclude though:

class LegacyFooFactory(DjangoModelFactory):
    class Meta:
        model = models.LegacyFoo
        exclude = ('bar',)

    class Params:
        has_bar = Trait(
            bar = SubFactory(BarFactory),
            bar_id = SelfAttribute('bar.id'),
        )

    bar = None
    bar_id = None

Proposed solution

If not supporting params, it may help to add a note in the reference and introduction that traits cannot assign to params, only to fields. Else, add support for it; I'm not familiar with how factory_boy is implemented :)

Extra notes

Supporting assignment to params may allow one to forget about exclude (see #549)

brechin commented 3 years ago

What about using Django's FK field to express the relationship between the two (which internally will use a bar_id field)?

class Bar(models.Model):
    pass

class LegacyFoo(models.Model):
    bar = models.ForeignKey(Bar, null=True, on_delete=models.CASCADE)

class BarFactory(DjangoModelFactory):
    class Meta:
        model = Bar

class LegacyFooFactory(DjangoModelFactory):
    class Meta:
        model = LegacyFoo

    class Params:
        has_bar = Trait(
            bar = SubFactory(BarFactory),
        )
timdiels commented 3 years ago

First, I think I got the naming backwards. bar_id was supposed to be a reference to the legacy row. You can mentally substitute LegacyFoo for UserV2 and Bar for UserV1.

The legacy tables are in a different DB, at the time it was on a different server even. So it is not possible to point an FK to it, or at least in MySQL it wasn't, switched to Postgres in the meantime. The use case is a legacy DB being migrated to a new DB; the migration is tested with these factories. The tests do put all tables in the same DB though.

FKs would be possible though when first copying the legacy tables to the new DB without making any changes yet. I prefer my own workaround to that though as it avoids the extra copy. I'll be able to get rid of these legacy/v1 factories after the migration, but thought I'd mention this problem in case someone else comes across it in the future.