Skip to content

Commit 60a425c

Browse files
committed
chore: add exploratory test for timeout
1 parent 61e881f commit 60a425c

File tree

3 files changed

+31
-14
lines changed

3 files changed

+31
-14
lines changed

pyproject.toml

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,9 @@ addopts = """\
9696
--cov-report=xml
9797
"""
9898
pythonpath = [ "src" ]
99+
markers = [
100+
"exploratory: local-only exploratory tests; skipped by default (run with --run-exploratory)",
101+
]
99102

100103
[tool.coverage.run]
101104
branch = true

src/py_app_dev/core/subprocess.py

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -119,8 +119,6 @@ def execute(self, handle_errors: bool = True) -> subprocess.CompletedProcess[Any
119119
time.sleep(0.01)
120120
# TIMEOUT CHECK
121121
if self.timeout is not None and (time.monotonic() - start_time) > self.timeout:
122-
self.logger.error(f"Process timed out after {self.timeout} seconds. Terminating process tree (PID={process.pid}).")
123-
self._terminate_process_tree(process.pid)
124122
raise subprocess.TimeoutExpired(
125123
cmd=self.command_str,
126124
timeout=self.timeout,
@@ -130,20 +128,10 @@ def execute(self, handle_errors: bool = True) -> subprocess.CompletedProcess[Any
130128
process.wait()
131129
# NON-STREAMING MODE (communicate with native timeout)
132130
elif self.capture_output and not self.print_output:
133-
try:
134-
stdout, stderr = process.communicate(timeout=self.timeout)
135-
except subprocess.TimeoutExpired:
136-
self.logger.error(f"Process timed out after {self.timeout} seconds. Terminating process tree (PID={process.pid}).")
137-
self._terminate_process_tree(process.pid)
138-
raise
131+
stdout, stderr = process.communicate(timeout=self.timeout)
139132
else:
140133
# No output capturing; just wait for completion or timeout
141-
try:
142-
process.wait(timeout=self.timeout)
143-
except subprocess.TimeoutExpired:
144-
self.logger.error(f"Process timed out after {self.timeout} seconds. Terminating process tree (PID={process.pid}).")
145-
self._terminate_process_tree(process.pid)
146-
raise
134+
process.wait(timeout=self.timeout)
147135
# HANDLE RETURN CODES AND ERROR INTERPRETATION
148136
if handle_errors:
149137
# Check return code
@@ -152,6 +140,7 @@ def execute(self, handle_errors: bool = True) -> subprocess.CompletedProcess[Any
152140
else:
153141
completed_process = subprocess.CompletedProcess(process.args, process.returncode, stdout, stderr)
154142
except subprocess.TimeoutExpired:
143+
self._terminate_process_tree(process.pid)
155144
raise UserNotificationException(f"Command '{self.command_str}' timed out after {self.timeout} seconds and was forcefully terminated.") from None
156145
except subprocess.CalledProcessError as e:
157146
raise UserNotificationException(f"Command '{self.command_str}' execution failed with return code {e.returncode}") from None

tests/test_subprocess.py

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -215,6 +215,31 @@ def test_timeout_kills_process_tree(self, tmp_path: Path) -> None:
215215
child_pid = int(pid_file.read_text().strip())
216216
assert not psutil.pid_exists(child_pid), f"Child process {child_pid} should have been killed by _terminate_process_tree"
217217

218+
@pytest.mark.exploratory
219+
def test_streaming_timeout_blocked_by_silent_process(self) -> None:
220+
"""
221+
Reproduces the bug where readline() in the streaming output loop blocks
222+
indefinitely when the process is alive but writes nothing to stdout.
223+
224+
Expected: UserNotificationException raised within ~timeout seconds.
225+
Actual: readline() blocks until the process exits (~10 s), so the
226+
timeout check is never reached on time.
227+
"""
228+
executor = SubprocessExecutor(
229+
["python", "-c", "import time; time.sleep(10)"],
230+
capture_output=True,
231+
print_output=True, # triggers the streaming readline() path
232+
timeout=2,
233+
)
234+
start = time.monotonic()
235+
with pytest.raises(UserNotificationException, match="timed out"):
236+
executor.execute()
237+
elapsed = time.monotonic() - start
238+
# BUG: elapsed will be ~10 s instead of ~2 s because readline() blocks
239+
# waiting for a newline or EOF; the timeout check is only reached after
240+
# the process finally exits.
241+
assert elapsed < 5, f"Timeout should fire within ~2 s but took {elapsed:.1f} s"
242+
218243
@pytest.mark.parametrize(
219244
"capture_output, print_output",
220245
[

0 commit comments

Comments
 (0)