Add option to run CLI tests from python, and add example test. #156
Conversation
|
thank you! |
There was a problem hiding this comment.
Pull request overview
This PR adds the ability to test the CLI from Python by making the main() function accept an optional argv parameter. This enables programmatic testing of CLI commands, which is particularly useful for automated testing.
Changes:
- Modified
main()function to accept an optionalargvparameter (defaults tosys.argvif not provided) - Added comprehensive tests for the
_walk_allutility function - Added integration test for create and extract CLI commands
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/littlefs/main.py | Modified main() to accept optional argv parameter for testability |
| test/cli/test_walk_all.py | Added unit tests for _walk_all function covering basic traversal, symlinks, empty directories, and nested structures |
| test/cli/test_create_and_extract.py | Added integration test for create and extract CLI commands with directory comparison |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| # Compare directories | ||
| comparison = filecmp.dircmp(source_dir, extract_dir) | ||
| assert not comparison.diff_files | ||
| assert not comparison.left_only | ||
| assert not comparison.right_only |
There was a problem hiding this comment.
The filecmp.dircmp comparison only performs a shallow comparison and doesn't recursively check subdirectories. This means that differences in files within subdirectories (like "subdir/file2.txt") may not be caught by the assertions on diff_files, left_only, and right_only. While the explicit file content checks on lines 47-49 will catch this specific case, consider using filecmp.dircmp recursively or using a recursive comparison function to make the test more robust and comprehensive.
|
|
||
|
|
||
| def main(): | ||
| # Getting argv optionally from the caller to enable call from python (generally for testing, but could be used for other purposes) |
There was a problem hiding this comment.
The comment should start with a capital letter and use proper sentence structure. Consider revising to: "Getting argv optionally from the caller to enable calls from Python (generally for testing, but could be used for other purposes)."
| # Getting argv optionally from the caller to enable call from python (generally for testing, but could be used for other purposes) | |
| # Getting argv optionally from the caller to enable calls from Python (generally for testing, but could be used for other purposes). |
| @@ -0,0 +1,49 @@ | |||
| from pathlib import Path | |||
There was a problem hiding this comment.
Import of 'Path' is not used.
| from pathlib import Path |
|
@amgross you can determine what's valuable/not from copilot and follow it up in your next PR (no need to separate it from the feature you are working on). At somepoint I need to setup Ruff for this repo to catch things like unused imports. Maybe i'll revisit that once your feature is complete! |
|
Great REPO |
2 participants
Added easy option to test CLI from python (it may be used also by users).
This is first step for changes we planned to do in #150 , to have good option to test it.