Repository: tmux-python/libtmux (language: python)

Issue:
`retry(seconds=...)` broke
Instead

```
import time
t = time.process_time()

while (time.process_time() - t) * 1000 < 2:
    # stuff happens
    time.sleep(0.1)
```

Find cross platform way to do timeout with signals

See also https://github.com/tmux-python/tmuxp/issues/620


Discussion / hints:
It turns out that this is the cause of https://github.com/tmux-python/tmuxp/issues/704#issuecomment-1128288348 . The current implementation of the `retry()` function is equivalent to `return True`.

Why is this function necessary? Maybe it can be replaced by a `sleep(some_millis)` in most cases. Seems strange needing to retry a block of code thousands of times until a condition meets. That makes me think (and taking a look at the places where it is used) that it can actually be accurately replaced by a `while` loop with the condition that breaks (which in fact would be equivalent since current implementation is a `return True`).
Ok, I just found a case where a piece of code was tried 89 times before it succeeded, which was about 300 ms.
I came up with this function:

```python
def retry(fun, secs=1):
    ini = time.time()

    while not fun():
        end = time.time()
        if end - ini >= secs:
            raise Exception('retry timed out')
```

which does the retrying thing accurately with the timeout, it is more or less clean to use (little change to the code) and reports when the retrying timed out:

```python
    def f():
        session.server._update_windows()
        return w.name == "sh"

    retry(f)

    assert w.name == "sh"
```

what do you think? Of course it can read the timeout from the environment just like the code does right now
@categulario I'm still working and won't be able to focus on this in earnest until the weekend, but here's what i have to offer (to make it easier, maybe):

```python
class WaitTimeout(Exception):
    pass


class WaitConditionCallbackProtocol(typing.Protocol):
    def __call__(self: Generic) -> bool:
        ...


def wait_for_condition(
    self,
    callback: WaitConditionCallbackProtocol,
    wait_time=0.5,
    interval=0.25,
    raises=False,
):
    if not callable(callback):
        raise TypeError("callback should be callable")
    time_spent = 0
    condition = False
    while condition:
        self.server.update_panes()
        if time_spent >= wait_time:
            if raises:
                raise WaitTimeout(f"Condition timed out after {time_spent}")
            break
        condition = callback(self)
        if condition is True:
            return True
        time.sleep(interval)
        time_spent += interval
    return False


class WaitableMixin:
    wait_for_condition = wait_for_condition


# In window.py (example):
class Window(..., WaitableMixin):
    ...

# In pane.py (example):
class Pane(..., WaitableMixin):
    ...
```

Usage:

```
>>> window.wait_for_condition(lambda w: w.window_name == "test")
raises WaitTimeout

>>> window.wait_for_condition(lambda w: w.window_name == "test", raises=False)
False

>>> window.wait_for_condition(lambda w: w.window_name == "test", timeout=5, interval=1)
After 5 seconds of checking every 1 second
```
For an instant I thought this issue was about the `libtmux.test` function only, which is used exactly 10 times in testing code.

I understand that its usage can be extended to `tmuxp` code for cases like https://github.com/tmux-python/tmuxp/issues/96 and that probably motivates this code. However I see a few shortcomings:

* If it is available only as a mixin then using it requires using it from the class that implements the mixin. It might be that implementing it in Window and Pane is enough, it might be not. To me seems restrictive for no good reason and if it ever needs to be used when no window or pane is available then it leads its users to doing some weird tricks to have the function. Function that can very easily be just a function attached to nothing. 
* I think the calculation of time spent should be done with the `time` module, just because operations (like calling the callback) take time, so for the sake of unnecessary precision I would just call `time()` as in my proposal.
* I think the function should rise by default, and be explicitly silenced. A condition that didn't meet after the given time is probably an error anyways. Knowing the reason of the error by default seems reasonable.
* the while loop never enters, but that's probably a typo.

Things that I like:

* the `WaitTimeout` error
* the check for callable argument
* sleeping a small interval between calls, although it should probably be around `.1` second or less to have a more "real-time" effect. Waiting for 250 ms and failing to meet the condition implies the user waiting for half a second, which starts to feel like lag.
@categulario Would you like to make a new PR with a wait function, using the `time` module?

