fixed some period() trigger logic and added more tests

Author: craigbarratt

custom_components/pyscript/trigger.py

@@ -391,7 +391,7 @@ def timer_trigger_next(cls, time_spec, now):
             if cron_match:
                 if not croniter.is_valid(cron_match.group("cron_expr")):
                     _LOGGER.error("Invalid cron expression: %s", cron_match)
-                    return None
+                    continue
 
                 val = croniter(cron_match.group("cron_expr"), now, dt.datetime).get_next()
                 if next_time is None or val < next_time:
@@ -408,36 +408,39 @@ def timer_trigger_next(cls, time_spec, now):
                     next_time = this_t
 
             elif len(match2) == 5:
-                start = cls.parse_date_time(match2[1].strip(), 0, now)
-                if match2[3] is not None:
-                    end = cls.parse_date_time(match2[3].strip(), 0, now)
-                    if end < start:
-                        if end <= now:
-                            # try end of tomorrow
-                            end = cls.parse_date_time(match2[3].strip(), 1, now)
-                        else:
-                            # try a start of yesterday
-                            start = cls.parse_date_time(match2[1].strip(), -1, now)
-                if now < start and (next_time is None or start < next_time):
-                    next_time = start
-                period = parse_time_offset(match2[2].strip())
-                if now >= start and period > 0:
-                    secs = period * (1.0 + math.floor((now - start).total_seconds() / period))
-                    this_t = start + dt.timedelta(seconds=secs)
-                    if match2[3] is None:
+                start_str, period_str = match2[1].strip(), match2[2].strip()
+                start = cls.parse_date_time(start_str, 0, now)
+                period = parse_time_offset(period_str)
+                if period <= 0:
+                    _LOGGER.error("Invalid non-positive period %s in period(): %s", period)
+                    continue
+
+                if match2[3] is None:
+                    if now < start and (next_time is None or start < next_time):
+                        next_time = start
+                    if now >= start:
+                        secs = period * (1.0 + math.floor((now - start).total_seconds() / period))
+                        this_t = start + dt.timedelta(seconds=secs)
                         if now < this_t and (next_time is None or this_t < next_time):
                             next_time = this_t
-                    else:
-                        if now < this_t <= end and (next_time is None or this_t < next_time):
+                    continue
+                end_str = match2[3].strip()
+                end = cls.parse_date_time(end_str, 0, now)
+                end_offset = 1 if end < start else 0
+                for day in [-1, 0, 1]:
+                    start = cls.parse_date_time(start_str, day, now)
+                    end = cls.parse_date_time(end_str, day + end_offset, now)
+                    if now < start:
+                        if next_time is None or start < next_time:
+                            next_time = start
+                        break
+                    secs = period * (1.0 + math.floor((now - start).total_seconds() / period))
+                    this_t = start + dt.timedelta(seconds=secs)
+                    if start <= this_t <= end:
+                        if next_time is None or this_t < next_time:
                             next_time = this_t
-                        if next_time is None or now >= end:
-                            #
-                            # Try tomorrow's start (won't make a difference if spec has
-                            # full date)
-                            #
-                            start = cls.parse_date_time(match2[1].strip(), 1, now)
-                            if now < start and (next_time is None or start < next_time):
-                                next_time = start
+                        break
+
             else:
                 _LOGGER.warning("Can't parse %s in time_trigger check", spec)
         return next_time

tests/test_unit_trigger.py

@@ -1,5 +1,7 @@
 """Unit tests for time trigger functions."""
+
 from datetime import datetime as dt
+import locale
 
 from custom_components.pyscript.function import Function
 from custom_components.pyscript.trigger import TrigTime
@@ -55,11 +57,6 @@
     ["+5 min", 0, dt(2019, 9, 1, 0, 5, 0, 0)],
 ]
 
-parseDateTimeTests2 = [
-    ["sunday", 0, dt(2019, 9, 8, 0, 0, 0, 0)],
-    ["monday", 0, dt(2019, 9, 9, 0, 0, 0, 0)],
-]
-
 
 async def test_parse_date_time(hass, caplog):
     """Run time parse datetime tests."""
@@ -73,6 +70,11 @@ async def test_parse_date_time(hass, caplog):
     hass.config.elevation = 0
     hass.config.time_zone = "America/Los_Angeles"
 
+    #
+    # Set English since the week names use locale
+    #
+    locale.setlocale(locale.LC_ALL, "en_US")
+
     Function.init(hass)
     TrigTime.init(hass)
 
@@ -89,14 +91,42 @@ async def test_parse_date_time(hass, caplog):
             out = TrigTime.parse_date_time(spec, date_offset, now)
             assert out == expect
 
+
+parseDateTimeTestsDayNames = [
+    ["thu", 0, dt(2019, 9, 5, 0, 0, 0, 0)],
+    ["fri noon", 0, dt(2019, 9, 6, 12, 0, 0, 0)],
+    ["sunday", 0, dt(2019, 9, 8, 0, 0, 0, 0)],
+    ["monday", 0, dt(2019, 9, 9, 0, 0, 0, 0)],
+]
+
+
+async def test_parse_date_time_day_names(hass, caplog):
+    """Run time parse datetime on day of week names."""
+    #
+    # Hardcode a location and timezone so we can check sunrise
+    # and sunset.
+    #
+    hass.config.latitude = 38
+    hass.config.longitude = -122
+    hass.config.elevation = 0
+    hass.config.time_zone = "America/Los_Angeles"
+
+    #
+    # Set English since the week names use locale
+    #
+    locale.setlocale(locale.LC_ALL, "en_US")
+
+    Function.init(hass)
+    TrigTime.init(hass)
+
     #
     # This set of tests assumes it's currently 13:00 on 2019/9/3
     #
     now = dt(2019, 9, 3, 13, 0, 0, 0)
     with patch("homeassistant.helpers.condition.dt_util.utcnow", return_value=now), patch(
         "homeassistant.util.dt.utcnow", return_value=now
     ):
