pytest-dev / pytest-factoryboy

factory_boy integration the pytest runner
MIT License
359 stars 42 forks source link

factory.SubFactory fixture collision when the model has the same name #218

Open mcosti opened 4 months ago

mcosti commented 4 months ago

Hi everyone! (Long time no talk)

I just came into the following situation that I can't seem to be able to untangle.

I have a "Product" model that is part of our core business logic, and a "Product" model coming in from Dj-stripe

When trying to set up the stripe models and relationships via the factories, I am running into a name collision


import factory
from djstripe.enums import SubscriptionStatus
from factory.django import DjangoModelFactory
from djstripe.models import (
    Product as SubscriptionProduct,
    Plan as SubscriptionPlan,
    Customer as SubscriptionCustomer,
    Price as SubscriptionPrice,
    Subscription, SubscriptionItem
)

class SubscriptionProductFactory(DjangoModelFactory):
    class Meta:
        model = SubscriptionProduct

    name = "Pro"

    @classmethod
    def _create(cls, model_class, *args, **kwargs):
        product = super()._create(model_class, *args, **kwargs)
        return product

class SubscriptionPriceFactory(DjangoModelFactory):
    class Meta:
        model = SubscriptionPrice

    active = True
    currency = "EUR"
    unit_amount = 1000
    product_test = factory.SubFactory("factories.stripe.SubscriptionProductFactory")
    lookup_key = "pro_monthly"

    @classmethod
    def _create(cls, model_class, *args, **kwargs):
        price = super()._create(model_class, *args, **kwargs)
        return price

But when the SubscriptionProductFactory is used as a SubFactory, the inner factoryboy code is evaluating it's model.name

image

Which returns the fixture of the other Product model.

Since the original Product class is used everywhere, refactoring is not an option, and the model coming in from the external package is not possible to change.

Is there any solution for this?

Thanks

youtux commented 4 months ago

You have a few options:

- register the parent fixture passing the correct fixture to use for `product_test`:
```python
register(
    SubscriptionPriceFactory,
    product_test=LazyFixture("subscription_product)
)
mcosti commented 4 months ago

Thank you for taking the time!

The first variant does not work. (the arguments are reversed, the class is first, but I did that and it didn't work anyway)

backend-tests-1  | ../../../factories/stripe.py:69: in <module>
backend-tests-1  |     class SubscriptionProductFactory(StripeBaseFactory):
backend-tests-1  | ../../../factories/stripe.py:70: in SubscriptionProductFactory
backend-tests-1  |     class Meta:
backend-tests-1  | ../../../factories/stripe.py:71: in Meta
backend-tests-1  |     model = named_model(SubscriptionProduct, "subscription_product")
backend-tests-1  | /usr/local/lib/python3.11/dist-packages/pytest_factoryboy/fixture.py:75: in named_model
backend-tests-1  |     return type(name, (model_cls,), {})
backend-tests-1  | /usr/local/lib/python3.11/dist-packages/django/db/models/base.py:105: in __new__
backend-tests-1  |     module = attrs.pop("__module__")
backend-tests-1  | E   KeyError: '__module__'

But the second option works. What do you think of my fix at the factory definition level?

Thanks

youtux commented 4 months ago

maybe the SubscriptionProduct class is not a django model? In that case you have to use ModelFactoty instead of DjangoModelFactory.

About the PR, I don’t think it’s needed since we have already 2 ways of solving this. Maybe we should just improve the README.md to showcase this scenario.

mcosti commented 4 months ago

The problem extends even further since in this specific case, all of my "Stripe" models have a SubFactory pointing to an "Account" model (also coming from Stripe, and also a namespace conflict), so the solution for overriding at on register(...) doesn't sound very pleasant, unless I write a custom register method that appends account=LazyFixture("stripe_account") to each registration

mcosti commented 4 months ago

SubscriptionProduct is actually an import alias. The model itself is called "Product", which is where this issue stems from

youtux commented 4 months ago

You can always use the first option I suggested, it should work