if not i can get around to it in this weekend or next weekend

i may not be around in the weekdays as much due to normal work deadlines
> @categulario Would you like to make a new PR with a wait function w/o the time function, using the time module?

Sorry, I didn't understand that. Got confused between "with a wait function" and "without the time function"
@categulario I edited it. I'm just asking if you want to make PR with an example of what you meant [here](https://github.com/tmux-python/libtmux/issues/368#issuecomment-1129427034)
Just updated #372 with that

Fix patch:
diff --git a/CHANGES b/CHANGES
index da08e4fe8..21ef7fc33 100644
--- a/CHANGES
+++ b/CHANGES
@@ -8,7 +8,7 @@ To install the unreleased libtmux version, see [developmental releases](https://
 $ pip install --user --upgrade --pre libtmux
 ```
 
-## libtmux current (unreleased)
+## libtmux 0.12.x (unreleased)
 
 - _Insert changes/features/fixes for next release here_
 
@@ -22,6 +22,11 @@ $ pip install --user --upgrade --pre libtmux
 
 - Try out sphinx-autoapi for its table of contents generation ({issue}`367`)
 
+### Testing
+
+- `retry()`: Add deprecation warning. This will be removed in 0.13.x ({issue}`368`, {issue}`372`)
+- New function `retry_until()`: Polls a callback function for a set period of time until it returns `True` or times out. By default it will raise {exc}`libtmux.exc.WaitTimeout`, with `raises=False` it will return `False`. Thank you @categulario! ({issue}`368`, {issue}`372`)
+
 ## libtmux 0.11.0 (2022-03-10)
 
 ### Compatibility
diff --git a/docs/api.md b/docs/api.md
index 11ab33040..8fcdc2499 100644
--- a/docs/api.md
+++ b/docs/api.md
@@ -170,6 +170,10 @@ versions.
 .. automethod:: libtmux.test.retry
 ```
 
+```{eval-rst}
+.. automethod:: libtmux.test.retry_until
+```
+
 ```{eval-rst}
 .. automethod:: libtmux.test.get_test_session_name
 ```
diff --git a/libtmux/exc.py b/libtmux/exc.py
index 541bcbca2..e76c41263 100644
--- a/libtmux/exc.py
+++ b/libtmux/exc.py
@@ -49,3 +49,8 @@ class InvalidOption(OptionError):
 class AmbiguousOption(OptionError):
 
     """Option that could potentially match more than one."""
+
+
+class WaitTimeout(LibTmuxException):
+
+    """Function timed out without meeting condition"""


Test patch:
diff --git a/libtmux/test.py b/libtmux/test.py
index 73f10eb83..ad5494e2b 100644
--- a/libtmux/test.py
+++ b/libtmux/test.py
@@ -4,11 +4,16 @@
 import os
 import tempfile
 import time
+import warnings
+from typing import Callable, Optional
+
+from .exc import WaitTimeout
 
 logger = logging.getLogger(__name__)
 
 TEST_SESSION_PREFIX = "libtmux_"
 RETRY_TIMEOUT_SECONDS = int(os.getenv("RETRY_TIMEOUT_SECONDS", 8))
+RETRY_INTERVAL_SECONDS = float(os.getenv("RETRY_INTERVAL_SECONDS", 0.05))
 
 namer = tempfile._RandomNameSequence()
 current_dir = os.path.abspath(os.path.dirname(__file__))
@@ -16,13 +21,17 @@
 fixtures_dir = os.path.realpath(os.path.join(current_dir, "fixtures"))
 
 
-def retry(seconds=RETRY_TIMEOUT_SECONDS):
+def retry(seconds: Optional[float] = RETRY_TIMEOUT_SECONDS) -> bool:
     """
     Retry a block of code until a time limit or ``break``.
 
+    .. deprecated:: 0.12.0
+          `retry` doesn't work, it will be removed in libtmux 0.13.0, it is replaced by
+          `retry_until`, more info: https://github.com/tmux-python/libtmux/issues/368.
+
     Parameters
     ----------
-    seconds : int
+    seconds : float
         Seconds to retry, defaults to ``RETRY_TIMEOUT_SECONDS``, which is
         configurable via environmental variables.
 
@@ -40,9 +49,64 @@ def retry(seconds=RETRY_TIMEOUT_SECONDS):
     ...     if p.current_path == pane_path:
     ...         break
     """
+    warnings.warn(
+        "retry() is being deprecated and will soon be replaced by retry_until()",
+        DeprecationWarning,
+    )
     return (lambda: time.time() < time.time() + seconds)()
 
 
+def retry_until(
+    fun: Callable,
+    seconds: float = RETRY_TIMEOUT_SECONDS,
+    *,
+    interval: Optional[float] = RETRY_INTERVAL_SECONDS,
+    raises: Optional[bool] = True,
+) -> bool:
+    """
+    Retry a function until a condition meets or the specified time passes.
+
+    Parameters
+    ----------
+    fun : callable
+        A function that will be called repeatedly until it returns ``True``  or
+        the specified time passes.
+    seconds : float
+        Seconds to retry. Defaults to ``8``, which is configurable via
+        ``RETRY_TIMEOUT_SECONDS`` environment variables.
+    interval : float
+        Time in seconds to wait between calls. Defaults to ``0.05`` and is
+        configurable via ``RETRY_INTERVAL_SECONDS`` environment variable.
+    raises : bool
+        Wether or not to raise an exception on timeout. Defaults to ``True``.
+
+    Examples
+    --------
+
+    >>> def f():
+    ...     p = w.attached_pane
+    ...     p.server._update_panes()
+    ...     return p.current_path == pane_path
+    ...
+    ... retry(f)
+
+    In pytest:
+
+    >>> assert retry(f, raises=False)
+    """
+    ini = time.time()
+
+    while not fun():
+        end = time.time()
+        if end - ini >= seconds:
+            if raises:
+                raise WaitTimeout()
+            else:
+                return False
+        time.sleep(interval)
+    return True
+
+
 def get_test_session_name(server, prefix=TEST_SESSION_PREFIX):
     """
     Faker to create a session name that doesn't exist.
diff --git a/tests/test_test.py b/tests/test_test.py
new file mode 100644
index 000000000..e98bd8771
--- /dev/null
+++ b/tests/test_test.py
@@ -0,0 +1,87 @@
+from time import time
+
+import pytest
+
+from libtmux.test import WaitTimeout, retry_until
+
+
+def test_retry_three_times():
+    ini = time()
+    value = 0
+
+    def call_me_three_times():
+        nonlocal value
+
+        if value == 2:
+            return True
+
+        value += 1
+
+        return False
+
+    retry_until(call_me_three_times, 1)
+
+    end = time()
+
+    assert abs((end - ini) - 0.1) < 0.01
+
+
+def test_function_times_out():
+    ini = time()
+
+    def never_true():
+        return False
+
+    with pytest.raises(WaitTimeout):
+        retry_until(never_true, 1)
+
+    end = time()
+
+    assert abs((end - ini) - 1.0) < 0.01
+
+
+def test_function_times_out_no_rise():
+    ini = time()
+
+    def never_true():
+        return False
+
+    retry_until(never_true, 1, raises=False)
+
+    end = time()
+
+    assert abs((end - ini) - 1.0) < 0.01
+
+
+def test_function_times_out_no_raise_assert():
+    ini = time()
+
+    def never_true():
+        return False
+
+    assert not retry_until(never_true, 1, raises=False)
+
+    end = time()
+
+    assert abs((end - ini) - 1.0) < 0.01
+
+
+def test_retry_three_times_no_raise_assert():
+    ini = time()
+    value = 0
+
+    def call_me_three_times():
+        nonlocal value
+
+        if value == 2:
+            return True
+
+        value += 1
+
+        return False
+
+    assert retry_until(call_me_three_times, 1, raises=False)
+
+    end = time()
+
+    assert abs((end - ini) - 0.1) < 0.01