fix: Add testing, fix bug in fetch_meeting_attendance_task (#6961)

* test: Test rfc_editor_index_update_task

* chore: Add docstring to test

* fix: Reuse stats instead of fetching twice

* test: Test fetch_meeting_attendance_task

* chore: Remove outdated tasks

* Revert "chore: Add docstring to test"

This reverts commit 0395410d665c0d310248fd151386f013357c5d13.

* Revert "test: Test rfc_editor_index_update_task"

This reverts commit 4dd9dd131723497db3d2aa76166169fd32c42fdd.

* test: Test rfc_editor_index_update_task

This time without reformatting the entire file...

* chore: Remove accidentally committed fragment

* test: Annotate function to satisfy mypy

* chore: Remove unused imports

ietf/stats/tasks.py

@@ -19,9 +19,9 @@ def fetch_meeting_attendance_task():
     except RuntimeError as err:
         log.log(f"Error in fetch_meeting_attendance_task: {err}")
     else:
-        for meeting, stats in zip(meetings, fetch_attendance_from_meetings(meetings)):
+        for meeting, meeting_stats in zip(meetings, stats):
             log.log(
                 "Fetched data for meeting {:>3}: {:4d} processed, {:4d} added, {:4d} in table".format(
-                    meeting.number, stats.processed, stats.added, stats.total
+                    meeting.number, meeting_stats.processed, meeting_stats.added, meeting_stats.total
                 )
             )

ietf/stats/tests.py

@@ -29,7 +29,8 @@ from ietf.name.models import FormalLanguageName, DocRelationshipName, CountryNam
 from ietf.review.factories import ReviewRequestFactory, ReviewerSettingsFactory, ReviewAssignmentFactory
 from ietf.stats.models import MeetingRegistration, CountryAlias
 from ietf.stats.factories import MeetingRegistrationFactory
-from ietf.stats.utils import get_meeting_registration_data
+from ietf.stats.tasks import fetch_meeting_attendance_task
+from ietf.stats.utils import get_meeting_registration_data, FetchStats
 from ietf.utils.timezone import date_today
 
 
@@ -300,3 +301,20 @@ class StatisticsTests(TestCase):
         get_meeting_registration_data(meeting)
         query = MeetingRegistration.objects.all()
         self.assertEqual(query.count(), 2)
+
+
+class TaskTests(TestCase):
+    @patch("ietf.stats.tasks.fetch_attendance_from_meetings")
+    def test_fetch_meeting_attendance_task(self, mock_fetch_attendance):
+        today = date_today()
+        meetings = [
+            MeetingFactory(type_id="ietf", date=today - datetime.timedelta(days=1)),
+            MeetingFactory(type_id="ietf", date=today - datetime.timedelta(days=2)),
+            MeetingFactory(type_id="ietf", date=today - datetime.timedelta(days=3)),
+        ]
+        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])

ietf/sync/tests.py

@@ -9,9 +9,12 @@ import datetime
 import mock
 import quopri
 
+from dataclasses import dataclass
+
 from django.conf import settings
 from django.urls import reverse as urlreverse
 from django.utils import timezone
+from django.test.utils import override_settings
 
 import debug                            # pyflakes:ignore
 
@@ -20,7 +23,7 @@ from ietf.doc.models import Document, DocEvent, DeletedEvent, DocTagName, Relate
 from ietf.doc.utils import add_state_change_event
 from ietf.group.factories import GroupFactory
 from ietf.person.models import Person
-from ietf.sync import iana, rfceditor
+from ietf.sync import iana, rfceditor, tasks
 from ietf.utils.mail import outbox, empty_outbox
 from ietf.utils.test_utils import login_testing_unauthorized
 from ietf.utils.test_utils import TestCase
@@ -672,3 +675,90 @@ class RFCEditorUndoTests(TestCase):
 
         e.content_type.model_class().objects.create(**json.loads(e.json))
         self.assertTrue(StateDocEvent.objects.filter(desc="First", doc=draft))
