Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
104 changes: 104 additions & 0 deletions autils/devel/wait.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
"""Utilities for waiting for conditions to be met.

This module provides utilities for polling functions until they return
a truthy value or a timeout expires, useful for testing and development
scenarios where you need to wait for system state changes.
"""

import logging
import time

LOG = logging.getLogger(__name__)


# pylint: disable=R0913
def wait_for(func, timeout, first=0.0, step=1.0, text=None, args=None, kwargs=None):
"""Wait until a function returns a truthy value or timeout expires.

This function repeatedly calls a given function with optional arguments
until it returns a truthy value (anything that evaluates to True in a
boolean context) or until the specified timeout expires. It provides
configurable delays before the first attempt and between subsequent
attempts, making it useful for polling operations in testing and
development scenarios.

The function uses time.monotonic() for reliable timeout calculation that
is not affected by system clock adjustments. Note that the step sleep
duration is not interrupted when timeout expires, so actual elapsed time
may exceed the specified timeout by up to one step duration.

:param func: Callable to be executed repeatedly until it returns a truthy
value. Can be any callable object (function, lambda, method,
callable class instance).
:type func: callable
:param timeout: Maximum time in seconds to wait for func to return a
truthy value. Must be a non-negative number. If timeout
expires before func returns truthy, None is returned.
:type timeout: float or int
Comment on lines +34 to +37
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Clarify timeout semantics for zero/negative values and alignment with tests.

The docstring states that timeout “Must be a non-negative number”, but the implementation accepts negative values and the unit test test_negative_timeout explicitly exercises timeout=-1 and expects None. For timeout <= 0 (or first > timeout), end_time <= start_time so the loop never runs and func is never called, but this subtle behavior is not clearly documented.

To avoid surprises for callers and future maintainers, I’d suggest rewording the timeout docs to describe the current behavior explicitly instead of implying validation that isn’t performed, for example:

-    :param timeout: Maximum time in seconds to wait for func to return a
-                    truthy value. Must be a non-negative number. If timeout
-                    expires before func returns truthy, None is returned.
+    :param timeout: Maximum time in seconds to wait for func to return a
+                    truthy value. If the timeout has already expired
+                    (for example, a zero or negative timeout, or when the
+                    initial ``first`` delay exceeds the timeout), ``func``
+                    will not be called and ``None`` is returned immediately.
+                    Otherwise, if the timeout expires before ``func`` returns
+                    a truthy value, ``None`` is returned.

(Adjust wording as you prefer, but making the “no call when already expired” behavior explicit would help a lot.)

Also applies to: 86-92

🤖 Prompt for AI Agents
In autils/devel/wait.py around lines 34-37 (and similarly 86-92), the docstring
misleadingly says "Must be a non-negative number" while the implementation
permits negative timeouts and the test suite expects timeout=-1 to result in no
calls and return None; update the docstrings to explicitly state the observed
behavior: timeout is interpreted as seconds (float|int), no validation is
performed, and if timeout <= 0 (or the computed end_time is already <=
start_time) the function will not call func and will return None immediately;
keep wording concise and mention alignment with existing tests.

:param first: Time in seconds to sleep before the first attempt to call
func. Useful when you know the condition won't be met
immediately. Defaults to 0.0 (no initial delay).
:type first: float or int
:param step: Time in seconds to sleep between successive calls to func.
The actual sleep happens after each failed attempt. Defaults
to 1.0 second. Note that this sleep is not interrupted when
timeout expires.
:type step: float or int
:param text: Optional debug message to log before each attempt. When
provided, logs at DEBUG level with elapsed time since start.
If None, no logging occurs. Useful for debugging wait
operations.
:type text: str or None
:param args: Optional list or tuple of positional arguments to pass to
func on each call. If None, defaults to empty list.
:type args: list, tuple, or None
:param kwargs: Optional dictionary of keyword arguments to pass to func on
each call. If None, defaults to empty dict.
:type kwargs: dict or None
:return: The truthy return value from func if it succeeds within timeout,
or None if timeout expires without func returning a truthy value.
The actual return value from func is preserved (e.g., strings,
numbers, lists, objects).
:rtype: Any (return type of func) or None
:raises: Any exception raised by func will be propagated to the caller.
No exception handling is performed on func calls.

Example::

>>> import os
>>> # Wait for a file to exist
>>> wait_for(lambda: os.path.exists("/tmp/myfile"), timeout=30, step=1)
True
>>> # Wait for a counter to reach threshold
>>> counter = [0]
>>> def check(): counter[0] += 1; return counter[0] >= 5
>>> wait_for(check, timeout=10, step=0.5)
True
>>> # Wait with custom function and arguments
>>> def check_value(expected, current):
... return current >= expected
>>> wait_for(check_value, timeout=5, step=0.1, args=[10, 15])
True
>>> # Wait with debug logging
>>> wait_for(lambda: False, timeout=2, step=0.5, text="Waiting for condition")
None
"""
args = args or []
kwargs = kwargs or {}

start_time = time.monotonic()
end_time = start_time + timeout

time.sleep(first)

while time.monotonic() < end_time:
if text:
LOG.debug("%s (%.9f secs)", text, (time.monotonic() - start_time))

output = func(*args, **kwargs)
if output:
return output

time.sleep(step)

Comment on lines +86 to +103
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Critical: Zero/negative timeout prevents any function execution.

With timeout=0 or negative values, the loop condition time.monotonic() < end_time at Line 87 is immediately false, so func is never called even once. This behavior may surprise users who expect at least one attempt, especially if the condition could succeed immediately.

