asyncio: Asynchronous libtmux
#554
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
tony
force-pushed
the
asyncio
branch
4 times, most recently
from
e7fdf41 to
1d31a1f
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #554 +/- ##
==========================================
+ Coverage 82.13% 86.16% +4.02%
==========================================
Files 36 36
Lines 3930 3990 +60
Branches 360 371 +11
==========================================
+ Hits 3228 3438 +210
+ Misses 551 385 -166
- Partials 151 167 +16 ☔ View full report in Codecov by Sentry. |
tony
force-pushed
the
asyncio
branch
3 times, most recently
from
d0d85ec to
1103e6a
Compare
|
@sourcery-ai review |
Reviewer's Guide by SourceryThis pull request introduces asynchronous functionality to libtmux using Python's Sequence diagram for async tmux command executionsequenceDiagram
participant C as Client Code
participant A as AsyncTmuxCmd
participant S as Subprocess
participant T as tmux
C->>+A: run(*args)
A->>A: validate tmux binary
A->>+S: create_subprocess_exec
S->>+T: execute command
T-->>-S: command output
S-->>-A: stdout, stderr, returncode
A->>A: process output
A-->>-C: AsyncTmuxCmd instance
Class diagram for new AsyncTmuxCmd and its integrationclassDiagram
class AsyncTmuxCmd {
+list[str] cmd
+list[str] stdout
+list[str] stderr
+int returncode
+__init__(cmd, stdout, stderr, returncode)
+run(*args) AsyncTmuxCmd
}
class Server {
+cmd(cmd, *args, target)
+acmd(cmd, *args, target) AsyncTmuxCmd
}
class Session {
+cmd(cmd, *args, target)
+acmd(cmd, *args, target) AsyncTmuxCmd
}
class Window {
+cmd(cmd, *args, target)
+acmd(cmd, *args, target) AsyncTmuxCmd
}
class Pane {
+cmd(cmd, *args, target)
+acmd(cmd, *args, target) AsyncTmuxCmd
}
Server --> AsyncTmuxCmd : uses
Session --> Server : uses
Window --> Server : uses
Pane --> Server : uses
note for AsyncTmuxCmd "New async class for tmux commands"
note for Server "Added async support via acmd"
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @tony - I've reviewed your changes - here's some feedback:
Overall Comments:
- Consider adding more test cases to cover error conditions and different tmux command scenarios in test_async.py
Here's what I looked at during the review
- 🟢 General issues: all looks good
- 🟢 Security: all looks good
- 🟡 Testing: 2 issues found
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| session_id=session_id, | ||
| server=server, | ||
| ) | ||
| assert isinstance(session, Session) |
There was a problem hiding this comment.
suggestion (testing): Test more aspects of the asynchronous functionality.
The current test only verifies the type of the returned session object. It would be beneficial to interact with the session asynchronously, for example, by creating a window or pane, executing a command within the session, and then verifying the results. This would provide more comprehensive coverage of the asynchronous API.
Suggested implementation:
@pytest.mark.asyncio
async def test_asyncio(server: Server) -> None:
"""Test basic asyncio usage."""
# Create new session
result = await server.acmd("new-session", "-d", "-P", "-F#{session_id}")
session_id = result.stdout[0]
session = Session.from_session_id(
session_id=session_id,
server=server,
)
assert isinstance(session, Session)
# Create new window
window = await session.new_window(window_name="test_window")
assert window.name == "test_window"
# Create new pane and execute command
pane = await window.split_window()
await pane.send_keys("echo 'Hello, tmux!'")
# Allow time for command execution
await asyncio.sleep(0.5)
# Capture and verify output
output = await pane.capture_pane()
assert "Hello, tmux!" in '\n'.join(output)
# Clean up
await window.kill_window()You may need to:
- Import asyncio if it's not already imported
- Adjust the sleep duration (0.5s) based on your system's performance
- Ensure the Session class has the async methods new_window(), and Window class has split_window() and kill_window()
- Ensure Pane class has send_keys() and capture_pane() async methods
| @pytest.mark.asyncio | ||
| async def test_asyncio(server: Server) -> None: | ||
| """Test basic asyncio usage.""" | ||
| result = await server.acmd("new-session", "-d", "-P", "-F#{session_id}") |
There was a problem hiding this comment.
suggestion (testing): Add tests for error handling.
Include tests to verify the behavior of the acmd function when the tmux command fails, for example, by trying to create a session with an already existing name. Check that the appropriate exceptions are raised and handled correctly.
Suggested implementation:
@pytest.mark.asyncio
async def test_asyncio(server: Server) -> None:
"""Test basic asyncio usage."""
result = await server.acmd("new-session", "-d", "-P", "-F#{session_id}")
session_id = result.stdout[0]
session = Session.from_session_id(
session_id=session_id,
server=server,
)
assert isinstance(session, Session)
@pytest.mark.asyncio
async def test_asyncio_duplicate_session(server: Server) -> None:
"""Test error handling when creating duplicate sessions."""
# Create first session
session_name = "test_duplicate"
await server.acmd("new-session", "-d", "-s", session_name)
# Attempt to create second session with same name
with pytest.raises(LibTmuxException) as excinfo:
await server.acmd("new-session", "-d", "-s", session_name)
assert "duplicate session" in str(excinfo.value).lower()
@pytest.mark.asyncio
async def test_asyncio_invalid_command(server: Server) -> None:
"""Test error handling with invalid tmux commands."""
with pytest.raises(LibTmuxException) as excinfo:
await server.acmd("invalid-command")
assert "unknown command" in str(excinfo.value).lower()You'll need to:
- Import LibTmuxException if not already imported (likely from libtmux.exc)
- Ensure any existing sessions are cleaned up in test teardown to prevent test interference
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Docs
Asyncio
See also
Summary by Sourcery
Introduce asynchronous support for libtmux using asyncio.
New Features:
AsyncTmuxCmdto run tmux commands asynchronously.acmdmethod toServer,Session,Window, andPaneobjects for asynchronous command execution.Tests: