Skip to content

Commit 0967d3d

Browse files
committed
test(run,sync/git[timeout]) Reduce flake risk in deadline tests
why: Several tests had wall-clock budgets sized for the 0.5 s SIGTERM grace and 0.1 s selector poll defaults, leaving ~2.2 s of headroom above the nominal timeout. On loaded CI runners (cold Python startup of 0.5-0.7 s on ARM Actions, slow filesystems), that headroom narrows to single digits. Separately, the early-stderr-close test relies on ``os.close(sys.stderr.fileno())`` which has POSIX-specific semantics not portable to Windows, and the 500-ref regression test invoked ``git update-ref`` 500 times -- inflating suite duration without sharpening the assertion. what: - Add a ``fast_timeout_constants`` fixture that monkeypatches ``_TIMEOUT_KILL_GRACE_SECONDS`` and ``_TIMEOUT_POLL_INTERVAL_SECONDS`` to 0.05 s so the wall-clock spread above the nominal deadline stays predictable across runners. - Apply the fixture to every deadline-firing test; tighten the upper-bound assertion in ``test_run_raises_command_timeout_when_deadline_exceeded`` from 2.5 to 2.0 with a comment showing the spread arithmetic. - Skip ``test_run_timeout_handles_early_stderr_close_without_hanging`` on Windows; document the POSIX dependency in the skip reason. - Replace the 500-iteration ``run(['git', 'update-ref', ...])`` loop with a single ``git update-ref --stdin`` call so the test setup costs one fork+exec instead of 500. The behavioural assertion (remote() returns under 5 s with 500 refs) is unchanged.
1 parent 306d69f commit 0967d3d

2 files changed

Lines changed: 59 additions & 20 deletions

File tree

tests/_internal/test_run.py

Lines changed: 43 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -12,9 +12,26 @@
1212
import pytest
1313

1414
from libvcs import exc
15+
from libvcs._internal import run as run_module
1516
from libvcs._internal.run import _normalize_command_args, run
1617

1718

19+
@pytest.fixture
20+
def fast_timeout_constants(monkeypatch: pytest.MonkeyPatch) -> None:
21+
"""Tighten the deadline-loop spread so test wall-clock budgets stay reliable.
22+
23+
The two ``_TIMEOUT_*`` constants in :mod:`libvcs._internal.run` are tuned
24+
for production use (0.5 s SIGTERM grace, 0.1 s selector poll). For tests
25+
that intentionally fire the deadline, those defaults add up to ~0.7 s of
26+
unavoidable wall-clock spread on top of the test's nominal timeout, which
27+
makes upper-bound assertions fragile on loaded CI runners. This fixture
28+
monkeypatches both to 0.05 s so the spread stays predictable; production
29+
behaviour is unchanged.
30+
"""
31+
monkeypatch.setattr(run_module, "_TIMEOUT_KILL_GRACE_SECONDS", 0.05)
32+
monkeypatch.setattr(run_module, "_TIMEOUT_POLL_INTERVAL_SECONDS", 0.05)
33+
34+
1835
def test_normalize_command_args_keeps_scalar_string() -> None:
1936
"""Scalar strings should remain a single subprocess argument."""
2037
assert _normalize_command_args("status") == ["status"]
@@ -46,7 +63,9 @@ def test_run_without_timeout_matches_legacy_behavior() -> None:
4663
assert "world" in output
4764

4865

49-
def test_run_raises_command_timeout_when_deadline_exceeded() -> None:
66+
def test_run_raises_command_timeout_when_deadline_exceeded(
67+
fast_timeout_constants: None,
68+
) -> None:
5069
"""A command sleeping past ``timeout`` raises ``CommandTimeoutError`` fast."""
5170
started = time.monotonic()
5271

@@ -59,12 +78,17 @@ def test_run_raises_command_timeout_when_deadline_exceeded() -> None:
5978
elapsed = time.monotonic() - started
6079

6180
# Upper bound keeps the test honest: the deadline must actually fire, not
62-
# fall through to the legacy unbounded wait.
63-
assert elapsed < 2.5, f"timeout took too long: {elapsed:.2f}s"
81+
# fall through to the legacy unbounded wait. With the fast-constants
82+
# fixture, the worst-case spread is 0.3 (deadline) + 0.05 (SIGTERM grace)
83+
# + 0.05 (poll) + Python startup (~0.5 s on cold CI) -> ~1 s; the budget
84+
# below leaves comfortable headroom for slow runners.
85+
assert elapsed < 2.0, f"timeout took too long: {elapsed:.2f}s"
6486
assert isinstance(excinfo.value, exc.CommandError)
6587

6688

