Refactor calculate() to properly handle paused jobs and fix tests

discord_reminder_bot/misc.py

@@ -6,12 +6,13 @@ from typing import TYPE_CHECKING
 from apscheduler.triggers.cron import CronTrigger
 from apscheduler.triggers.date import DateTrigger
 from apscheduler.triggers.interval import IntervalTrigger
+from loguru import logger
 
 if TYPE_CHECKING:
     from apscheduler.job import Job
 
 
-def calculate(job: Job) -> str | None:
+def calculate(job: Job) -> str:
     """Calculate the time left for a job.
 
     Args:
@@ -22,11 +23,19 @@ def calculate(job: Job) -> str | None:
     """
     trigger_time = None
     if isinstance(job.trigger, DateTrigger | IntervalTrigger):
-        trigger_time = job.next_run_time if hasattr(job, "next_run_time") else None
+        trigger_time = job.next_run_time or None
+
     elif isinstance(job.trigger, CronTrigger):
+        if not job.next_run_time:
+            logger.debug("No next run time found so probably paused?")
+            return "Paused"
+
         trigger_time = job.trigger.get_next_fire_time(None, datetime.datetime.now(tz=job._scheduler.timezone))  # noqa: SLF001
 
+    logger.debug(f"{type(job.trigger)=}, {trigger_time=}")
+
     if not trigger_time:
+        logger.debug("No trigger time found")
         return "Paused"
 
     return f"<t:{int(trigger_time.timestamp())}:R>"
@@ -70,4 +79,10 @@ def calc_time(time: datetime.datetime | None) -> str:
     if not time:
         return "None"
 
+    if time.tzinfo is None or time.tzinfo.utcoffset(time) is None:
+        logger.warning(f"Time is not timezone-aware: {time}")
+
+    if time < datetime.datetime.now(tz=time.tzinfo):
+        logger.warning(f"Time is in the past: {time}")
+
     return f"<t:{int(time.timestamp())}:R>"

pyproject.toml

@@ -6,29 +6,26 @@ readme = "README.md"
 requires-python = ">=3.10"
 dependencies = [
     # The Discord bot library uses discord.py
-    # legacy-cgi and audioop-lts are because Python 3.13 removed cgi module and audioop module
+    "discord-py[speed]>=2.5.0", # https://github.com/Rapptz/discord.py
-    "discord-py[speed]>=2.4.0,<3.0.0",                     # https://github.com/Rapptz/discord.py
-    "legacy-cgi>=2.6.2,<3.0.0; python_version >= '3.13'",  # https://github.com/jackrosenthal/legacy-cgi
-    "audioop-lts>=0.2.1,<1.0.0; python_version >= '3.13'", # https://github.com/AbstractUmbra/audioop
 
     # For parsing dates and times in /remind commands
     "dateparser>=1.0.0", # https://github.com/scrapinghub/dateparser
 
     # For sending webhook messages to Discord
-    "discord-webhook>=1.3.1,<2.0.0", # https://github.com/lovvskillz/python-discord-webhook
+    "discord-webhook>=1.3.1", # https://github.com/lovvskillz/python-discord-webhook
 
     # For scheduling reminders, sqlalchemy is needed for storing reminders in a database
-    "apscheduler>=3.11.0,<4.0.0", # https://github.com/agronholm/apscheduler
+    "apscheduler>=3.11.0", # https://github.com/agronholm/apscheduler
-    "sqlalchemy>=2.0.37,<3.0.0",  # https://github.com/sqlalchemy/sqlalchemy
+    "sqlalchemy>=2.0.37",  # https://github.com/sqlalchemy/sqlalchemy
 
     # For loading environment variables from a .env file
-    "python-dotenv>=1.0.1,<2.0.0", # https://github.com/theskumar/python-dotenv
+    "python-dotenv>=1.0.1", # https://github.com/theskumar/python-dotenv
 
     # For error tracking
-    "sentry-sdk>=2.20.0,<3.0.0", # https://github.com/getsentry/sentry-python
+    "sentry-sdk>=2.20.0", # https://github.com/getsentry/sentry-python
 
     # For logging
-    "loguru>=0.7.3,<1.0.0", # https://github.com/Delgan/loguru
+    "loguru>=0.7.3", # https://github.com/Delgan/loguru
 ]
 
 [dependency-groups]
@@ -142,4 +139,7 @@ log_cli = true
 log_cli_level = "INFO"
 log_cli_format = "%(asctime)s [%(levelname)8s] %(message)s (%(filename)s:%(lineno)s)"
 log_cli_date_format = "%Y-%m-%d %H:%M:%S"
