pytest-dev / pytest-factoryboy

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

pytest-factoryboy seems to break factory.django.mute_signals #156

Open justin-f-perez opened 2 years ago

justin-f-perez commented 2 years ago

Work around

Decorate the class with mute_signals, and additionally override _after_postgeneration and mute signals there too.

@mute_signals(signals.pre_save, signals.post_save)
@register(_name="subscription_no_signals")
class SubscriptionNoSignalsFactory(SubscriptionFactory):
    @classmethod
    def _after_postgeneration(cls, *args, **kwargs):
        with mute_signals(signals.pre_save, signals.post_save):
            super()._after_postgeneration(*args, **kwargs)

Problem description

I have the following factory definition using the mute_signals decorator:

@register(_name="subscription_no_signals")
@mute_signals(signals.pre_save, signals.post_save)
class SubscriptionNoSignalsFactory(SubscriptionFactory):
    @classmethod
    def _create(cls, *args, **kwargs):
        print("what the &$#*")
        # super().create(*args, **kwargs)
        from core.models import Subscription

        print(f"{signals.post_save._live_receivers(Subscription)}")
        print(f"{signals.pre_save._live_receivers(Subscription)}")

        return super()._create(*args, **kwargs)

NOTE: I also attempted changing the order of the decorators.

And the following unit test:

@pytest.mark.parametrize("subscription_no_signals__complete", [False])
def test_subscription_too_old_to_mark_complete(subscription_no_signals):
    subscription = subscription_no_signals
    assert len(mail.outbox) == 1
    email = mail.outbox[0]
    assert subscription.pk in email.subject
    assert subscription.pk in email.body
    assert "too old" in email.body

The test fails with an error as a result of signals being executed when they shouldn't be. The following output is captured from the print statements in _create. The empty lists being printed show that at the time of _create, no signals are connected. The "unique" message ("what the...") ensures this output is not coming from anywhere else.:

>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>> captured stdout >>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>>
what the &$#*
[]
[]

I also poked around in pdb and found that pytest-factoryboy is saving the model with signals connected.

