Merge pull request #22795 from ssuda/fix-cron-twice

fix: cron job firing immediately after save

frappe/core/doctype/scheduled_job_type/scheduled_job_type.py

@@ -105,9 +105,12 @@ class ScheduledJobType(Document):
 		if not self.cron_format:
 			self.cron_format = CRON_MAP[self.frequency]
 
-		return croniter(
-			self.cron_format, get_datetime(self.last_execution or datetime(2000, 1, 1))
-		).get_next(datetime)
+		# If this is a cold start then last_execution will not be set.
+		# Creation is set as fallback because if very old fallback is set job might trigger
+		# immediately, even when it's meant to be daily.
+		# A dynamic fallback like current time might miss the scheduler interval and job will never start.
+		last_execution = get_datetime(self.last_execution or self.creation)
+		return croniter(self.cron_format, last_execution).get_next(datetime)
 
 	def execute(self):
 		self.scheduler_log = None

frappe/core/doctype/scheduled_job_type/test_scheduled_job_type.py

@@ -1,9 +1,12 @@
 # Copyright (c) 2019, Frappe Technologies and Contributors
 # License: MIT. See LICENSE
+from datetime import timedelta
+
 import frappe
 from frappe.core.doctype.scheduled_job_type.scheduled_job_type import sync_jobs
 from frappe.tests.utils import FrappeTestCase
 from frappe.utils import get_datetime
+from frappe.utils.data import add_to_date, now_datetime
 
 
 class TestScheduledJobType(FrappeTestCase):
@@ -65,9 +68,34 @@ class TestScheduledJobType(FrappeTestCase):
 		self.assertFalse(job.is_event_due(get_datetime("2019-01-31 23:59:59")))
 
 	def test_cron_job(self):
+		# Daily but offset by 45 minutes
+		job = frappe.get_doc(
+			"Scheduled Job Type",
+			dict(method="frappe.core.doctype.log_settings.log_settings.run_log_clean_up"),
+		)
+		self.assertEqual(
+			job.next_execution,
+			add_to_date(None, days=1).replace(hour=0, minute=45, second=0, microsecond=0),
+		)
 		# runs every 15 mins
 		job = frappe.get_doc("Scheduled Job Type", dict(method="frappe.oauth.delete_oauth2_data"))
 		job.db_set("last_execution", "2019-01-01 00:00:00")
+		self.assertEqual(job.next_execution, get_datetime("2019-01-01 00:15:00"))
 		self.assertTrue(job.is_event_due(get_datetime("2019-01-01 00:15:01")))
 		self.assertFalse(job.is_event_due(get_datetime("2019-01-01 00:05:06")))
 		self.assertFalse(job.is_event_due(get_datetime("2019-01-01 00:14:59")))
+
+	def test_cold_start(self):
+		now = now_datetime()
+		just_before_12_am = now.replace(hour=11, minute=59, second=30)
+		just_after_12_am = now.replace(hour=0, minute=0, second=30) + timedelta(days=1)
+
+		job = frappe.new_doc("Scheduled Job Type")
+		job.frequency = "Daily"
+		job.set_user_and_timestamp()
+
+		with self.freeze_time(just_before_12_am):
+			self.assertFalse(job.is_event_due())
+
+		with self.freeze_time(just_after_12_am):
+			self.assertTrue(job.is_event_due())

frappe/tests/test_test_utils.py

@@ -1,5 +1,8 @@
+from datetime import timedelta
+
 import frappe
 from frappe.tests.utils import FrappeTestCase, change_settings
+from frappe.utils.data import now_datetime
 
 
 class TestTestUtils(FrappeTestCase):
@@ -27,6 +30,13 @@ class TestTestUtils(FrappeTestCase):
 		restored_settings = frappe.get_system_settings("logout_on_password_reset")
 		self.assertEqual(current_setting, restored_settings)
 
+	def test_time_freezing(self):
+		now = now_datetime()
+
+		tomorrow = now + timedelta(days=1)
+		with self.freeze_time(tomorrow):
+			self.assertEqual(now_datetime(), tomorrow)
+
 
 def tearDownModule():
 	"""assertions for ensuring tests didn't leave state behind"""

frappe/tests/utils.py

@@ -7,9 +7,12 @@ from collections.abc import Sequence
 from contextlib import contextmanager
 from unittest.mock import patch
 
+import pytz
+
 import frappe
 from frappe.model.base_document import BaseDocument, get_controller
 from frappe.utils import cint
+from frappe.utils.data import convert_utc_to_timezone, get_datetime, get_system_timezone
 
 datetime_like_types = (datetime.datetime, datetime.date, datetime.time, datetime.timedelta)
 
@@ -154,6 +157,17 @@ class FrappeTestCase(unittest.TestCase):
 		frappe.init(old_site, force=True)
 		frappe.connect()
 
+	@contextmanager
+	def freeze_time(self, time_to_freeze, *args, **kwargs):
+		from freezegun import freeze_time
+
+		# Freeze time expects UTC or tzaware objects. We have neither, so convert to UTC.
+		timezone = pytz.timezone(get_system_timezone())
+		fake_time_with_tz = timezone.localize(get_datetime(time_to_freeze)).astimezone(pytz.utc)
+
+		with freeze_time(fake_time_with_tz, *args, **kwargs):
+			yield
+
 
 class MockedRequestTestCase(FrappeTestCase):
 	def setUp(self):

pyproject.toml

@@ -106,3 +106,4 @@ unittest-xml-reporting = "~=3.2.0"
 watchdog = "~=3.0.0"
 hypothesis = "~=6.77.0"
 responses = "==0.23.1"
+freezegun = "~=1.2.2"