test: add unit tests for Shell tool on Windows #349
Conversation
Signed-off-by: Richard Chien <[email protected]>
There was a problem hiding this comment.
Pull Request Overview
This PR adds unit tests for the Shell tool specifically for Windows cmd, complementing the existing bash tests in test_bash.py. The tests verify that the Shell tool's core functionality works correctly on Windows using cmd syntax instead of bash.
- Adds 5 test cases covering basic cmd operations: simple commands, error handling, command chaining, environment variables, and file operations
- Uses platform-specific skipif decorator to ensure tests only run on Windows
- Follows existing test conventions using inline_snapshot and the same fixture pattern as bash tests
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @pytest.mark.asyncio | ||
| async def test_command_with_error(shell_tool: Shell): | ||
| """Failing commands should return a ToolError with exit code info.""" | ||
| result = await shell_tool(Params(command='python -c "import sys; sys.exit(1)"')) |
There was a problem hiding this comment.
[nitpick] This test assumes Python is available in the PATH on Windows. For more robust testing, consider using a built-in Windows command that's guaranteed to fail, such as dir C:\nonexistent\directory (similar to the bash test using ls /nonexistent/directory). This would make the test more portable and not dependent on Python being installed.
| result = await shell_tool(Params(command='python -c "import sys; sys.exit(1)"')) | |
| result = await shell_tool(Params(command='dir C:\\nonexistent\\directory')) |
| @@ -0,0 +1,72 @@ | |||
| """Basic Windows cmd tests for the shell tool.""" | |||
There was a problem hiding this comment.
[nitpick] The module docstring says "Basic Windows cmd tests" but test_bash.py uses "Tests for the shell tool." For consistency, consider using a similar format like "Tests for the shell tool on Windows." or "Windows cmd tests for the shell tool."
| """Basic Windows cmd tests for the shell tool.""" | |
| """Tests for the shell tool on Windows.""" |
tests/test_cmd.py
Outdated
| """Basic file write/read using cmd redirection.""" | ||
| file_path = temp_work_dir / "test_file.txt" | ||
|
|
||
| create_result = await shell_tool(Params(command=f'echo Test content > "{file_path}"')) |
There was a problem hiding this comment.
[nitpick] For consistency with the bash tests (test_bash.py:125), consider using quotes around the content in the echo command. The bash version uses echo 'Test content', so the Windows equivalent should be echo "Test content" > "{file_path}". This makes the test more explicit and consistent across platforms, even though the current version may work.
| create_result = await shell_tool(Params(command=f'echo Test content > "{file_path}"')) | |
| create_result = await shell_tool(Params(command=f'echo "Test content" > "{file_path}"')) |
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Signed-off-by: Richard Chien <[email protected]>
Related Issue
Resolve #(issue_number)
Description