-        for test_data in parseDateTimeTests2:
+        for test_data in parseDateTimeTestsDayNames:
             spec, date_offset, expect = test_data
             out = TrigTime.parse_date_time(spec, date_offset, now)
             assert out == expect
@@ -148,14 +178,17 @@ async def test_parse_date_time(hass, caplog):
 )
 def test_timer_active_check(hass, spec, now, expected):
     """Run time active check tests."""
+    #
+    # Set English since the week names use locale
+    #
+    locale.setlocale(locale.LC_ALL, "en_US")
     Function.init(hass)
     TrigTime.init(hass)
     out = TrigTime.timer_active_check(spec, now)
     assert out == expected
 
 
 timerTriggerNextTests = [
-    [["period(sunset, 60s, sunrise)"], [dt(2019, 9, 1, 19, 15, 16)]],
     [["once(2019/9/1 8:00)"], [None]],
     [["once(2019/9/1 15:00)"], [dt(2019, 9, 1, 15, 0, 0, 0)]],
     [["once(15:00)"], [dt(2019, 9, 1, 15, 0, 0, 0)]],
@@ -199,6 +232,55 @@ def test_timer_active_check(hass, spec, now, expected):
             dt(2019, 9, 2, 6, 0, 0, 0),
             dt(2019, 9, 2, 18, 0, 0, 0),
             dt(2019, 9, 2, 22, 0, 0, 0),
+            dt(2019, 9, 3, 2, 0, 0, 0),
+            dt(2019, 9, 3, 6, 0, 0, 0),
+            dt(2019, 9, 3, 18, 0, 0, 0),
+        ],
+    ],
+    [
+        ["period(18:00, 12 hr, 6:00)"],
+        [
+            dt(2019, 9, 1, 18, 0, 0, 0),
+            dt(2019, 9, 2, 6, 0, 0, 0),
+            dt(2019, 9, 2, 18, 0, 0, 0),
+            dt(2019, 9, 3, 6, 0, 0, 0),
+        ],
+    ],
+    [
+        ["period(18:00, 12.0001 hr, 6:00)"],
+        [
+            dt(2019, 9, 1, 18, 0, 0, 0),
+            dt(2019, 9, 2, 18, 0, 0, 0),
+            dt(2019, 9, 3, 18, 0, 0, 0),
+            dt(2019, 9, 4, 18, 0, 0, 0),
+        ],
+    ],
+    [
+        ["period(6:00, 12 hr, 18:00)"],
+        [
+            dt(2019, 9, 1, 18, 0, 0, 0),
+            dt(2019, 9, 2, 6, 0, 0, 0),
+            dt(2019, 9, 2, 18, 0, 0, 0),
+            dt(2019, 9, 3, 6, 0, 0, 0),
+            dt(2019, 9, 3, 18, 0, 0, 0),
+            dt(2019, 9, 4, 6, 0, 0, 0),
+        ],
+    ],
+    [
+        ["period(6:00, 12.0001 hr, 18:00)"],
+        [dt(2019, 9, 2, 6, 0, 0, 0), dt(2019, 9, 3, 6, 0, 0, 0), dt(2019, 9, 4, 6, 0, 0, 0)],
+    ],
+    [
+        ["period(sunset, 4 hours, sunrise)"],
+        [
+            dt(2019, 9, 1, 19, 39, 15),
+            dt(2019, 9, 1, 23, 39, 15),
+            dt(2019, 9, 2, 3, 39, 15),
+            dt(2019, 9, 2, 19, 37, 46),
+            dt(2019, 9, 2, 23, 37, 46),
+            dt(2019, 9, 3, 3, 37, 46),
+            dt(2019, 9, 3, 19, 36, 17),
+            dt(2019, 9, 3, 23, 36, 17),
         ],
     ],
     [
@@ -269,8 +351,23 @@ def test_timer_active_check(hass, spec, now, expected):
 
 def test_timer_trigger_next(hass):
     """Run trigger next tests."""
+    #
+    # Hardcode a location and timezone so we can check sunrise
+    # and sunset.
+    #
+    hass.config.latitude = 38
+    hass.config.longitude = -122
+    hass.config.elevation = 0
+    hass.config.time_zone = "America/Los_Angeles"
+    #
+
+    # Set English since the week names use locale
+    #
+    locale.setlocale(locale.LC_ALL, "en_US")
+
     Function.init(hass)
     TrigTime.init(hass)
+
     for test_data in timerTriggerNextTests:
         now = dt(2019, 9, 1, 13, 0, 0, 100000)
         spec, expect_seq = test_data
@@ -377,6 +474,10 @@ def test_timer_trigger_next(hass):
 
 def test_timer_trigger_next_month_rollover(hass):
     """Run month rollover tests."""
+    #
+    # Set English since the week names use locale
+    #
+    locale.setlocale(locale.LC_ALL, "en_US")
     Function.init(hass)
     TrigTime.init(hass)
     for test_data in timerTriggerNextTestsMonthRollover: