diff --git a/CHANGES b/CHANGES index 7a691b0cc..290cf591e 100644 --- a/CHANGES +++ b/CHANGES @@ -45,6 +45,16 @@ $ uvx --from 'libtmux' --prerelease allow python _Notes on the upcoming release will go here._ +### Fixes + +- {func}`~libtmux.test.retry.retry_until` measures its budget with + {func}`time.monotonic` instead of the wall clock, so an NTP correction, a + container clock sync, or a VM resume can no longer cut a wait short or + stretch it (#726) +- {func}`~libtmux.test.retry.retry_until` accepts an `args` tuple forwarded to + the predicate, so waiting on a loop variable no longer needs a closure that + ruff's `B023` rejects and mypy cannot infer (#726) + ### Documentation #### Cleaner `from_env` examples (#719) diff --git a/src/libtmux/test/retry.py b/src/libtmux/test/retry.py index bad491171..7c79328d8 100644 --- a/src/libtmux/test/retry.py +++ b/src/libtmux/test/retry.py @@ -19,10 +19,32 @@ pass +@t.overload def retry_until( fun: Callable[[], bool], + seconds: float = ..., + *, + interval: float = ..., + raises: bool | None = ..., +) -> bool: ... + + +@t.overload +def retry_until( + fun: Callable[..., bool], + seconds: float = ..., + *, + args: tuple[t.Any, ...], + interval: float = ..., + raises: bool | None = ..., +) -> bool: ... + + +def retry_until( + fun: Callable[..., bool], seconds: float = RETRY_TIMEOUT_SECONDS, *, + args: tuple[t.Any, ...] = (), interval: float = RETRY_INTERVAL_SECONDS, raises: bool | None = True, ) -> bool: @@ -33,16 +55,28 @@ def retry_until( ---------- fun : callable A function that will be called repeatedly until it returns ``True`` or - the specified time passes. + the specified time passes. Called with ``args`` unpacked, so it accepts + no arguments unless ``args`` is given. seconds : float Seconds to retry. Defaults to ``8``, which is configurable via ``RETRY_TIMEOUT_SECONDS`` environment variables. + args : tuple + Positional arguments passed to ``fun`` on every call. Pass the subject + being waited on here instead of closing over it, which keeps predicates + written inside a loop free of a loop-variable binding. interval : float Time in seconds to wait between calls. Defaults to ``0.05`` and is configurable via ``RETRY_INTERVAL_SECONDS`` environment variable. raises : bool Whether or not to raise an exception on timeout. Defaults to ``True``. + Notes + ----- + The budget is measured with :func:`time.monotonic`, which only ever moves + forward. A wall clock does not: an NTP correction, a container clock sync, + or a VM resume steps it, and a step landing inside the wait would end it + early or stretch it by the size of the step. + Examples -------- >>> def fn(): @@ -55,11 +89,28 @@ def retry_until( In pytest: >>> assert retry_until(fn, raises=False) + + Waiting on something from a loop takes ``args`` rather than a closure: + + >>> window = session.new_window(window_name='retry_until_args') + >>> panes = [window.active_pane, window.split()] + >>> for pane in panes: + ... pane.send_keys('echo ready') + + Compare whole lines. ``capture_pane`` returns the command tmux echoed onto + the pane, so ``'ready' in line`` matches that echo and is already true + before the shell has run anything: + + >>> for pane in panes: + ... assert retry_until( + ... lambda p: any(line.strip() == 'ready' for line in p.capture_pane()), + ... args=(pane,), + ... ) """ - ini = time.time() + ini = time.monotonic() - while not fun(): - end = time.time() + while not fun(*args): + end = time.monotonic() if end - ini >= seconds: if raises: raise WaitTimeout diff --git a/tests/test/test_retry.py b/tests/test/test_retry.py index c2a0f9255..e45aa50f6 100644 --- a/tests/test/test_retry.py +++ b/tests/test/test_retry.py @@ -2,17 +2,21 @@ from __future__ import annotations -from time import sleep, time +import typing as t +from time import monotonic, sleep import pytest from libtmux import exc from libtmux.test.retry import retry_until +if t.TYPE_CHECKING: + from libtmux.session import Session + def test_retry_three_times() -> None: """Test retry_until().""" - ini = time() + ini = monotonic() value = 0 def call_me_three_times() -> bool: @@ -27,14 +31,14 @@ def call_me_three_times() -> bool: retry_until(call_me_three_times, 1) - end = time() + end = monotonic() assert 0.9 <= (end - ini) <= 1.1 # Allow for small timing variations def test_function_times_out() -> None: """Test time outs with retry_until().""" - ini = time() + ini = monotonic() def never_true() -> bool: sleep( @@ -45,14 +49,14 @@ def never_true() -> bool: with pytest.raises(exc.WaitTimeout): retry_until(never_true, 1) - end = time() + end = monotonic() assert 0.9 <= (end - ini) <= 1.1 # Allow for small timing variations def test_function_times_out_no_raise() -> None: """Tests retry_until() with exception raising disabled.""" - ini = time() + ini = monotonic() def never_true() -> bool: sleep( @@ -62,13 +66,13 @@ def never_true() -> bool: retry_until(never_true, 1, raises=False) - end = time() + end = monotonic() assert 0.9 <= (end - ini) <= 1.1 # Allow for small timing variations def test_function_times_out_no_raise_assert() -> None: """Tests retry_until() with exception raising disabled, returning False.""" - ini = time() + ini = monotonic() def never_true() -> bool: sleep( @@ -78,13 +82,13 @@ def never_true() -> bool: assert not retry_until(never_true, 1, raises=False) - end = time() + end = monotonic() assert 0.9 <= (end - ini) <= 1.1 # Allow for small timing variations def test_retry_three_times_no_raise_assert() -> None: """Tests retry_until() with exception raising disabled, with closure variable.""" - ini = time() + ini = monotonic() value = 0 def call_me_three_times() -> bool: @@ -101,5 +105,68 @@ def call_me_three_times() -> bool: assert retry_until(call_me_three_times, 1, raises=False) - end = time() + end = monotonic() assert 0.9 <= (end - ini) <= 1.1 # Allow for small timing variations + + +def test_retry_until_forwards_args_from_loop(session: Session) -> None: + """Waiting on a loop variable needs no closure, so no B023 suppression.""" + window = session.new_window(window_name="retry_until_args") + active_pane = window.active_pane + assert active_pane is not None + panes = [active_pane, window.split()] + + for pane in panes: + pane.send_keys("echo ready") + + for pane in panes: + # Whole lines, not a substring: capture_pane returns the command tmux + # echoed onto the pane, so ``"ready" in ...`` is true before the shell + # has run and the wait would prove nothing. + assert retry_until( + lambda p: any(line.strip() == "ready" for line in p.capture_pane()), + 2, + args=(pane,), + ) + + +def test_retry_until_args_reach_predicate() -> None: + """Every retry passes the same ``args`` to the predicate.""" + seen: list[tuple[str, int]] = [] + + def ready(name: str, threshold: int) -> bool: + seen.append((name, threshold)) + return len(seen) >= threshold + + assert retry_until(ready, 1, args=("pane", 3), interval=0) + assert seen == [("pane", 3)] * 3 + + +def test_wall_clock_step_does_not_end_the_wait( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """A wall-clock jump mid-wait neither shortens nor lengthens the budget. + + Regression: the budget was measured with :func:`time.time`, so an NTP + correction, a container clock sync, or a VM resume landing inside the + wait ended it early. Stepping :func:`time.time` forward by an hour here + must not be observable, because nothing reads it. + """ + stepped = 0.0 + + def stepping_time() -> float: + nonlocal stepped + stepped += 3600.0 + return monotonic() + stepped + + monkeypatch.setattr("time.time", stepping_time) + + calls = 0 + + def true_on_third() -> bool: + nonlocal calls + calls += 1 + return calls >= 3 + + assert retry_until(true_on_third, 5) + assert calls == 3