From 36c43c8520b4d1faa8fe03138a43f221d950f088 Mon Sep 17 00:00:00 2001 From: Jennifer Richards Date: Wed, 24 Jan 2024 10:53:42 -0400 Subject: [PATCH] chore: add task tests; move message task to message app (#6964) * test: Test send_review_reminders_task * refactor: Move send_scheduled_mail_task to message app * chore: Remove unused import * test: Add Message/SendQueue factories * test: Test send_scheduled_mail_task * test: Reset mocks before reuse * test: Cover error conditions * test: Return non-empty change set * test: Test SMTPException handling * test: Test fetch_attendance_from_meetings() * test: Test RuntimeError handling * test: RFC index sync should populate authors --- ietf/message/factories.py | 27 ++++++++ ietf/{utils => message}/tasks.py | 0 ietf/message/tests.py | 26 +++++++- ietf/review/tests.py | 66 +++++++++++++++++++ ietf/stats/tests.py | 32 ++++++++- ietf/sync/tests.py | 52 +++++++++++++-- .../management/commands/periodic_tasks.py | 2 +- 7 files changed, 195 insertions(+), 10 deletions(-) create mode 100644 ietf/message/factories.py rename ietf/{utils => message}/tasks.py (100%) diff --git a/ietf/message/factories.py b/ietf/message/factories.py new file mode 100644 index 000000000..72781512e --- /dev/null +++ b/ietf/message/factories.py @@ -0,0 +1,27 @@ +# Copyright The IETF Trust 2024, All Rights Reserved +import factory + +from ietf.person.models import Person +from .models import Message, SendQueue + + +class MessageFactory(factory.django.DjangoModelFactory): + class Meta: + model = Message + + by = factory.LazyFunction(lambda: Person.objects.get(name="(System)")) + subject = factory.Faker("sentence") + to = factory.Faker("email") + frm = factory.Faker("email") + cc = factory.Faker("email") + bcc = factory.Faker("email") + body = factory.Faker("paragraph") + content_type = "text/plain" + + +class SendQueueFactory(factory.django.DjangoModelFactory): + class Meta: + model = SendQueue + + by = factory.LazyFunction(lambda: Person.objects.get(name="(System)")) + message = factory.SubFactory(MessageFactory) diff --git a/ietf/utils/tasks.py b/ietf/message/tasks.py similarity index 100% rename from ietf/utils/tasks.py rename to ietf/message/tasks.py diff --git a/ietf/message/tests.py b/ietf/message/tests.py index a027df447..7fbd29167 100644 --- a/ietf/message/tests.py +++ b/ietf/message/tests.py @@ -1,8 +1,9 @@ # Copyright The IETF Trust 2013-2020, All Rights Reserved # -*- coding: utf-8 -*- - - import datetime +import mock + +from smtplib import SMTPException from django.urls import reverse as urlreverse from django.utils import timezone @@ -10,7 +11,9 @@ from django.utils import timezone import debug # pyflakes:ignore from ietf.group.factories import GroupFactory +from ietf.message.factories import SendQueueFactory from ietf.message.models import Message, SendQueue +from ietf.message.tasks import send_scheduled_mail_task from ietf.message.utils import send_scheduled_message_from_send_queue from ietf.person.models import Person from ietf.utils.mail import outbox, send_mail_text, send_mail_message, get_payload_text @@ -128,3 +131,22 @@ class SendScheduledAnnouncementsTests(TestCase): self.assertTrue("This is a test" in outbox[-1]["Subject"]) self.assertTrue("--NextPart" in outbox[-1].as_string()) self.assertTrue(SendQueue.objects.get(id=q.id).sent_at) + + +class TaskTests(TestCase): + @mock.patch("ietf.message.tasks.log_smtp_exception") + @mock.patch("ietf.message.tasks.send_scheduled_message_from_send_queue") + def test_send_scheduled_mail_task(self, mock_send_message, mock_log_smtp_exception): + not_yet_sent = SendQueueFactory() + SendQueueFactory(sent_at=timezone.now()) # already sent + send_scheduled_mail_task() + self.assertEqual(mock_send_message.call_count, 1) + self.assertEqual(mock_send_message.call_args[0], (not_yet_sent,)) + self.assertFalse(mock_log_smtp_exception.called) + + mock_send_message.reset_mock() + mock_send_message.side_effect = SMTPException + send_scheduled_mail_task() + self.assertEqual(mock_send_message.call_count, 1) + self.assertEqual(mock_send_message.call_args[0], (not_yet_sent,)) + self.assertTrue(mock_log_smtp_exception.called) diff --git a/ietf/review/tests.py b/ietf/review/tests.py index f9d55d9d1..e9ddbd47a 100644 --- a/ietf/review/tests.py +++ b/ietf/review/tests.py @@ -1,9 +1,11 @@ # Copyright The IETF Trust 2019-2020, All Rights Reserved # -*- coding: utf-8 -*- import datetime +import mock import debug # pyflakes:ignore from pyquery import PyQuery + from ietf.group.factories import RoleFactory from ietf.doc.factories import WgDraftFactory from ietf.utils.mail import empty_outbox, get_payload_text, outbox @@ -13,6 +15,7 @@ from ietf.utils.timezone import date_today, datetime_from_date from .factories import ReviewAssignmentFactory, ReviewRequestFactory, ReviewerSettingsFactory from .mailarch import hash_list_message_id from .models import ReviewerSettings, ReviewSecretarySettings, ReviewTeamSettings, UnavailablePeriod +from .tasks import send_review_reminders_task from .utils import (email_secretary_reminder, review_assignments_needing_secretary_reminder, email_reviewer_reminder, review_assignments_needing_reviewer_reminder, send_reminder_unconfirmed_assignments, send_review_reminder_overdue_assignment, @@ -550,3 +553,66 @@ class AddReviewCommentTestCase(TestCase): # But can't have the comment we are goint to add. self.assertContains(r, 'This is a test.') + +class TaskTests(TestCase): + # hyaaa it's mockzilla + @mock.patch("ietf.review.tasks.date_today") + @mock.patch("ietf.review.tasks.review_assignments_needing_reviewer_reminder") + @mock.patch("ietf.review.tasks.email_reviewer_reminder") + @mock.patch("ietf.review.tasks.review_assignments_needing_secretary_reminder") + @mock.patch("ietf.review.tasks.email_secretary_reminder") + @mock.patch("ietf.review.tasks.send_unavailability_period_ending_reminder") + @mock.patch("ietf.review.tasks.send_reminder_all_open_reviews") + @mock.patch("ietf.review.tasks.send_review_reminder_overdue_assignment") + @mock.patch("ietf.review.tasks.send_reminder_unconfirmed_assignments") + def test_send_review_reminders_task( + self, + mock_send_reminder_unconfirmed_assignments, + mock_send_review_reminder_overdue_assignment, + mock_send_reminder_all_open_reviews, + mock_send_unavailability_period_ending_reminder, + mock_email_secretary_reminder, + mock_review_assignments_needing_secretary_reminder, + mock_email_reviewer_reminder, + mock_review_assignments_needing_reviewer_reminder, + mock_date_today, + ): + """Test that send_review_reminders calls functions correctly + + Does not test individual methods, just that they are called as expected. + """ + mock_today = object() + assignment = ReviewAssignmentFactory() + secretary_role = RoleFactory(name_id="secr") + + mock_date_today.return_value = mock_today + mock_review_assignments_needing_reviewer_reminder.return_value = [assignment] + mock_review_assignments_needing_secretary_reminder.return_value = [[assignment, secretary_role]] + mock_send_unavailability_period_ending_reminder.return_value = ["pretending I sent a period end reminder"] + mock_send_review_reminder_overdue_assignment.return_value = ["pretending I sent an overdue reminder"] + mock_send_reminder_all_open_reviews.return_value = ["pretending I sent an open review reminder"] + mock_send_reminder_unconfirmed_assignments.return_value = ["pretending I sent an unconfirmed reminder"] + + send_review_reminders_task() + + self.assertEqual(mock_review_assignments_needing_reviewer_reminder.call_count, 1) + self.assertEqual(mock_review_assignments_needing_reviewer_reminder.call_args[0], (mock_today,)) + self.assertEqual(mock_email_reviewer_reminder.call_count, 1) + self.assertEqual(mock_email_reviewer_reminder.call_args[0], (assignment,)) + + self.assertEqual(mock_review_assignments_needing_secretary_reminder.call_count, 1) + self.assertEqual(mock_review_assignments_needing_secretary_reminder.call_args[0], (mock_today,)) + self.assertEqual(mock_email_secretary_reminder.call_count, 1) + self.assertEqual(mock_email_secretary_reminder.call_args[0], (assignment, secretary_role)) + + self.assertEqual(mock_send_unavailability_period_ending_reminder.call_count, 1) + self.assertEqual(mock_send_unavailability_period_ending_reminder.call_args[0], (mock_today,)) + + self.assertEqual(mock_send_review_reminder_overdue_assignment.call_count, 1) + self.assertEqual(mock_send_review_reminder_overdue_assignment.call_args[0], (mock_today,)) + + self.assertEqual(mock_send_reminder_all_open_reviews.call_count, 1) + self.assertEqual(mock_send_reminder_all_open_reviews.call_args[0], (mock_today,)) + + self.assertEqual(mock_send_reminder_unconfirmed_assignments.call_count, 1) + self.assertEqual(mock_send_reminder_unconfirmed_assignments.call_args[0], (mock_today,)) diff --git a/ietf/stats/tests.py b/ietf/stats/tests.py index e4194a7f9..f0e8a19c4 100644 --- a/ietf/stats/tests.py +++ b/ietf/stats/tests.py @@ -30,7 +30,7 @@ from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory, from ietf.stats.models import MeetingRegistration, CountryAlias from ietf.stats.factories import MeetingRegistrationFactory from ietf.stats.tasks import fetch_meeting_attendance_task -from ietf.stats.utils import get_meeting_registration_data, FetchStats +from ietf.stats.utils import get_meeting_registration_data, FetchStats, fetch_attendance_from_meetings from ietf.utils.timezone import date_today @@ -302,6 +302,28 @@ class StatisticsTests(TestCase): query = MeetingRegistration.objects.all() self.assertEqual(query.count(), 2) + @patch("ietf.stats.utils.get_meeting_registration_data") + def test_fetch_attendance_from_meetings(self, mock_get_mtg_reg_data): + mock_meetings = [object(), object(), object()] + mock_get_mtg_reg_data.side_effect = ( + (1, 2, 3), + (4, 5, 6), + (7, 8, 9), + ) + stats = fetch_attendance_from_meetings(mock_meetings) + self.assertEqual( + [mock_get_mtg_reg_data.call_args_list[n][0][0] for n in range(3)], + mock_meetings, + ) + self.assertEqual( + stats, + [ + FetchStats(1, 2, 3), + FetchStats(4, 5, 6), + FetchStats(7, 8, 9), + ] + ) + class TaskTests(TestCase): @patch("ietf.stats.tasks.fetch_attendance_from_meetings") @@ -315,6 +337,12 @@ class TaskTests(TestCase): mock_fetch_attendance.return_value = [FetchStats(1,2,3), FetchStats(1,2,3)] fetch_meeting_attendance_task() - self.assertEqual(mock_fetch_attendance.call_count, 1) self.assertCountEqual(mock_fetch_attendance.call_args[0][0], meetings[0:2]) + + # test handling of RuntimeError + mock_fetch_attendance.reset_mock() + mock_fetch_attendance.side_effect = RuntimeError + fetch_meeting_attendance_task() + self.assertTrue(mock_fetch_attendance.called) + # Good enough that we got here without raising an exception diff --git a/ietf/sync/tests.py b/ietf/sync/tests.py index d660774d0..6ce1d1252 100644 --- a/ietf/sync/tests.py +++ b/ietf/sync/tests.py @@ -8,6 +8,7 @@ import json import datetime import mock import quopri +import requests from dataclasses import dataclass @@ -18,7 +19,7 @@ from django.test.utils import override_settings import debug # pyflakes:ignore -from ietf.doc.factories import WgDraftFactory, RfcFactory +from ietf.doc.factories import WgDraftFactory, RfcFactory, DocumentAuthorFactory from ietf.doc.models import Document, DocEvent, DeletedEvent, DocTagName, RelatedDocument, State, StateDocEvent from ietf.doc.utils import add_state_change_event from ietf.group.factories import GroupFactory @@ -238,6 +239,7 @@ class RFCSyncTests(TestCase): external_url="http://my-external-url.example.com", note="this is a note", ) + DocumentAuthorFactory.create_batch(2, document=draft_doc) draft_doc.action_holders.add(draft_doc.ad) # not normally set, but add to be sure it's cleared RfcFactory(rfc_number=123) @@ -381,6 +383,7 @@ class RFCSyncTests(TestCase): rfc_doc = Document.objects.filter(rfc_number=1234, type_id="rfc").first() self.assertIsNotNone(rfc_doc, "RFC document should have been created") + self.assertEqual(rfc_doc.authors(), draft_doc.authors()) rfc_events = rfc_doc.docevent_set.all() self.assertEqual(len(rfc_events), 8) expected_events = [ @@ -715,11 +718,14 @@ class TaskTests(TestCase): errata_response = MockResponse( text="these are the errata", json_length=rfceditor.MIN_ERRATA_RESULTS ) - + rfc = RfcFactory() + # Test with full_index = False requests_get_mock.side_effect = (index_response, errata_response) # will step through these parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS) - update_docs_mock.return_value = [] # not tested + update_docs_mock.return_value = ( + (rfc.rfc_number, ("something changed",), rfc, False), + ) tasks.rfc_editor_index_update_task(full_index=False) @@ -741,9 +747,10 @@ class TaskTests(TestCase): self.assertIsNotNone(update_docs_kwargs["skip_older_than_date"]) # Test again with full_index = True + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() requests_get_mock.side_effect = (index_response, errata_response) # will step through these - parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS) - update_docs_mock.return_value = [] # not tested tasks.rfc_editor_index_update_task(full_index=True) # Check parse_index() call @@ -762,3 +769,38 @@ class TaskTests(TestCase): update_docs_args, (parse_index_mock.return_value, errata_response.json()) ) self.assertIsNone(update_docs_kwargs["skip_older_than_date"]) + + # Test error handling + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = requests.Timeout # timeout on every get() + tasks.rfc_editor_index_update_task(full_index=False) + self.assertFalse(parse_index_mock.called) + self.assertFalse(update_docs_mock.called) + + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = [index_response, requests.Timeout] # timeout second get() + tasks.rfc_editor_index_update_task(full_index=False) + self.assertFalse(update_docs_mock.called) + + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = [index_response, errata_response] + # feed in an index that is too short + parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS - 1) + tasks.rfc_editor_index_update_task(full_index=False) + self.assertTrue(parse_index_mock.called) + self.assertFalse(update_docs_mock.called) + + requests_get_mock.reset_mock() + parse_index_mock.reset_mock() + update_docs_mock.reset_mock() + requests_get_mock.side_effect = [index_response, errata_response] + errata_response.json_length = rfceditor.MIN_ERRATA_RESULTS - 1 # too short + parse_index_mock.return_value = MockIndexData(length=rfceditor.MIN_INDEX_RESULTS) + tasks.rfc_editor_index_update_task(full_index=False) + self.assertFalse(update_docs_mock.called) diff --git a/ietf/utils/management/commands/periodic_tasks.py b/ietf/utils/management/commands/periodic_tasks.py index a6e44f467..18310430b 100644 --- a/ietf/utils/management/commands/periodic_tasks.py +++ b/ietf/utils/management/commands/periodic_tasks.py @@ -56,7 +56,7 @@ class Command(BaseCommand): def create_default_tasks(self): PeriodicTask.objects.get_or_create( name="Send scheduled mail", - task="ietf.utils.tasks.send_scheduled_mail_task", + task="ietf.meeting.tasks.send_scheduled_mail_task", defaults=dict( enabled=False, crontab=self.crontabs["every_15m"],