Skip to content

Commit

Permalink
Tweak retry tests to account for Windows timer's coarse precision
Browse files Browse the repository at this point in the history
Despite my best efforts to account for Windows' coarse timer precision,
the test_retry_wait tests are still randomly failing as it isn't
sleeping for long enough by a small margin:

Example 1: 472.171 - 472.156 >= 0.015 (~1% too low)
Example 2: 554.406 - 554.265 >= 0.15 (~7% too low)

(call_time, test_start_time >= expected sleep duration between retries)

To avoid these random failures, I've adjusted the required sleep
duration to be 10% less which should be enough.

I've also replaced time.monotonic() with time.perf_counter() in the
retry utility and test suite since it's effectively monotonic[^1] while
providing much higher resolution (esp. on Windows).

[^1]: It's not guaranteed to be monotonic in the Python docs, but it is
      _indeed_ monotonic on all platforms we care about.
  • Loading branch information
ichard26 committed Jul 11, 2024
1 parent 6b54c19 commit 578510a
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 10 deletions.
8 changes: 5 additions & 3 deletions src/pip/_internal/utils/retry.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import functools
from time import monotonic, sleep
from time import perf_counter, sleep
from typing import Callable, TypeVar

from pip._vendor.typing_extensions import ParamSpec
Expand All @@ -26,12 +26,14 @@ def wrapper(func: Callable[P, T]) -> Callable[P, T]:

@functools.wraps(func)
def retry_wrapped(*args: P.args, **kwargs: P.kwargs) -> T:
start_time = monotonic()
# The performance counter is monotonic on all platforms we care
# about and has much better resolution than time.monotonic().
start_time = perf_counter()
while True:
try:
return func(*args, **kwargs)
except Exception:
if monotonic() - start_time > stop_after_delay:
if perf_counter() - start_time > stop_after_delay:
raise
sleep(wait)

Expand Down
16 changes: 9 additions & 7 deletions tests/unit/test_utils_retry.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import random
from time import monotonic, sleep
from time import perf_counter, sleep
from typing import List, NoReturn, Tuple, Type
from unittest.mock import Mock

Expand Down Expand Up @@ -65,7 +65,7 @@ def create_timestamped_callable(sleep_per_call: float = 0) -> Tuple[Mock, List[f
timestamps = []

def _raise_error() -> NoReturn:
timestamps.append(monotonic())
timestamps.append(perf_counter())
if sleep_per_call:
sleep(sleep_per_call)
raise RuntimeError
Expand All @@ -79,25 +79,27 @@ def test_retry_wait(wait_duration: float) -> None:
function, timestamps = create_timestamped_callable()
# Only the first retry will be scheduled before the time limit is exceeded.
wrapped = retry(wait=wait_duration, stop_after_delay=0.01)(function)
start_time = monotonic()
start_time = perf_counter()
with pytest.raises(RuntimeError):
wrapped()
assert len(timestamps) == 2
assert timestamps[1] - start_time >= wait_duration
# Add a margin of 10% to permit for unavoidable variation.
assert timestamps[1] - start_time >= (wait_duration * 0.9)


@pytest.mark.parametrize(
"call_duration, max_allowed_calls", [(0.01, 10), (0.04, 3), (0.15, 1)]
"call_duration, max_allowed_calls", [(0.01, 11), (0.04, 3), (0.15, 1)]
)
def test_retry_time_limit(call_duration: float, max_allowed_calls: int) -> None:
function, timestamps = create_timestamped_callable(sleep_per_call=call_duration)
wrapped = retry(wait=0, stop_after_delay=0.1)(function)

start_time = monotonic()
start_time = perf_counter()
with pytest.raises(RuntimeError):
wrapped()
assert len(timestamps) <= max_allowed_calls
assert all(t - start_time <= 0.1 for t in timestamps)
# Add a margin of 10% to permit for unavoidable variation.
assert all(t - start_time <= (0.1 * 0.9) for t in timestamps)


def test_retry_method() -> None:
Expand Down

0 comments on commit 578510a

Please sign in to comment.