Add more tests to the PeriodicTimer

We use the hypothesis library to test policies more thoroughly and add
some more tests for timer construction and most notably a test for a
timer using the `SkipMissedAndResync` policy.

This commit also include some minor naming and documention improvements
in existing tests.

Signed-off-by: Leandro Lucarella <[email protected]>

Author: llucax

noxfile.py

@@ -35,6 +35,7 @@ def pylint(session: nox.Session) -> None:
         "pytest",
         "nox",
         "async-solipsism",
+        "hypothesis",
     )
     session.run("pylint", *check_dirs, *check_files)
 
@@ -49,6 +50,7 @@ def mypy(session: nox.Session) -> None:
         "nox",
         "mypy",
         "async-solipsism",
+        "hypothesis",
     )
 
     common_args = [
@@ -92,6 +94,7 @@ def pytest(session: nox.Session) -> None:
         "pytest-mock",
         "pytest-asyncio",
         "async-solipsism",
+        "hypothesis",
     )
     session.install("-e", ".")
     session.run(

tests/utils/test_periodic_timer.py

@@ -11,9 +11,16 @@
 from datetime import timedelta
 
 import async_solipsism
+import hypothesis
 import pytest
+from hypothesis import strategies as st
 
-from frequenz.channels.util import PeriodicTimer, SkipMissedAndDrift, TriggerAllMissed
+from frequenz.channels.util import (
+    PeriodicTimer,
+    SkipMissedAndDrift,
+    SkipMissedAndResync,
+    TriggerAllMissed,
+)
 
 
 # Setting 'autouse' has no effect as this method replaces the event loop for all tests in the file.
@@ -25,7 +32,187 @@ def event_loop() -> Iterator[async_solipsism.EventLoop]:
     loop.close()
 
 
-async def test_contruction() -> None:
+_max_timedelta_microseconds = (
+    int(
+        timedelta.max.total_seconds() * 1_000_000,
+    )
+    - 1
+)
+
+_min_timedelta_microseconds = (
+    int(
+        timedelta.min.total_seconds() * 1_000_000,
+    )
+    + 1
+)
+
+_calculate_next_tick_time_args = {
+    "now": st.integers(),
+    "scheduled_tick_time": st.integers(),
+    "interval": st.integers(min_value=1, max_value=_max_timedelta_microseconds),
+}
+
+
+def _assert_tick_is_aligned(
+    next_tick_time: int, now: int, scheduled_tick_time: int, interval: int
+) -> None:
+    # Can be equals if scheduled_tick_time == now
+    assert next_tick_time <= now + interval
+    assert (next_tick_time - scheduled_tick_time) % interval == pytest.approx(0.0)
+
+
+@hypothesis.given(**_calculate_next_tick_time_args)
+def test_policy_trigger_all_missed(
+    now: int, scheduled_tick_time: int, interval: int
+) -> None:
+    """Test the TriggerAllMissed policy."""
+    hypothesis.assume(now >= scheduled_tick_time)
+    assert (
+        TriggerAllMissed().calculate_next_tick_time(
+            now=now, interval=interval, scheduled_tick_time=scheduled_tick_time
+        )
+        == scheduled_tick_time + interval
+    )
+
+
+def test_policy_trigger_all_missed_examples() -> None:
+    """Test the TriggerAllMissed policy with minimal examples.
+
+    This is just a sanity check to make sure we are not missing to test any important
+    properties with the hypothesis tests.
+    """
+    policy = TriggerAllMissed()
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_200_000, scheduled_tick_time=9_000_000, interval=1_000_000
+        )
+        == 10_000_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_000_000, scheduled_tick_time=9_000_000, interval=1_000_000
+        )
+        == 10_000_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_500_000, scheduled_tick_time=1_000_000, interval=1_000_000
+        )
+        == 2_000_000
+    )
+
+
+@hypothesis.given(**_calculate_next_tick_time_args)
+def test_policy_skip_missed_and_resync(
+    now: int, scheduled_tick_time: int, interval: int
+) -> None:
+    """Test the SkipMissedAndResync policy."""
+    hypothesis.assume(now >= scheduled_tick_time)
+
+    next_tick_time = SkipMissedAndResync().calculate_next_tick_time(
+        now=now, interval=interval, scheduled_tick_time=scheduled_tick_time
+    )
+    assert next_tick_time > now
+    _assert_tick_is_aligned(next_tick_time, now, scheduled_tick_time, interval)
+
+
+def test_policy_skip_missed_and_resync_examples() -> None:
+    """Test the SkipMissedAndResync policy with minimal examples.
+
+    This is just a sanity check to make sure we are not missing to test any important
+    properties with the hypothesis tests.
+    """
+    policy = SkipMissedAndResync()
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_200_000, scheduled_tick_time=9_000_000, interval=1_000_000
+        )
+        == 11_000_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_000_000, scheduled_tick_time=9_000_000, interval=1_000_000
+        )
+        == 11_000_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_500_000, scheduled_tick_time=1_000_000, interval=1_000_000
+        )
+        == 11_000_000
+    )
+
+
+@hypothesis.given(
+    tolerance=st.integers(min_value=_min_timedelta_microseconds, max_value=-1)
+)
+def test_policy_skip_missed_and_drift_invalid_tolerance(tolerance: int) -> None:
+    """Test the SkipMissedAndDrift policy raises an error for invalid tolerances."""
+    with pytest.raises(ValueError, match="delay_tolerance must be positive"):
+        SkipMissedAndDrift(delay_tolerance=timedelta(microseconds=tolerance))
+
+
+@hypothesis.given(
+    tolerance=st.integers(min_value=0, max_value=_max_timedelta_microseconds),
+    **_calculate_next_tick_time_args,
+)
+def test_policy_skip_missed_and_drift(
+    tolerance: int, now: int, scheduled_tick_time: int, interval: int
+) -> None:
+    """Test the SkipMissedAndDrift policy."""
+    hypothesis.assume(now >= scheduled_tick_time)
+
+    next_tick_time = SkipMissedAndDrift(
+        delay_tolerance=timedelta(microseconds=tolerance)
+    ).calculate_next_tick_time(
+        now=now, interval=interval, scheduled_tick_time=scheduled_tick_time
+    )
+    if tolerance <= interval:
+        assert next_tick_time > now
+    drift = now - scheduled_tick_time
+    if drift > tolerance:
+        assert next_tick_time == now + interval
+    else:
+        _assert_tick_is_aligned(next_tick_time, now, scheduled_tick_time, interval)
+
+
+def test_policy_skip_missed_and_drift_examples() -> None:
+    """Test the SkipMissedAndDrift policy with minimal examples.
+
+    This is just a sanity check to make sure we are not missing to test any important
+    properties with the hypothesis tests.
+    """
+    tolerance = 100_000
+    policy = SkipMissedAndDrift(delay_tolerance=timedelta(microseconds=tolerance))
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_200_000, scheduled_tick_time=9_000_000, interval=1_000_000
+        )
+        == 11_200_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_000_000, scheduled_tick_time=9_000_000, interval=1_000_000
+        )
+        == 11_000_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_500_000, scheduled_tick_time=1_000_000, interval=1_000_000
+        )
+        == 11_500_000
+    )
+    assert (
+        policy.calculate_next_tick_time(
+            now=10_000_000 + tolerance,
+            scheduled_tick_time=10_000_000,
+            interval=1_000_000,
+        )
+        == 11_000_000
+    )
+
+
+async def test_timer_contruction_defaults() -> None:
     """Test the construction of a periodic timer with default values."""
     timer = PeriodicTimer(timedelta(seconds=1.0))
     assert timer.interval == timedelta(seconds=1.0)
