From 3127ac2b6cbdb27e155fbfb151494b11fb9aee1d Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Mon, 5 Jan 2015 15:31:13 -0500 Subject: [PATCH 1/3] mock html_to_text for tests --- .../bulk_email/tests/test_course_optout.py | 3 ++- lms/djangoapps/bulk_email/tests/test_email.py | 17 ++++++++++++++++- .../bulk_email/tests/test_err_handling.py | 4 ++-- lms/djangoapps/bulk_email/tests/test_models.py | 4 ++-- lms/djangoapps/bulk_email/tests/test_tasks.py | 1 + lms/djangoapps/instructor/tests/test_api.py | 2 ++ .../instructor_task/tests/test_api.py | 2 ++ 7 files changed, 27 insertions(+), 6 deletions(-) diff --git a/lms/djangoapps/bulk_email/tests/test_course_optout.py b/lms/djangoapps/bulk_email/tests/test_course_optout.py index 88926561e0..da000f8592 100644 --- a/lms/djangoapps/bulk_email/tests/test_course_optout.py +++ b/lms/djangoapps/bulk_email/tests/test_course_optout.py @@ -3,7 +3,7 @@ Unit tests for student optouts from course email """ import json -from mock import patch +from mock import patch, Mock from django.core import mail from django.core.management import call_command @@ -18,6 +18,7 @@ from xmodule.modulestore.tests.django_utils import ModuleStoreTestCase from xmodule.modulestore.tests.factories import CourseFactory +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) class TestOptoutCourseEmails(ModuleStoreTestCase): diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index dad65d6e66..810c057acd 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -3,7 +3,9 @@ Unit tests for sending course email """ import json -from mock import patch +from mock import patch, Mock +import os +from unittest import skipIf from django.conf import settings from django.core import mail @@ -90,6 +92,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): """ patch.stopall() + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) def test_email_disabled(self): """ @@ -105,6 +108,8 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): # We should get back a HttpResponseForbidden (status code 403) self.assertContains(response, "Email is not enabled for this course.", status_code=403) + + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_send_to_self(self): """ Make sure email send to myself goes to myself. @@ -130,6 +135,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): '[' + self.course.display_name + ']' + ' test subject for myself' ) + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_send_to_staff(self): """ Make sure email send to staff and instructors goes there. @@ -153,6 +159,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): [self.instructor.email] + [s.email for s in self.staff] ) + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_send_to_all(self): """ Make sure email send to all goes there. @@ -176,6 +183,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students] ) + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_no_duplicate_emails_staff_instructor(self): """ Test that no duplicate emails are sent to a course instructor that is @@ -184,6 +192,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): CourseStaffRole(self.course.id).add_users(self.instructor) self.test_send_to_all() + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_no_duplicate_emails_enrolled_staff(self): """ Test that no duplicate emials are sent to a course instructor that is @@ -192,6 +201,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): CourseEnrollment.enroll(self.instructor, self.course.id) self.test_send_to_all() + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_unicode_subject_send_to_all(self): """ Make sure email (with Unicode characters) send to all goes there. @@ -220,9 +230,12 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): '[' + self.course.display_name + '] ' + uni_subject ) + @skipIf(os.environ.get("Travis")=='true', "Skip this test in Travis CI.") def test_unicode_message_send_to_all(self): """ Make sure email (with Unicode characters) send to all goes there. + Note that this test is skipped on Travis because we can't use the + function `html_to_text` as it is currently implemented on Travis. """ # Now we know we have pulled up the instructor dash's email view # (in the setUp method), we can test sending an email. @@ -247,6 +260,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): message_body = mail.outbox[0].body self.assertIn(uni_message, message_body) + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_unicode_students_send_to_all(self): """ Make sure email (with Unicode characters) send to all goes there. @@ -278,6 +292,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): @override_settings(BULK_EMAIL_EMAILS_PER_TASK=3) @patch('bulk_email.tasks.update_subtask_status') + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_chunked_queries_send_numerous_emails(self, email_mock): """ Test sending a large number of emails, to test the chunked querying diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index fc889f36aa..e3de3e8f50 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -11,7 +11,7 @@ from django.core.management import call_command from django.core.urlresolvers import reverse from django.db import DatabaseError import json -from mock import patch +from mock import patch, Mock from smtplib import SMTPDataError, SMTPServerDisconnected, SMTPConnectError from bulk_email.models import CourseEmail, SEND_TO_ALL @@ -36,7 +36,7 @@ class EmailTestException(Exception): """Mock exception for email testing.""" pass - +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) class TestEmailErrors(ModuleStoreTestCase): diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py index c7165d37e3..f004fcf799 100644 --- a/lms/djangoapps/bulk_email/tests/test_models.py +++ b/lms/djangoapps/bulk_email/tests/test_models.py @@ -7,12 +7,12 @@ from django.conf import settings from student.tests.factories import UserFactory -from mock import patch +from mock import patch, Mock from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization from opaque_keys.edx.locations import SlashSeparatedCourseKey - +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) class CourseEmailTest(TestCase): """Test the CourseEmail model.""" diff --git a/lms/djangoapps/bulk_email/tests/test_tasks.py b/lms/djangoapps/bulk_email/tests/test_tasks.py index d2b00e0327..759025335a 100644 --- a/lms/djangoapps/bulk_email/tests/test_tasks.py +++ b/lms/djangoapps/bulk_email/tests/test_tasks.py @@ -71,6 +71,7 @@ def my_update_subtask_status(entry_id, current_task_id, new_subtask_status): update_subtask_status(entry_id, current_task_id, new_subtask_status) +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) class TestBulkEmailInstructorTask(InstructorTaskCourseTestCase): """Tests instructor task that send bulk email.""" diff --git a/lms/djangoapps/instructor/tests/test_api.py b/lms/djangoapps/instructor/tests/test_api.py index 208a62c8d9..9b8bcc9c12 100644 --- a/lms/djangoapps/instructor/tests/test_api.py +++ b/lms/djangoapps/instructor/tests/test_api.py @@ -139,6 +139,7 @@ class TestCommonExceptions400(TestCase): @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) class TestInstructorAPIDenyLevels(ModuleStoreTestCase, LoginEnrollmentTestCase): """ @@ -2228,6 +2229,7 @@ class TestInstructorAPIRegradeTask(ModuleStoreTestCase, LoginEnrollmentTestCase) @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) class TestInstructorSendEmail(ModuleStoreTestCase, LoginEnrollmentTestCase): """ diff --git a/lms/djangoapps/instructor_task/tests/test_api.py b/lms/djangoapps/instructor_task/tests/test_api.py index 81a63527d4..b7ed397c2a 100644 --- a/lms/djangoapps/instructor_task/tests/test_api.py +++ b/lms/djangoapps/instructor_task/tests/test_api.py @@ -1,6 +1,7 @@ """ Test for LMS instructor background task queue management """ +from mock import patch, Mock from bulk_email.models import CourseEmail, SEND_TO_ALL from courseware.tests.factories import UserFactory from xmodule.modulestore.exceptions import ItemNotFoundError @@ -158,6 +159,7 @@ class InstructorTaskModuleSubmitTest(InstructorTaskModuleTestCase): self._test_submit_task(submit_delete_problem_state_for_all_students) +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) class InstructorTaskCourseSubmitTest(TestReportMixin, InstructorTaskCourseTestCase): """Tests API methods that involve the submission of course-based background tasks.""" From c7ede0df4bf949c7705c672bef4aad48aea8216a Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Mon, 5 Jan 2015 16:19:26 -0500 Subject: [PATCH 2/3] fixing pep8 violations --- lms/djangoapps/bulk_email/tests/test_email.py | 3 +-- lms/djangoapps/bulk_email/tests/test_err_handling.py | 1 + lms/djangoapps/bulk_email/tests/test_models.py | 1 + 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 810c057acd..5249fdf290 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -108,7 +108,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): # We should get back a HttpResponseForbidden (status code 403) self.assertContains(response, "Email is not enabled for this course.", status_code=403) - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_send_to_self(self): """ @@ -230,7 +229,7 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): '[' + self.course.display_name + '] ' + uni_subject ) - @skipIf(os.environ.get("Travis")=='true', "Skip this test in Travis CI.") + @skipIf(os.environ.get("Travis") == 'true', "Skip this test in Travis CI.") def test_unicode_message_send_to_all(self): """ Make sure email (with Unicode characters) send to all goes there. diff --git a/lms/djangoapps/bulk_email/tests/test_err_handling.py b/lms/djangoapps/bulk_email/tests/test_err_handling.py index e3de3e8f50..867183cb2b 100644 --- a/lms/djangoapps/bulk_email/tests/test_err_handling.py +++ b/lms/djangoapps/bulk_email/tests/test_err_handling.py @@ -36,6 +36,7 @@ class EmailTestException(Exception): """Mock exception for email testing.""" pass + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) @override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) diff --git a/lms/djangoapps/bulk_email/tests/test_models.py b/lms/djangoapps/bulk_email/tests/test_models.py index f004fcf799..4ee232ecfe 100644 --- a/lms/djangoapps/bulk_email/tests/test_models.py +++ b/lms/djangoapps/bulk_email/tests/test_models.py @@ -12,6 +12,7 @@ from mock import patch, Mock from bulk_email.models import CourseEmail, SEND_TO_STAFF, CourseEmailTemplate, CourseAuthorization from opaque_keys.edx.locations import SlashSeparatedCourseKey + @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) class CourseEmailTest(TestCase): """Test the CourseEmail model.""" From 9dc85bf10dbf7311d2ec8a3f10d931ac4a9d85ed Mon Sep 17 00:00:00 2001 From: Christine Lytwynec Date: Mon, 5 Jan 2015 17:38:18 -0500 Subject: [PATCH 3/3] PR updates --- lms/djangoapps/bulk_email/tests/test_email.py | 89 ++++++++++--------- 1 file changed, 48 insertions(+), 41 deletions(-) diff --git a/lms/djangoapps/bulk_email/tests/test_email.py b/lms/djangoapps/bulk_email/tests/test_email.py index 5249fdf290..2526c0c1c3 100644 --- a/lms/djangoapps/bulk_email/tests/test_email.py +++ b/lms/djangoapps/bulk_email/tests/test_email.py @@ -44,9 +44,7 @@ class MockCourseEmailResult(object): return mock_update_subtask_status -@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) -@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) -class TestEmailSendFromDashboard(ModuleStoreTestCase): +class EmailSendFromDashboardTestCase(ModuleStoreTestCase): """ Test that emails send correctly. """ @@ -92,7 +90,14 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): """ patch.stopall() - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) + +@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) +@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) +@patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) +class TestEmailSendFromDashboardMockedHtmlToText(EmailSendFromDashboardTestCase): + """ + Tests email sending with mocked html_to_text. + """ @patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': True}) def test_email_disabled(self): """ @@ -134,7 +139,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): '[' + self.course.display_name + ']' + ' test subject for myself' ) - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_send_to_staff(self): """ Make sure email send to staff and instructors goes there. @@ -158,7 +162,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): [self.instructor.email] + [s.email for s in self.staff] ) - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_send_to_all(self): """ Make sure email send to all goes there. @@ -182,7 +185,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students] ) - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_no_duplicate_emails_staff_instructor(self): """ Test that no duplicate emails are sent to a course instructor that is @@ -191,7 +193,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): CourseStaffRole(self.course.id).add_users(self.instructor) self.test_send_to_all() - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_no_duplicate_emails_enrolled_staff(self): """ Test that no duplicate emials are sent to a course instructor that is @@ -200,7 +201,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): CourseEnrollment.enroll(self.instructor, self.course.id) self.test_send_to_all() - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_unicode_subject_send_to_all(self): """ Make sure email (with Unicode characters) send to all goes there. @@ -229,37 +229,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): '[' + self.course.display_name + '] ' + uni_subject ) - @skipIf(os.environ.get("Travis") == 'true', "Skip this test in Travis CI.") - def test_unicode_message_send_to_all(self): - """ - Make sure email (with Unicode characters) send to all goes there. - Note that this test is skipped on Travis because we can't use the - function `html_to_text` as it is currently implemented on Travis. - """ - # Now we know we have pulled up the instructor dash's email view - # (in the setUp method), we can test sending an email. - - uni_message = u'ẗëṡẗ ṁëṡṡäġë ḟöṛ äḷḷ ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ fоѓ аll' - test_email = { - 'action': 'Send email', - 'send_to': 'all', - 'subject': 'test subject for all', - 'message': uni_message - } - # Post the email to the instructor dashboard API - response = self.client.post(self.send_mail_url, test_email) - self.assertEquals(json.loads(response.content), self.success_content) - - self.assertEquals(len(mail.outbox), 1 + len(self.staff) + len(self.students)) - self.assertItemsEqual( - [e.to[0] for e in mail.outbox], - [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students] - ) - - message_body = mail.outbox[0].body - self.assertIn(uni_message, message_body) - - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_unicode_students_send_to_all(self): """ Make sure email (with Unicode characters) send to all goes there. @@ -291,7 +260,6 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): @override_settings(BULK_EMAIL_EMAILS_PER_TASK=3) @patch('bulk_email.tasks.update_subtask_status') - @patch('bulk_email.models.html_to_text', Mock(return_value='Mocking CourseEmail.text_message')) def test_chunked_queries_send_numerous_emails(self, email_mock): """ Test sending a large number of emails, to test the chunked querying @@ -329,3 +297,42 @@ class TestEmailSendFromDashboard(ModuleStoreTestCase): [s.email for s in self.students] + [s.email for s in added_users if s not in optouts]) self.assertItemsEqual(outbox_contents, should_send_contents) + + +@override_settings(MODULESTORE=TEST_DATA_MOCK_MODULESTORE) +@patch.dict(settings.FEATURES, {'ENABLE_INSTRUCTOR_EMAIL': True, 'REQUIRE_COURSE_EMAIL_AUTH': False}) +@skipIf(os.environ.get("TRAVIS") == 'true', "Skip this test in Travis CI.") +class TestEmailSendFromDashboard(EmailSendFromDashboardTestCase): + """ + Tests email sending without mocked html_to_text. + + Note that these tests are skipped on Travis because we can't use the + function `html_to_text` as it is currently implemented on Travis. + """ + + def test_unicode_message_send_to_all(self): + """ + Make sure email (with Unicode characters) send to all goes there. + """ + # Now we know we have pulled up the instructor dash's email view + # (in the setUp method), we can test sending an email. + + uni_message = u'ẗëṡẗ ṁëṡṡäġë ḟöṛ äḷḷ ｲ乇丂ｲ ﾶ乇丂丂ﾑg乇 ｷo尺 ﾑﾚﾚ тэѕт мэѕѕаБэ fоѓ аll' + test_email = { + 'action': 'Send email', + 'send_to': 'all', + 'subject': 'test subject for all', + 'message': uni_message + } + # Post the email to the instructor dashboard API + response = self.client.post(self.send_mail_url, test_email) + self.assertEquals(json.loads(response.content), self.success_content) + + self.assertEquals(len(mail.outbox), 1 + len(self.staff) + len(self.students)) + self.assertItemsEqual( + [e.to[0] for e in mail.outbox], + [self.instructor.email] + [s.email for s in self.staff] + [s.email for s in self.students] + ) + + message_body = mail.outbox[0].body + self.assertIn(uni_message, message_body)