Skip to content

Commit c7ed82a

Browse files
committed
test(hygiene[run,sync/git]) Use monkeypatch.setattr; lift import time
why: ``test_run_timeout_reaps_child_process`` swapped ``subprocess.Popen`` via raw attribute assignment guarded by ``try/finally``. That pattern is brittle under ``pytest-xdist`` parallel runs and unnecessary now that pytest's ``monkeypatch`` fixture provides automatic restoration. Separately, ``test_remote_is_fast_for_repos_with_many_refs`` imported ``time`` inside its body where module-scope imports are the project convention. what: - Take the ``monkeypatch: pytest.MonkeyPatch`` fixture and replace the global Popen swap with ``monkeypatch.setattr``; drop the try/finally and both ``# type: ignore`` comments. - Move ``import time`` from inside the test body to the module-top alphabetical import block in ``tests/sync/test_git.py``.
1 parent 4c9dee5 commit c7ed82a

2 files changed

Lines changed: 9 additions & 12 deletions

File tree

tests/_internal/test_run.py

Lines changed: 8 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -85,7 +85,7 @@ def test_run_timeout_captures_partial_stderr_output() -> None:
8585
assert "first" in excinfo.value.output
8686

8787

88-
def test_run_timeout_reaps_child_process() -> None:
88+
def test_run_timeout_reaps_child_process(monkeypatch: pytest.MonkeyPatch) -> None:
8989
"""Timed-out processes are terminated; no zombies remain in the group."""
9090
script = "import time; time.sleep(10)"
9191

@@ -97,15 +97,13 @@ def _capturing_popen(*args: t.Any, **kwargs: t.Any) -> t.Any:
9797
captured["proc"] = proc
9898
return proc
9999

100-
# Attribute replacement keeps the test narrow -- we just need a handle on
101-
# the Popen that ``run`` created so we can assert the child was actually
102-
# reaped rather than abandoned (no zombie left behind).
103-
subprocess.Popen = _capturing_popen # type: ignore[misc,assignment]
104-
try:
105-
with pytest.raises(exc.CommandTimeoutError):
106-
run([sys.executable, "-c", script], timeout=0.3)
107-
finally:
108-
subprocess.Popen = original_popen # type: ignore[misc]
100+
# ``monkeypatch.setattr`` auto-restores even if the test body raises and
101+
# is safe under ``pytest-xdist`` parallel runs, unlike a hand-rolled
102+
# try/finally around a global assignment.
103+
monkeypatch.setattr(subprocess, "Popen", _capturing_popen)
104+
105+
with pytest.raises(exc.CommandTimeoutError):
106+
run([sys.executable, "-c", script], timeout=0.3)
109107

110108
proc = captured["proc"]
111109
# ``returncode`` is only populated once ``wait`` succeeds, so a populated

tests/sync/test_git.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import random
88
import shutil
99
import textwrap
10+
import time
1011
import typing as t
1112
from collections.abc import Callable
1213

@@ -1684,8 +1685,6 @@ def test_remote_is_fast_for_repos_with_many_refs(
16841685
enough to catch the pathological old path (many seconds on WSL2) without
16851686
flaking on slow CI.
16861687
"""
1687-
import time
1688-
16891688
# Point origin at a commit we already have so fake refs don't dangle.
16901689
head_sha = run(["git", "rev-parse", "HEAD"], cwd=git_repo.path).strip()
16911690

0 commit comments

Comments
 (0)