From 16813d834ba66bcf8f9d80d3ab23f6492f6cbd4f Mon Sep 17 00:00:00 2001 From: Prafyl <86287866+Prafyl@users.noreply.github.com> Date: Wed, 16 Sep 2026 17:27:00 +0545 Subject: [PATCH] fix(#47): count step values from the field minimum */n was checking target % n == 0, which only works when the field starts at 0. Day of month and month start at 1, so */6 on day of month matched 6, 12, 18, 24, 30 instead of 1, 7, 13, 19, 25, 31, and */3 on month gave Mar, Jun, Sep, Dec instead of Jan, Apr, Jul, Oct. _parse_arg now takes a min_value and checks (target - min_value) % n == 0. is_now passes min_value=1 for the dom and month fields. Minute, hour and day of week start at 0, so they keep the default and are unchanged. test_dom.py and test_month.py both use June 18th, and 18 is divisible by 6 and June is divisible by 2, so two of their assertions were passing on the old behaviour and had to be corrected. Added test_dom_step and test_month_step to cover dates on and off the step. --- pycron/__init__.py | 18 ++++++++++++------ tests/test_dom.py | 12 +++++++++++- tests/test_month.py | 14 ++++++++++++-- 3 files changed, 35 insertions(+), 9 deletions(-) diff --git a/pycron/__init__.py b/pycron/__init__.py index 349ebe9..dbe079f 100644 --- a/pycron/__init__.py +++ b/pycron/__init__.py @@ -37,7 +37,9 @@ def _to_int(value: Any, allow_daynames: bool = False) -> int: raise ValueError("Failed to parse string to integer") -def _parse_arg(value: str, target: int, allow_daynames: bool = False) -> bool: +def _parse_arg( + value: str, target: int, allow_daynames: bool = False, min_value: int = 0 +) -> bool: """ Parses a given value and checks if it matches the provided target. Allowing day names is optional, but can be useful for certain situations. @@ -46,6 +48,8 @@ def _parse_arg(value: str, target: int, allow_daynames: bool = False) -> bool: value = value to parse and check target = target value to compare with allow_daynames = True, to allow values like Mon or Monday + min_value = smallest value the field accepts, used as the + starting point for step values like */2 @output: True if the value matches the target, False otherwise """ # pylint: disable=too-many-branches @@ -99,8 +103,10 @@ def _parse_arg(value: str, target: int, allow_daynames: bool = False) -> bool: # Not sure if applicable for every situation, but just to make sure... if v != "*": continue - # If the remainder is zero, this matches - if target % _to_int(interval, allow_daynames=allow_daynames) == 0: + # Steps count from the start of the field, not from zero, + # so */2 on a 1-based field means 1, 3, 5... not 2, 4, 6... + interval_int = _to_int(interval, allow_daynames=allow_daynames) + if (target - min_value) % interval_int == 0: return True return False @@ -124,18 +130,18 @@ def is_now(s: str, dt: Optional[datetime] = None) -> bool: # Special case if both of the 'day' -fields are set -> allow either one to match # See: https://github.com/kipe/pycron/issues/29 if "*" not in dom and "*" not in dow: - day_rule = _parse_arg(dom, dt.day) or _parse_arg( + day_rule = _parse_arg(dom, dt.day, min_value=1) or _parse_arg( dow, 0 if weekday == 7 else weekday, True ) else: - day_rule = _parse_arg(dom, dt.day) and _parse_arg( + day_rule = _parse_arg(dom, dt.day, min_value=1) and _parse_arg( dow, 0 if weekday == 7 else weekday, True ) return ( _parse_arg(minute, dt.minute) and _parse_arg(hour, dt.hour) - and _parse_arg(month, dt.month) + and _parse_arg(month, dt.month, min_value=1) and day_rule ) diff --git a/tests/test_dom.py b/tests/test_dom.py index 02a3631..04b0ac8 100644 --- a/tests/test_dom.py +++ b/tests/test_dom.py @@ -13,7 +13,9 @@ def test_dom(self): def run(now): assert pycron.is_now("* * * * *", now) assert pycron.is_now("* * 18 * *", now) - assert pycron.is_now("* * */6 * *", now) + # Steps count from the 1st, so */6 is the 1st, 7th, 13th, 19th... + # and the 18th is not one of those. + assert pycron.is_now("* * */6 * *", now) is False assert pycron.is_now("* * 1,16,18 * *", now) assert pycron.is_now("* * 19 * *", now) is False assert pycron.is_now("* * */4 * *", now) is False @@ -29,3 +31,11 @@ def run(now): run(arrow.get(now)) run(udatetime.from_string(now.isoformat())) run(Delorean(datetime=now, timezone="UTC").datetime) + + def test_dom_step(self): + # */6 is just a shorthand for 1-31/6, both start counting from the 1st. + assert pycron.is_now("* * */6 * *", datetime(2015, 6, 1, 16, 7)) + assert pycron.is_now("* * */6 * *", datetime(2015, 6, 7, 16, 7)) + assert pycron.is_now("* * */6 * *", datetime(2015, 6, 19, 16, 7)) + assert pycron.is_now("* * */6 * *", datetime(2015, 6, 6, 16, 7)) is False + assert pycron.is_now("* * 1-31/6 * *", datetime(2015, 6, 7, 16, 7)) diff --git a/tests/test_month.py b/tests/test_month.py index 76639ec..9db99d0 100644 --- a/tests/test_month.py +++ b/tests/test_month.py @@ -13,10 +13,12 @@ def test_parser(self): def run(now): assert pycron.is_now("* * * * *", now) assert pycron.is_now("* * * 6 *", now) - assert pycron.is_now("* * * */2 *", now) + # Steps count from January, so */2 is Jan, Mar, May... and */5 is + # Jan, Jun, Nov. June matches the second one, not the first. + assert pycron.is_now("* * * */2 *", now) is False assert pycron.is_now("* * * 1,4,6,12 *", now) assert pycron.is_now("* * * 5 *", now) is False - assert pycron.is_now("* * * */5 *", now) is False + assert pycron.is_now("* * * */5 *", now) assert pycron.is_now("* * * 1,4,12 *", now) is False assert pycron.MONTH_CHOICES[now.month - 1][1] == "June" assert pycron.is_now("* * * 5-8 *", now) @@ -29,3 +31,11 @@ def run(now): run(arrow.get(now)) run(udatetime.from_string(now.isoformat())) run(Delorean(datetime=now, timezone="UTC").datetime) + + def test_month_step(self): + # */3 is just a shorthand for 1-12/3, both start counting from January. + assert pycron.is_now("* * * */3 *", datetime(2015, 1, 18, 16, 7)) + assert pycron.is_now("* * * */3 *", datetime(2015, 4, 18, 16, 7)) + assert pycron.is_now("* * * */3 *", datetime(2015, 7, 18, 16, 7)) + assert pycron.is_now("* * * */3 *", datetime(2015, 6, 18, 16, 7)) is False + assert pycron.is_now("* * * 1-12/3 *", datetime(2015, 4, 18, 16, 7))