Fix AttributeError: 'SendCampaignThread' object has no attribute 'campaign'

[danhayden]

addresses https://github.com/neon-jungle/wagtail-birdsong/issues/26

[danhayden]

I'd like to add a test for handling an SMTPException however I'm new to writing Python/Django tests and do not know how to trigger an SMTPException such as a 429 HTTP response within a test.

Would you be able to offer some guidance?

Use the existing tests as a reference and base so far I have the following:

class TestErrorHandling(WagtailTestUtils, TransactionTestCase):
    def setUp(self):
        self.campaign = SaleCampaign.objects.create(
            name='Test campaign',
            subject='The subject',
            body=[('rich_text', RichText('<p>The body</p>'))]
        )
        for person in [
            ('Terry', 'Testington', 'North', 'terry@tests.com'),
            ('Wag', 'Tail', 'South', 'wag@tail.com'),
        ]:
            ExtendedContact.objects.create(
                first_name=person[0],
                last_name=person[1],
                location=person[2],
                email=person[3],
            )
        self.login()

    def test_smtp_exception(self):
        self.client.get(f'/admin/app/salecampaign/send_campaign/{self.campaign.id}/')

        sleep(10)  # Allow time  to send
        self.assertEquals(len(mail.outbox), 0)
        self.assertEquals(self.campaign.receipts.all().count(), 0)
        # Get fresh from db (altered in a thread)
        fresh_campaign = SaleCampaign.objects.get(pk=self.campaign.pk)
        self.assertEquals(fresh_campaign.status, CampaignStatus.FAILED)

But this fails as no SMTPException occurs

Found 7 test(s).
Creating test database for alias 'default'...
System check identified no issues (0 silenced).
....F..
======================================================================
FAIL: test_smtp_exception (tests.test_admin.TestErrorHandling)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/srv/tests/test_admin.py", line 134, in test_smtp_exception
    self.assertEquals(len(mail.outbox), 0)
AssertionError: 2 != 0

----------------------------------------------------------------------
Ran 7 tests in 38.539s

FAILED (failures=1)
Destroying test database for alias 'default'...

[david-smejkal]

What you want to do is to simulate the situation where the code throws such an exception in your test by supplying specific test data and calling core birdsong functions. That's because you want to figure out whether the exception is an acceptable situation or whether it's something that should be remedied in some way.

Now to answer your question: You can "throw" an exception forcefully with the raise keyword and then test for it as follows:

from smtplib import SMTPException
...
def something_that_i_am_testing_that_raises_smtp_exception():
    raise SMTPException
...
class TestErrorHandling(WagtailTestUtils, TransactionTestCase):
    ...
    def test_smtp_exception(self):
        ...
        with self.assertRaises(SMTPException):
            something_that_i_am_testing_that_raises_smtp_exception()

@danhayden, here is an example where you can see testing for an expected exception in practice: https://github.com/neon-jungle/wagtail-birdsong/pull/36/files#diff-041ae52229a7d4829a12f2c872c197a22146eaee3c02ea522d940d6fb886998fR23-R24

However, as said in the begining. Don't just raise an exception. Simulate it with test data and core birdsong function calls.

[danhayden]

@david-smejkal thanks for your feedback.

I haven't written tests for Wagtail or Django before so still not sure how to approach this. The original error that caused me to find this bug was due to errors (SMTPRecipientsRefused & SMTPSenderRefused) coming from a third-party (Google SMTP) - I have no idea currently how I would simulate that with test data and core birdsong function calls.

If you have any further advise or recommendations it would be appreciated.

[danhayden]

Still no tests for this but I don't think that should prevent this small fix being merged. Let me know if any changes are required.