###########################################
# Signals are connected now. Why? They should be disconnected.
###########################################
> /Users/me/dev/my-project/754-get-rid-of-hardcoded-usernames/core/signals.py(106)subscription_changed()
(Pdb) from django.db.models import signals
(Pdb) from core.models import Subscription
(Pdb) signals.post_save._live_receivers(Subscription)
[<bound method HistoricalRecords.post_save of <simple_history.models.HistoricalRecords object at 0x123047490>>, <function subscription_changed at 0x1314039d0>, <function redcap_subscription_changed at 0x1314099d0>]
> /Users/me/dev/my-project/754-get-rid-of-hardcoded-usernames/core/signals.py(106)subscription_changed()
-> instance.check_complete()
(Pdb) up
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/django/dispatch/dispatcher.py(174)<listcomp>()
-> (receiver, receiver(signal=self, sender=sender, **named))
(Pdb) up
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/django/dispatch/dispatcher.py(173)send()
-> return [
(Pdb) up
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/django/db/models/base.py(791)save_base()
-> post_save.send(
(Pdb) up
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/django/db/models/base.py(743)save()
-> self.save_base(using=using, force_insert=force_insert,
(Pdb) up

##############################
# NOTE: We are using the "NoSignals" factory
##############################
-> factory._after_postgeneration(obj, create=True, results=results)
(Pdb) factory
<class 'conftest.SubscriptionNoSignalsFactory'>

> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/factory/django.py(192)_after_postgeneration()
-> instance.save()
(Pdb) up
###################################
# pytest-factoryboy seems to be the culprit
###################################
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/pytest_factoryboy/plugin.py(89)after_postgeneration()
-> factory._after_postgeneration(obj, create=True, results=results)
(Pdb) up
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/pytest_factoryboy/plugin.py(104)evaluate()
-> self.after_postgeneration(request)
(Pdb) up
> /opt/homebrew/Caskroom/miniconda/base/envs/my-project-dev/lib/python3.8/site-packages/pytest_factoryboy/fixture.py(320)model_fixture()
-> factoryboy_request.evaluate(request)
(Pdb) request
<SubRequest 'subscription_no_signals' for <Function test_subscription_too_old_to_mark_complete[study__end_date0-study__start_date0-subscription_no_signals__start_date0-subscription_no_signals__end_date0-False]>>

So it seems mute_signals is broken by pytest-factoryboy. Are there any workarounds?

justin-f-perez commented 2 years ago

~Nevermind. In fact, the issue was not with mute_signals at all. After defining a custom fixture to invoke my factories manually, I realized the study decorators should have been subscription_no_signals__subject__study (i.e., I had related models I was trying to pass parameters to.)~

~Factoryboy made the problem clear by throwing an exception; I'm unsure why the same doesn't occur for pytest-factoryboy, perhaps it's silently suppressing the error, or perhaps it just passed those parameters along to the related model factory despite that related model not being requested. It's still unclear why pytest-factoryboy unmuted the signals I explicitly muted. In any case, I'm closing this issue since it no longer affects me, but feel free to re-open it if you'd like to request any further details about my test setup.~

Nvm again. It seems I forgot to save after switching back to the pytest-factoryboy fixture and assumed the most recent output from pytest-watch was using this fixture. Now pytest-factoryboy just complains

In test_subscription_too_old_to_mark_complete: function uses no argument 'subscription_no_signalssubjectstudy__end_date'

The SubscriptionFactory that SubscriptionNoSignalsFactory inherits from does indeed have a subject subfactory, and the SubjectFactory does have a study subfactory with an end_date field. The test looks like this when receiving this error:

@pytest.mark.parametrize("subscription_no_signals__complete", [False])
@pytest.mark.parametrize("subscription_no_signals__end_date", [days_ago(100)])
@pytest.mark.parametrize("subscription_no_signals__start_date", [days_ago(107)])
@pytest.mark.parametrize("subscription_no_signals__subject__study__start_date", [days_ago(110)])
@pytest.mark.parametrize("subscription_no_signals__subject__study__end_date", [days_ago(-10)])
def test_subscription_too_old_to_mark_complete(subscription_no_signals):
    # def test_subscription_too_old_to_mark_complete(old_incomplete_subscription):
    subscription = subscription_no_signals
    # subscription = old_incomplete_subscription
    assert len(mail.outbox) == 1
    email = mail.outbox[0]
    assert subscription.pk in email.subject
    assert subscription.pk in email.body
    assert "too old" in email.body

I had assumed from the Book/Author example in the docs that I should just pass the name of the registered subfactory fixture (i.e., study__... as shown in the issue description), and based on this error I believe that is the case. The same error is thrown if I attempt to parametrize by the intermediate subfactory (i.e., subject__study...)

justin-f-perez commented 2 years ago

After digging around the code some more I was able to come up with a solution, but I'm leaving this issue open because this looks like a bug. _after_post_generation is invoked twice. The first time, none of the signal receivers are connected. The second time, they're all connected.

My factory (with debug code; cleaned version in issue description) now looks like this:

@mute_signals(signals.pre_save, signals.post_save)
@register(_name="subscription_no_signals")
class SubscriptionNoSignalsFactory(SubscriptionFactory):
    @classmethod
    def _create(cls, *args, **kwargs):
        print("what the &$#*")
        # super().create(*args, **kwargs)

        print(f"{signals.post_save._live_receivers(Subscription)}")
        print(f"{signals.pre_save._live_receivers(Subscription)}")

        return super()._create(*args, **kwargs)

    @classmethod
    def _after_postgeneration(cls, *args, **kwargs):
        print("What the two")
        print(f"{signals.post_save._live_receivers(Subscription)}")
        print(f"{signals.pre_save._live_receivers(Subscription)}")
        with mute_signals(signals.pre_save, signals.post_save):
            super()._after_postgeneration(*args, **kwargs)

I had to override _after_postgeneration and use mute_signals as a context manager while invoking super's _after_postgeneration to get the correct behavior. Notably, the decorator on the class is still required; otherwise signals will be connected during _create invocation.

-------------------------------- Captured stdout setup --------------------------------
what the &$#*
[]
[]
What the two
[]
[]
What the two
[<signal receiver1>, ... <signal receiverN>]
[<signal receiver1>, ... <signal receiverN>]

I thought perhaps _after_postgeneration is invoked the second time due to the subfactory, but I don't believe this is the case. I attempted adding additional subfactory parameterizations (both for the "through" model factory and for a separate relation that study does not pass through) and did not see extra invocations; I also commented out the subfactory parametrization and the two _after_postgeneration calls remained present.

youtux commented 2 years ago

it seems that there are 2 bugs here. The first one is that pytest-factoryboy invokes _after_postgeneration twice. The second one is that we invoke _after_postgeneration when we are not in the context of mute_signals.

The first one should not be too difficult to fix, I'm not sure about the second one though. I'll try when I have some time.

Indeed decorating the _after_postgeneration with mute_signals should address bug # 2.

Thank you for the bug report.

youtux commented 1 year ago

hi, is this still an issue?