-filterwarnings = ["ignore::DeprecationWarning:aiohttp.cookiejar"]
+filterwarnings = [
+    "ignore:Parsing dates involving a day of month without a year specified is ambiguious:DeprecationWarning:dateparser\\.utils\\.strptime",
+    "ignore::DeprecationWarning:aiohttp.cookiejar",
+]

tests/test_misc.py

@@ -73,25 +73,26 @@ def dummy_job() -> None:
 def test_calculate() -> None:
     """Test the calculate function with various job inputs."""
     scheduler = BackgroundScheduler()
+    scheduler.timezone = timezone.utc
     scheduler.start()
 
     # Create a job with a DateTrigger
-    run_date = datetime(2270, 10, 1, 12, 0, 0, tzinfo=timezone.utc)
+    run_date = datetime(2270, 10, 1, 12, 0, 0, tzinfo=scheduler.timezone)
     job: Job = scheduler.add_job(dummy_job, trigger=DateTrigger(run_date=run_date), id="test_job", name="Test Job")
 
     expected_output = "<t:9490737600:R>"
-    assert_msg: str = f"Expected {expected_output}, got {calculate(job)}"
+    assert_msg: str = f"Expected {expected_output}, got {calculate(job)}\nState:{job.__getstate__()}"
     assert calculate(job) == expected_output, assert_msg
 
     # Modify the job to have a next_run_time
     job.modify(next_run_time=run_date)
-    assert_msg: str = f"Expected {expected_output}, got {calculate(job)}"
+    assert_msg: str = f"Expected {expected_output}, got {calculate(job)}\nState:{job.__getstate__()}"
     assert calculate(job) == expected_output, assert_msg
 
-    # Paused job should still return the same output
+    # Paused job should return "Paused"
     job.pause()
-    assert_msg: str = f"Expected None, got {calculate(job)}"
+    assert_msg: str = f"Expected 'Paused', got {calculate(job)}\nState:{job.__getstate__()}"
-    assert not calculate(job), assert_msg
+    assert calculate(job) == "Paused", assert_msg
 
     scheduler.shutdown()
 
@@ -101,7 +102,7 @@ def test_calculate_cronjob() -> None:
     scheduler = BackgroundScheduler()
     scheduler.start()
 
-    run_date = datetime(2270, 10, 1, 12, 0, 0, tzinfo=timezone.utc)
+    run_date = datetime(2270, 10, 1, 12, 0, 0, tzinfo=scheduler.timezone)
     job: Job = scheduler.add_job(
         dummy_job,
         trigger=CronTrigger(
@@ -117,11 +118,10 @@ def test_calculate_cronjob() -> None:
     job.modify(next_run_time=run_date)
 
     expected_output: str = f"<t:{int(run_date.timestamp())}:R>"
-    assert calculate(job) == expected_output, f"Expected {expected_output}, got {calculate(job)}"
+    assert calculate(job) == expected_output, f"Expected {expected_output}, got {calculate(job)}\nState:{job.__getstate__()}"
 
-    # You can't pause a CronTrigger job so this should return the same output
     job.pause()
-    assert calculate(job) == expected_output, f"Expected {expected_output}, got {calculate(job)}"
+    assert calculate(job) == "Paused", f"Expected Paused, got {calculate(job)}\nState:{job.__getstate__()}"
     scheduler.shutdown()
 
 
@@ -130,15 +130,15 @@ def test_calculate_intervaljob() -> None:
     scheduler = BackgroundScheduler()
     scheduler.start()
 
-    run_date = datetime(2270, 12, 31, 23, 59, 59, tzinfo=timezone.utc)
+    run_date = datetime(2270, 12, 31, 23, 59, 59, tzinfo=scheduler.timezone)
     job = scheduler.add_job(dummy_job, trigger=IntervalTrigger(seconds=3600), id="test_interval_job", name="Test Interval Job")
     # Force next_run_time to expected value for testing
     job.modify(next_run_time=run_date)
 
     expected_output = f"<t:{int(run_date.timestamp())}:R>"
-    assert calculate(job) == expected_output, f"Expected {expected_output}, got {calculate(job)}"
+    assert calculate(job) == expected_output, f"Expected {expected_output}, got {calculate(job)}\nState:{job.__getstate__()}"
 
-    # Paused job should return False
+    # Paused job should return "Paused"
     job.pause()
-    assert not calculate(job), f"Expected None, got {calculate(job)}"
+    assert calculate(job) == "Paused", f"Expected Paused, got {calculate(job)}\nState:{job.__getstate__()}"
     scheduler.shutdown()