Consider calling func at least once before checking the timeout, or explicitly document this behavior more prominently in the docstring (currently buried in the :param timeout: description).

Example fix to ensure at least one attempt:

-    while time.monotonic() < end_time:
+    while True:
         if text:
             LOG.debug("%s (%.9f secs)", text, (time.monotonic() - start_time))
 
         output = func(*args, **kwargs)
         if output:
             return output
+        
+        if time.monotonic() >= end_time:
+            break
 
         time.sleep(step)

Alternatively, if the current behavior is intentional, add a prominent note at the start of the docstring:

     """Wait until a function returns a truthy value or timeout expires.
+    
+    Note: If timeout is zero or negative, the function will not be called at all.
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
args = args or []
kwargs = kwargs or {}
start_time = time.monotonic()
end_time = start_time + timeout
time.sleep(first)
while time.monotonic() < end_time:
if text:
LOG.debug("%s (%.9f secs)", text, (time.monotonic() - start_time))
output = func(*args, **kwargs)
if output:
return output
time.sleep(step)
args = args or []
kwargs = kwargs or {}
start_time = time.monotonic()
end_time = start_time + timeout
time.sleep(first)
while True:
if text:
LOG.debug("%s (%.9f secs)", text, (time.monotonic() - start_time))
output = func(*args, **kwargs)
if output:
return output
if time.monotonic() >= end_time:
break
time.sleep(step)

return None
4 changes: 4 additions & 0 deletions docs/source/utils.rst
Original file line number Diff line number Diff line change
Expand Up @@ -38,3 +38,7 @@ GDB
Output
------
.. automodule:: autils.devel.output

Wait
----
.. automodule:: autils.devel.wait
17 changes: 17 additions & 0 deletions metadata/devel/wait.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: wait
description: Wait for a function to return a truthy value or timeout

categories:
- Development
maintainers:
- name: Harvey James Lynden
email: hlynden@redhat.com
github_usr_name: harvey0100
supported_platforms:
- CentOS Stream 9
- Fedora 36
- Fedora 37
tests:
- tests/unit/modules/devel/wait.py
- tests/functional/modules/devel/wait.py
remote: false
54 changes: 54 additions & 0 deletions tests/functional/modules/devel/wait.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import os
import threading
import time
import unittest

from autils.devel import wait
from tests.utils import TestCaseTmpDir


class WaitForFunctionalTest(TestCaseTmpDir):
"""Functional tests for wait.wait_for with real-world scenarios."""

def test_condition_becomes_true(self):
"""Test wait_for with condition that eventually becomes true (real I/O)."""
filepath = os.path.join(self.tmpdir.name, "test_file.txt")

# Create file after a delay
def create_file_delayed():
time.sleep(0.3)
with open(filepath, "w", encoding="utf-8") as f:
f.write("test content")

# Start file creation in background
thread = threading.Thread(target=create_file_delayed)
thread.start()

# Wait for file to exist
result = wait.wait_for(
lambda: os.path.exists(filepath),
timeout=2.0,
step=0.1,
text="Waiting for file to appear",
)

thread.join()
self.assertTrue(result)
self.assertTrue(os.path.exists(filepath))

def test_timeout_when_condition_never_true(self):
"""Test that wait_for respects timeout when condition never becomes true."""
filepath = os.path.join(self.tmpdir.name, "nonexistent.txt")

# Wait for a file that will never be created
start = time.time()
result = wait.wait_for(lambda: os.path.exists(filepath), timeout=0.5, step=0.1)
elapsed = time.time() - start

self.assertIsNone(result)
self.assertGreaterEqual(elapsed, 0.5)
self.assertLess(elapsed, 0.7)
Comment on lines +44 to +50
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Upper bound timing assertion may be flaky.

The assertion self.assertLess(elapsed, 0.7) checks that execution completes within 0.2 seconds of the 0.5s timeout. On slower systems or under load (CI environments), this 0.2s margin might be insufficient, leading to intermittent test failures.

Consider either removing the upper bound check or increasing the margin to ~1.0 second to account for system variance.

Apply this diff to increase the margin:

 self.assertIsNone(result)
 self.assertGreaterEqual(elapsed, 0.5)
-self.assertLess(elapsed, 0.7)
+self.assertLess(elapsed, 1.0)  # More lenient for slow systems
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
start = time.time()
result = wait.wait_for(lambda: os.path.exists(filepath), timeout=0.5, step=0.1)
elapsed = time.time() - start
self.assertIsNone(result)
self.assertGreaterEqual(elapsed, 0.5)
self.assertLess(elapsed, 0.7)
start = time.time()
result = wait.wait_for(lambda: os.path.exists(filepath), timeout=0.5, step=0.1)
elapsed = time.time() - start
self.assertIsNone(result)
self.assertGreaterEqual(elapsed, 0.5)
self.assertLess(elapsed, 1.0) # More lenient for slow systems
🤖 Prompt for AI Agents
In tests/functional/modules/devel/wait.py around lines 44 to 50, the upper-bound
timing assertion self.assertLess(elapsed, 0.7) is too tight and can be flaky on
slow/loaded CI; replace that assertion with a larger margin — e.g. change it to
self.assertLess(elapsed, 1.5) (or remove the upper-bound check entirely) so the
test tolerates ~1.0s of extra variance beyond the 0.5s timeout.



if __name__ == "__main__":
unittest.main()
Loading
Loading