fix: pre-existing issues in server.py and common.py (raise_if_dead, has_minimum_version)#641
fix: pre-existing issues in server.py and common.py (raise_if_dead, has_minimum_version)#641tony wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #641 +/- ##
=======================================
Coverage 52.44% 52.44%
=======================================
Files 26 26
Lines 3726 3726
Branches 747 747
=======================================
Hits 1954 1954
Misses 1468 1468
Partials 304 304 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
cd81d15 to
1cccc1d
Compare
1cccc1d to
451573b
Compare
451573b to
63c205a
Compare
63c205a to
08ed44d
Compare
|
Flagging that this PR is more current than its title suggests, and that it silently fixes an issue filed four months after it. The two things the title and body advertise have already landed on master independently — - subprocess.check_call([resolved, *cmd_args])
+ subprocess.check_call(
+ [resolved, *cmd_args],
+ stdout=subprocess.DEVNULL,
+ stderr=subprocess.DEVNULL,
+ )That is exactly the fix #711 asks for: Worth retitling this PR to name that fix, and deciding one open question here rather than in a second PR: |
why: subprocess.check_call without stdout/stderr redirection inherits the
parent process's file descriptors, printing tmux output directly to
the terminal. Library code should not produce terminal noise.
what:
- Pass stdout=subprocess.DEVNULL, stderr=subprocess.DEVNULL to check_call
08ed44d to
218c0fb
Compare
Summary
Three pre-existing issues surfaced during a fresh code review of PR #636. This PR addresses them as atomic commits against
master.common(fix[has_minimum_version]):has_minimum_versionwas callingget_version()twice, spawning two subprocesses for no reason. Now cached in a local variable.server(docs[raise_if_dead]):raise_if_deadhad noRaisessection in its docstring. BothTmuxCommandNotFound(missing binary) andCalledProcessError(server not running) are now documented.server(fix[raise_if_dead]):subprocess.check_callinraise_if_deadinherited the parent process's stdout/stderr, leakinglist-sessionsoutput to the terminal. Now redirected tosubprocess.DEVNULL.Test plan
uv run ruff check . --fix --show-fixes— cleanuv run ruff format .— no changesuv run mypy— no issues (60 source files)uv run py.test -x -q— 885 passed, 1 skipped