mirror of https://github.com/zulip/zulip.git
email_backends: Fix concurrent backend testing for generate_emails.
Previously, this command would reliably fail: ``` tools/test-backend --skip-provision-check --parallel=3 zerver.tests.test_email_log.EmailLogTest.test_forward_address_details zerver.tests.test_email_log.EmailLogTest.test_generate_and_clear_email_log zerver.tests.test_example.TestDevelopmentEmailsLog ``` and now it reliably succeeds. :-) After hours of fiddling/googling/hair-tearing, I found that mocking-away Django Connection.send_messages() was the best: - We're testing Zulip and not Django. - Mocking at this lower level exercises more of our code. - EmailLogBackEnd._do_send_messages() helper method added to simplify mocking. Fixes #21925.
This commit is contained in:
parent
e9ba9b0e0d
commit
dbd03b5054
|
@ -10,7 +10,7 @@ from zproject.email_backends import get_forward_address
|
||||||
class EmailLogTest(ZulipTestCase):
|
class EmailLogTest(ZulipTestCase):
|
||||||
def test_generate_and_clear_email_log(self) -> None:
|
def test_generate_and_clear_email_log(self) -> None:
|
||||||
with self.settings(EMAIL_BACKEND="zproject.email_backends.EmailLogBackEnd"), mock.patch(
|
with self.settings(EMAIL_BACKEND="zproject.email_backends.EmailLogBackEnd"), mock.patch(
|
||||||
"zproject.email_backends.EmailBackend.send_messages"
|
"zproject.email_backends.EmailLogBackEnd._do_send_messages", lambda *args: 1
|
||||||
), self.assertLogs(level="INFO") as m, self.settings(DEVELOPMENT_LOG_EMAILS=True):
|
), self.assertLogs(level="INFO") as m, self.settings(DEVELOPMENT_LOG_EMAILS=True):
|
||||||
result = self.client_get("/emails/generate/")
|
result = self.client_get("/emails/generate/")
|
||||||
self.assertEqual(result.status_code, 302)
|
self.assertEqual(result.status_code, 302)
|
||||||
|
@ -35,8 +35,9 @@ class EmailLogTest(ZulipTestCase):
|
||||||
|
|
||||||
self.assertEqual(get_forward_address(), forward_address)
|
self.assertEqual(get_forward_address(), forward_address)
|
||||||
|
|
||||||
with self.settings(EMAIL_BACKEND="zproject.email_backends.EmailLogBackEnd"):
|
with self.settings(EMAIL_BACKEND="zproject.email_backends.EmailLogBackEnd"), mock.patch(
|
||||||
with mock.patch("zproject.email_backends.EmailBackend.send_messages"):
|
"zproject.email_backends.EmailLogBackEnd._do_send_messages", lambda *args: 1
|
||||||
|
):
|
||||||
result = self.client_get("/emails/generate/")
|
result = self.client_get("/emails/generate/")
|
||||||
self.assertEqual(result.status_code, 302)
|
self.assertEqual(result.status_code, 302)
|
||||||
self.assertIn("emails", result["Location"])
|
self.assertIn("emails", result["Location"])
|
||||||
|
|
|
@ -401,8 +401,9 @@ class TestDevelopmentEmailsLog(ZulipTestCase):
|
||||||
# https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertLogs
|
# https://docs.python.org/3/library/unittest.html#unittest.TestCase.assertLogs
|
||||||
with self.settings(EMAIL_BACKEND="zproject.email_backends.EmailLogBackEnd"), self.settings(
|
with self.settings(EMAIL_BACKEND="zproject.email_backends.EmailLogBackEnd"), self.settings(
|
||||||
DEVELOPMENT_LOG_EMAILS=True
|
DEVELOPMENT_LOG_EMAILS=True
|
||||||
), self.assertLogs(level="INFO") as logger:
|
), self.assertLogs(level="INFO") as logger, mock.patch(
|
||||||
|
"zproject.email_backends.EmailLogBackEnd._do_send_messages", lambda *args: 1
|
||||||
|
):
|
||||||
result = self.client_get(
|
result = self.client_get(
|
||||||
"/emails/generate/"
|
"/emails/generate/"
|
||||||
) # Generates emails and redirects to /emails/
|
) # Generates emails and redirects to /emails/
|
||||||
|
|
|
@ -82,11 +82,19 @@ class EmailLogBackEnd(EmailBackend):
|
||||||
|
|
||||||
email_message.to = [get_forward_address()]
|
email_message.to = [get_forward_address()]
|
||||||
|
|
||||||
|
# This wrapper function exists to allow tests easily to mock the
|
||||||
|
# step of trying to send the emails. Previously, we had mocked
|
||||||
|
# Django's connection.send_messages(), which caused unexplained
|
||||||
|
# test failures when running test-backend at very high
|
||||||
|
# concurrency.
|
||||||
|
def _do_send_messages(self, email_messages: List[EmailMultiAlternatives]) -> int:
|
||||||
|
return super().send_messages(email_messages) # nocoverage
|
||||||
|
|
||||||
def send_messages(self, email_messages: List[EmailMultiAlternatives]) -> int:
|
def send_messages(self, email_messages: List[EmailMultiAlternatives]) -> int:
|
||||||
num_sent = len(email_messages)
|
num_sent = len(email_messages)
|
||||||
if get_forward_address():
|
if get_forward_address():
|
||||||
self.prepare_email_messages_for_forwarding(email_messages)
|
self.prepare_email_messages_for_forwarding(email_messages)
|
||||||
num_sent = super().send_messages(email_messages)
|
num_sent = self._do_send_messages(email_messages)
|
||||||
|
|
||||||
if settings.DEVELOPMENT_LOG_EMAILS:
|
if settings.DEVELOPMENT_LOG_EMAILS:
|
||||||
for email in email_messages:
|
for email in email_messages:
|
||||||
|
|
Loading…
Reference in New Issue