@@ -34,7 +221,23 @@ async def test_contruction() -> None:
     assert timer.is_running is True
 
 
-async def test_contruction_auto_start() -> None:
+def test_timer_contruction_no_async() -> None:
+    """Test the construction outside of async (using a custom loop)."""
+    loop = async_solipsism.EventLoop()
+    timer = PeriodicTimer(timedelta(seconds=1.0), loop=loop)
+    assert timer.interval == timedelta(seconds=1.0)
+    assert isinstance(timer.missed_tick_policy, TriggerAllMissed)
+    assert timer.loop is loop
+    assert timer.is_running is True
+
+
+def test_timer_contruction_no_event_loop() -> None:
+    """Test the construction outside of async (without a custom loop) fails."""
+    with pytest.raises(RuntimeError, match="no running event loop"):
+        PeriodicTimer(timedelta(seconds=1.0))
+
+
+async def test_timer_contruction_auto_start() -> None:
     """Test the construction of a periodic timer with auto_start=False."""
     policy = TriggerAllMissed()
     timer = PeriodicTimer(
@@ -49,7 +252,22 @@ async def test_contruction_auto_start() -> None:
     assert timer.is_running is False
 
 
-async def test_autostart(
+async def test_timer_contruction_custom_args() -> None:
+    """Test the construction of a periodic timer with custom arguments."""
+    policy = TriggerAllMissed()
+    timer = PeriodicTimer(
+        timedelta(seconds=5.0),
+        auto_start=True,
+        missed_tick_policy=policy,
+        loop=None,
+    )
+    assert timer.interval == timedelta(seconds=5.0)
+    assert timer.missed_tick_policy is policy
+    assert timer.loop is asyncio.get_running_loop()
+    assert timer.is_running is True
+
+
+async def test_timer_autostart(
     event_loop: async_solipsism.EventLoop,  # pylint: disable=redefined-outer-name
 ) -> None:
     """Test the autostart of a periodic timer."""
@@ -72,7 +290,7 @@ class _StartMethod(enum.Enum):
 
 
 @pytest.mark.parametrize("start_method", list(_StartMethod))
-async def test_no_autostart(
+async def test_timer_no_autostart(
     start_method: _StartMethod,
     event_loop: async_solipsism.EventLoop,  # pylint: disable=redefined-outer-name
 ) -> None:
@@ -106,7 +324,7 @@ async def test_no_autostart(
     assert event_loop.time() == pytest.approx(1.5)
 
 
-async def test_trigger_all(
+async def test_timer_trigger_all_missed(
     event_loop: async_solipsism.EventLoop,  # pylint: disable=redefined-outer-name
 ) -> None:
     """Test the trigger all missed behavior."""
@@ -167,10 +385,63 @@ async def test_trigger_all(
     assert drift == pytest.approx(timedelta(seconds=0.0))
 
 
-async def test_skip_and_drift(
+async def test_timer_skip_missed_and_resync(
+    event_loop: async_solipsism.EventLoop,  # pylint: disable=redefined-outer-name
+) -> None:
+    """Test a timer using the SkipMissedAndResync policy."""
+    interval = 1.0
+    timer = PeriodicTimer(
+        timedelta(seconds=interval), missed_tick_policy=SkipMissedAndResync()
+    )
+
+    # We let the first tick be triggered on time
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval)
+    assert drift == pytest.approx(timedelta(seconds=0.0))
+
+    # Now we let the time pass interval plus a bit more, so we should get
+    # a drift, but the next tick should be triggered still at a multiple of the
+    # interval because we are using TRIGGER_ALL.
+    await asyncio.sleep(interval + 0.1)
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 2 + 0.1)
+    assert drift == pytest.approx(timedelta(seconds=0.1))
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 3)
+    assert drift == pytest.approx(timedelta(seconds=0.0))
+
+    # Now we let the time pass by two times the interval, so we should get
+    # a drift of a whole interval and then next tick should an interval later,
+    # as the delayed tick will be skipped and the timer will resync.
+    await asyncio.sleep(2 * interval)
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 5)
+    assert drift == pytest.approx(timedelta(seconds=interval))
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 6)
+    assert drift == pytest.approx(timedelta(seconds=0.0))
+
+    # Finally we let the time pass by 5 times the interval plus some extra
+    # delay. The timer should fire immediately once with a drift of 4 intervals
+    # plus the extra delay, and then it should resync and fire again with no
+    # drift, skipping the missed ticks.
+    extra_delay = 0.8
+    await asyncio.sleep(5 * interval + extra_delay)
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 11 + extra_delay)
+    assert drift == pytest.approx(timedelta(seconds=interval * 4 + extra_delay))
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 12)
+    assert drift == pytest.approx(timedelta(seconds=0.0))
+    drift = await timer.receive()
+    assert event_loop.time() == pytest.approx(interval * 13)
+    assert drift == pytest.approx(timedelta(seconds=0.0))
+
+
+async def test_timer_skip_missed_and_drift(
     event_loop: async_solipsism.EventLoop,  # pylint: disable=redefined-outer-name
 ) -> None:
-    """Test the skip missed and drift behavior."""
+    """Test a timer using the SkipMissedAndDrift policy."""
     interval = 1.0
     tolerance = 0.1
     timer = PeriodicTimer(