+
+
+class TaskTests(TestCase):
+    @override_settings(
+        RFC_EDITOR_INDEX_URL="https://rfc-editor.example.com/index/",
+        RFC_EDITOR_ERRATA_JSON_URL="https://rfc-editor.example.com/errata/",
+    )
+    @mock.patch("ietf.sync.tasks.update_docs_from_rfc_index")
+    @mock.patch("ietf.sync.tasks.parse_index")
+    @mock.patch("ietf.sync.tasks.requests.get")
+    def test_rfc_editor_index_update_task(
+        self, requests_get_mock, parse_index_mock, update_docs_mock
+    ) -> None:  # the annotation here prevents mypy from complaining about annotation-unchecked
+        """rfc_editor_index_update_task calls helpers correctly
+        
+        This tests that data flow is as expected. Assumes the individual helpers are
+        separately tested to function correctly.
+        """
+        @dataclass
+        class MockIndexData:
+            """Mock index item that claims to be a specified length"""
+            length: int
+
+            def __len__(self):
+                return self.length
+
+        @dataclass
+        class MockResponse:
+            """Mock object that contains text and json() that claims to be a specified length"""
+            text: str
+            json_length: int = 0
+
+            def json(self):
+                return MockIndexData(length=self.json_length)
+
+        # Response objects
+        index_response = MockResponse(text="this is the index")
+        errata_response = MockResponse(
+            text="these are the errata", json_length=rfceditor.MIN_ERRATA_RESULTS
+        )
+
+        # 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
+
+        tasks.rfc_editor_index_update_task(full_index=False)
+
+        # Check parse_index() call
+        self.assertTrue(parse_index_mock.called)
+        (parse_index_args, _) = parse_index_mock.call_args
+        self.assertEqual(
+            parse_index_args[0].read(),  # arg is a StringIO
+            "this is the index",
+            "parse_index is called with the index text in a StringIO",
+        )
+
+        # Check update_docs_from_rfc_index call
+        self.assertTrue(update_docs_mock.called)
+        (update_docs_args, update_docs_kwargs) = update_docs_mock.call_args
+        self.assertEqual(
+            update_docs_args, (parse_index_mock.return_value, errata_response.json())
+        )
+        self.assertIsNotNone(update_docs_kwargs["skip_older_than_date"])
+
+        # Test again with full_index = True
+        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
+        self.assertTrue(parse_index_mock.called)
+        (parse_index_args, _) = parse_index_mock.call_args
+        self.assertEqual(
+            parse_index_args[0].read(),  # arg is a StringIO
+            "this is the index",
+            "parse_index is called with the index text in a StringIO",
+        )
+
+        # Check update_docs_from_rfc_index call
+        self.assertTrue(update_docs_mock.called)
+        (update_docs_args, update_docs_kwargs) = update_docs_mock.call_args
+        self.assertEqual(
+            update_docs_args, (parse_index_mock.return_value, errata_response.json())
+        )
+        self.assertIsNone(update_docs_kwargs["skip_older_than_date"])

ietf/utils/tasks.py

@@ -7,33 +7,10 @@ from smtplib import SMTPException
 
 from ietf.message.utils import send_scheduled_message_from_send_queue
 from ietf.message.models import SendQueue
-from ietf.review.tasks import send_review_reminders_task
-from ietf.stats.tasks import fetch_meeting_attendance_task
-from ietf.sync.tasks import rfc_editor_index_update_task
 from ietf.utils import log
 from ietf.utils.mail import log_smtp_exception, send_error_email
 
 
-@shared_task
-def every_15m_task():
-    """Queue four-times-hourly tasks for execution"""
-    # todo decide whether we want this to be a meta-task or to individually schedule the tasks
-    send_scheduled_mail_task.delay()
-    # Parse the last year of RFC index data to get new RFCs. Needed until
-    # https://github.com/ietf-tools/datatracker/issues/3734 is addressed.
-    rfc_editor_index_update_task.delay(full_index=False)
-
-
-@shared_task
-def daily_task():
-    """Queue daily tasks for execution"""
-    fetch_meeting_attendance_task.delay()
-    send_review_reminders_task.delay()
-    # Run an extended version of the rfc editor update to catch changes
-    # with backdated timestamps
-    rfc_editor_index_update_task.delay(full_index=True)
-
-
 @shared_task
 def send_scheduled_mail_task():
     """Send scheduled email