diff --git a/lms/djangoapps/bulk_email/tests/fake_smtp.py b/lms/djangoapps/bulk_email/tests/fake_smtp.py deleted file mode 100755 index b6fb9b92a5..0000000000 --- a/lms/djangoapps/bulk_email/tests/fake_smtp.py +++ /dev/null @@ -1,88 +0,0 @@ -""" -Fake SMTP Server used for testing error handling for sending email. -We could have mocked smptlib to raise connection errors, but this simulates -connection errors from an SMTP server. -""" -import smtpd -import socket -import asyncore -import asynchat -import errno - - -class FakeSMTPChannel(smtpd.SMTPChannel): - """ - A fake SMTPChannel for sending fake error response through socket. - This causes smptlib to raise an SMTPConnectError. - - Adapted from http://hg.python.org/cpython/file/2.7/Lib/smtpd.py - """ - # Disable pylint warnings that arise from subclassing SMTPChannel - # and calling init -- overriding SMTPChannel's init to return error - # message but keeping the rest of the class. - # pylint: disable=W0231, W0233 - def __init__(self, server, conn, addr): - asynchat.async_chat.__init__(self, conn) - self.__server = server - self.__conn = conn - self.__addr = addr - self.__line = [] - self.__state = self.COMMAND - self.__greeting = 0 - self.__mailfrom = None - self.__rcpttos = [] - self.__data = '' - self.__fqdn = socket.getfqdn() - try: - self.__peer = conn.getpeername() - except socket.error, err: - # a race condition may occur if the other end is closing - # before we can get the peername - self.close() - if err[0] != errno.ENOTCONN: - raise - return - self.push('421 SMTP Server error: too many concurrent sessions, please try again later.') - self.set_terminator('\r\n') - - -class FakeSMTPServer(smtpd.SMTPServer): - """A fake SMTP server for generating different smptlib exceptions.""" - def __init__(self, *args, **kwargs): - smtpd.SMTPServer.__init__(self, *args, **kwargs) - self.errtype = None - self.response = None - - def set_errtype(self, errtype, response=''): - """Specify the type of error to cause smptlib to raise, with optional response string. - - `errtype` -- "DATA": The server will cause smptlib to throw SMTPDataError. - "CONN": The server will cause smptlib to throw SMTPConnectError. - "DISCONN": The server will cause smptlib to throw SMTPServerDisconnected. - - """ - self.errtype = errtype - self.response = response - - def handle_accept(self): - if self.errtype == "DISCONN": - self.accept() - elif self.errtype == "CONN": - pair = self.accept() - if pair is not None: - conn, addr = pair - _channel = FakeSMTPChannel(self, conn, addr) - else: - smtpd.SMTPServer.handle_accept(self) - - def process_message(self, *_args, **_kwargs): - if self.errtype == "DATA": - # After failing on the first email, succeed on the rest. - self.errtype = None - return self.response - else: - return None - - def serve_forever(self): - """Start the server running until close() is called on the server.""" - asyncore.loop() diff --git a/lms/djangoapps/bulk_email/tests/smtp_server_thread.py b/lms/djangoapps/bulk_email/tests/smtp_server_thread.py deleted file mode 100644 index 713cd9ca64..0000000000 --- a/lms/djangoapps/bulk_email/tests/smtp_server_thread.py +++ /dev/null @@ -1,47 +0,0 @@ -""" -Defines a class for a thread that runs a Fake SMTP server, used for testing -error handling from sending email. -""" -import threading -from bulk_email.tests.fake_smtp import FakeSMTPServer - - -class FakeSMTPServerThread(threading.Thread): - """ - Thread for running a fake SMTP server - """ - def __init__(self, host, port): - self.host = host - self.port = port - self.is_ready = threading.Event() - self.error = None - self.server = None - super(FakeSMTPServerThread, self).__init__() - - def start(self): - self.daemon = True - super(FakeSMTPServerThread, self).start() - self.is_ready.wait() - if self.error: - raise self.error # pylint: disable=E0702 - - def stop(self): - """ - Stop the thread by closing the server instance. - Wait for the server thread to terminate. - """ - if hasattr(self, 'server'): - self.server.close() - self.join() - - def run(self): - """ - Sets up the test smtp server and handle requests. - """ - try: - self.server = FakeSMTPServer((self.host, self.port), None) - self.is_ready.set() - self.server.serve_forever() - except Exception, exc: # pylint: disable=W0703 - self.error = exc - self.is_ready.set() diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index 61bdd315e9..e0fd222c45 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -1,7 +1,7 @@ """ Unit tests for handling email sending errors """ - +from itertools import cycle from django.test.utils import override_settings from django.conf import settings from django.core.management import call_command @@ -14,24 +14,16 @@ from student.tests.factories import UserFactory, AdminFactory, CourseEnrollmentF from bulk_email.models import CourseEmail from bulk_email.tasks import delegate_email_batches -from bulk_email.tests.smtp_server_thread import FakeSMTPServerThread from mock import patch, Mock from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError -TEST_SMTP_PORT = 1025 - class EmailTestException(Exception): pass -@override_settings( - MODULESTORE=TEST_DATA_MONGO_MODULESTORE, - EMAIL_BACKEND='django.core.mail.backends.smtp.EmailBackend', - EMAIL_HOST='localhost', - EMAIL_PORT=TEST_SMTP_PORT -) +@override_settings(MODULESTORE=TEST_DATA_MONGO_MODULESTORE) class TestEmailErrors(ModuleStoreTestCase): """ Test that errors from sending email are handled properly. @@ -44,26 +36,18 @@ class TestEmailErrors(ModuleStoreTestCase): # load initial content (since we don't run migrations as part of tests): call_command("loaddata", "course_email_template.json") - - self.smtp_server_thread = FakeSMTPServerThread('localhost', TEST_SMTP_PORT) - self.smtp_server_thread.start() - self.url = reverse('instructor_dashboard', kwargs={'course_id': self.course.id}) def tearDown(self): - self.smtp_server_thread.stop() patch.stopall() + @patch('bulk_email.tasks.get_connection', autospec=True) @patch('bulk_email.tasks.course_email.retry') - def test_data_err_retry(self, retry): + def test_data_err_retry(self, retry, get_conn): """ Test that celery handles transient SMTPDataErrors by retrying. """ - self.smtp_server_thread.server.set_errtype( - "DATA", - "454 Throttling failure: Daily message quota exceeded." - ) - + get_conn.return_value.send_messages.side_effect = SMTPDataError(455, "Throttling: Sending rate exceeded") test_email = { 'action': 'Send email', 'to_option': 'myself', @@ -78,17 +62,15 @@ class TestEmailErrors(ModuleStoreTestCase): exc = kwargs['exc'] self.assertTrue(type(exc) == SMTPDataError) + @patch('bulk_email.tasks.get_connection', autospec=True) @patch('bulk_email.tasks.course_email_result') @patch('bulk_email.tasks.course_email.retry') - def test_data_err_fail(self, retry, result): + def test_data_err_fail(self, retry, result, get_conn): """ Test that celery handles permanent SMTPDataErrors by failing and not retrying. """ - self.smtp_server_thread.server.set_errtype( - "DATA", - "554 Message rejected: Email address is not verified." - ) - + get_conn.return_value.send_messages.side_effect = cycle([SMTPDataError(554, "Email address is blacklisted"), + None]) students = [UserFactory() for _ in xrange(settings.EMAILS_PER_TASK)] for student in students: CourseEnrollmentFactory.create(user=student, course_id=self.course.id) @@ -106,18 +88,16 @@ class TestEmailErrors(ModuleStoreTestCase): # Test that after the rejected email, the rest still successfully send ((sent, fail, optouts), _) = result.call_args self.assertEquals(optouts, 0) - self.assertEquals(fail, 1) - self.assertEquals(sent, settings.EMAILS_PER_TASK - 1) + self.assertEquals(fail, settings.EMAILS_PER_TASK / 2) + self.assertEquals(sent, settings.EMAILS_PER_TASK / 2) + @patch('bulk_email.tasks.get_connection', autospec=True) @patch('bulk_email.tasks.course_email.retry') - def test_disconn_err_retry(self, retry): + def test_disconn_err_retry(self, retry, get_conn): """ Test that celery handles SMTPServerDisconnected by retrying. """ - self.smtp_server_thread.server.set_errtype( - "DISCONN", - "Server disconnected, please try again later." - ) + get_conn.return_value.open.side_effect = SMTPServerDisconnected(425, "Disconnecting") test_email = { 'action': 'Send email', 'to_option': 'myself', @@ -131,13 +111,13 @@ class TestEmailErrors(ModuleStoreTestCase): exc = kwargs['exc'] self.assertTrue(type(exc) == SMTPServerDisconnected) + @patch('bulk_email.tasks.get_connection', autospec=True) @patch('bulk_email.tasks.course_email.retry') - def test_conn_err_retry(self, retry): + def test_conn_err_retry(self, retry, get_conn): """ Test that celery handles SMTPConnectError by retrying. """ - # SMTP reply is already specified in fake SMTP Channel created - self.smtp_server_thread.server.set_errtype("CONN") + get_conn.return_value.open.side_effect = SMTPConnectError(424, "Bad Connection") test_email = { 'action': 'Send email',