67-
def test_run_timeout_captures_partial_stderr_output() -> None:
89+
def test_run_timeout_captures_partial_stderr_output(
90+
fast_timeout_constants: None,
91+
) -> None:
6892
"""Output produced before the timeout is preserved on ``CommandTimeoutError``."""
6993
script = (
7094
"import sys, time;"
@@ -86,7 +110,10 @@ def test_run_timeout_captures_partial_stderr_output() -> None:
86110
assert "first" in excinfo.value.output
87111

88112

89-
def test_run_timeout_reaps_child_process(monkeypatch: pytest.MonkeyPatch) -> None:
113+
def test_run_timeout_reaps_child_process(
114+
monkeypatch: pytest.MonkeyPatch,
115+
fast_timeout_constants: None,
116+
) -> None:
90117
"""Timed-out processes are terminated; no zombies remain in the group."""
91118
script = "import time; time.sleep(10)"
92119

@@ -151,7 +178,9 @@ def test_command_timeout_error_without_timeout_falls_back() -> None:
151178
assert "partial" in rendered
152179

153180

154-
def test_run_timeout_message_includes_duration() -> None:
181+
def test_run_timeout_message_includes_duration(
182+
fast_timeout_constants: None,
183+
) -> None:
155184
"""``run(..., timeout=X)`` raises an exception whose ``str()`` shows X."""
156185
with pytest.raises(exc.CommandTimeoutError) as excinfo:
157186
run([sys.executable, "-c", "import time; time.sleep(10)"], timeout=0.3)
@@ -186,7 +215,9 @@ def test_run_timeout_preserves_stdout_after_exit() -> None:
186215
assert "lines" in output
187216

188217

189-
def test_run_timeout_captures_partial_stdout_on_timeout() -> None:
218+
def test_run_timeout_captures_partial_stdout_on_timeout(
219+
fast_timeout_constants: None,
220+
) -> None:
190221
"""Stdout flushed before the deadline appears in ``CommandTimeoutError.output``."""
191222
script = (
192223
"import sys, time; "
@@ -203,6 +234,7 @@ def test_run_timeout_captures_partial_stdout_on_timeout() -> None:
203234

204235
def test_run_timeout_logs_deadline_exceeded(
205236
caplog: pytest.LogCaptureFixture,
237+
fast_timeout_constants: None,
206238
) -> None:
207239
"""Deadline-fired path emits a WARNING with the canonical ``vcs_cmd`` extra."""
208240
with (
@@ -224,6 +256,10 @@ def test_run_timeout_logs_deadline_exceeded(
224256
assert "time.sleep" in cmd_extra
225257

226258

259+
@pytest.mark.skipif(
260+
sys.platform == "win32",
261+
reason="os.close(sys.stderr.fileno()) has POSIX-specific semantics",
262+
)
227263
def test_run_timeout_handles_early_stderr_close_without_hanging() -> None:
228264
"""A child that closes stderr long before exiting must not stall the loop.
229265

tests/sync/test_git.py

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
import pathlib
77
import random
88
import shutil
9+
import subprocess
910
import textwrap
1011
import time
1112
import typing as t
@@ -1683,23 +1684,25 @@ def test_remote_is_fast_for_repos_with_many_refs(
16831684
This test seeds synthetic refs/remotes/origin/branch-N entries and asserts
16841685
that ``remote('origin')`` returns promptly. A generous 5-second budget is
16851686
enough to catch the pathological old path (many seconds on WSL2) without
1686-
flaking on slow CI.
1687+
flaking on slow CI. Refs are batched through ``git update-ref --stdin``
1688+
(one subprocess) instead of one invocation per ref so the test setup
1689+
doesn't dominate suite duration.
16871690
"""
16881691
# Point origin at a commit we already have so fake refs don't dangle.
16891692
head_sha = run(["git", "rev-parse", "HEAD"], cwd=git_repo.path).strip()
16901693

1691-
# Seed 500 fake remote-tracking refs. ``git update-ref`` is quick but the
1692-
# cumulative count is what would blow up ``git remote show -n``.
1693-
for i in range(500):
1694-
run(
1695-
[
1696-
"git",
1697-
"update-ref",
1698-
f"refs/remotes/origin/fake-branch-{i:04d}",
1699-
head_sha,
1700-
],
1701-
cwd=git_repo.path,
1702-
)
1694+
# Seed 500 fake remote-tracking refs in a single ``git update-ref`` call;
1695+
# the cumulative count is what would blow up ``git remote show -n``.
1696+
stdin_input = "".join(
1697+
f"update refs/remotes/origin/fake-branch-{i:04d} {head_sha}\n"
1698+
for i in range(500)
1699+
)
1700+
subprocess.run(
1701+
["git", "-C", str(git_repo.path), "update-ref", "--stdin"],
1702+
input=stdin_input,
1703+
text=True,
1704+
check=True,
1705+
)
17031706

17041707
started = time.monotonic()
17051708
remote = git_repo.remote("origin")

0 commit comments

Comments
 (0)