feat: Show summary of used methods on CLI with wakepy -v#567
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughA substantial refactor of the CLI module replacing monolithic rendering with modular architecture. Introduces a new public Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/wakepy/__main__.pytests/unit/test_main.py
🧰 Additional context used
📓 Path-based instructions (2)
**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
**/*.py: Use mypy strict mode with full type annotations
Maintain 100% test coverage
Place top-level functions before subfunctions in file organization
Place imports at the top of the file, not inside functions
Files:
tests/unit/test_main.pysrc/wakepy/__main__.py
tests/**/*.py
📄 CodeRabbit inference engine (AGENTS.md)
tests/**/*.py: Use pytest with test classes for organizing tests
Prefer using fixtures in tests instead of inline setup
Files:
tests/unit/test_main.py
🪛 GitHub Actions: Fast Tests 🚀
tests/unit/test_main.py
[error] 12-12: Error during test collection due to ImportError in wakepy.main.
src/wakepy/__main__.py
[error] 24-24: ImportError: cannot import name 'TypedDict' from 'typing' (Python 3.7) - TypedDict is not available in this Python version.
🔇 Additional comments (21)
src/wakepy/__main__.py (11)
60-84: LGTM!The display layout constants are well-defined, and
wait_for_interruptcorrectly handles theKeyboardInterruptfor graceful termination.
87-115: LGTM!The
DisplayThemedataclass is well-structured with immutability enforced viafrozen=True. The factory method pattern withcreate()provides a clean API for theme instantiation.
117-121: LGTM once the import is fixed.The
SystemInfoTypedDict is well-defined. This will work correctly after resolving theTypedDictimport issue flagged earlier.
123-137: LGTM!The
main()function cleanly implements dependency injection for testability. The command dispatch logic correctly handles both the default mode and the "methods" subcommand.
139-182: LGTM!The argument parsing is well-structured with good reuse of
_add_mode_argumentsfor both the main command and the "methods" subcommand.
185-206: LGTM!The mode determination logic is clear and handles deprecated flags gracefully. The conflict detection with a helpful error message is appropriate.
275-360: LGTM!The
CliAppclass is well-structured with clear separation of concerns. Therun_wakepymethod properly handles the mode lifecycle, verbose output, and graceful termination.
363-376: LGTM!The
get_system_infofactory andspinner_framesgenerator are well-implemented. The# pragma: no branchcomment correctly handles coverage for the infinite loop.
379-410: LGTM!The rendering functions properly handle string truncation to maintain consistent layout, and the template formatting is clean.
413-469: LGTM!The rendering utilities are well-organized with consistent text wrapping. The activation error message provides helpful diagnostic information for issue reporting.
472-533: LGTM!The
render_methods_outputfunction cleanly handles both verbose and non-verbose modes. The_add_mode_argumentshelper promotes DRY by reusing argument definitions across parsers.tests/unit/test_main.py (10)
10-29: Imports are blocked by the upstreamTypedDictissue.The imports from
wakepy.__main__will fail until theTypedDictimport compatibility issue is resolved in the source file. Once fixed, these imports are correctly structured.
37-76: LGTM!The fixtures are well-organized using pytest's fixture pattern as per coding guidelines, providing reusable test components for working and broken methods.
79-116: LGTM!The
TestGetModeNameclass provides thorough coverage of mode selection logic with well-organized parametrized tests for different argument combinations.
130-154: LGTM!Good coverage for
wait_for_interruptedge cases including keyboard interrupt handling and empty iterator. The tests correctly usecapsysfor output verification.
157-215: LGTM!The
TestCliAppRunWakepyclass provides comprehensive coverage of the main CLI flow, including success, failure, custom theme, and deprecation warning scenarios.
217-257: LGTM!The verbose mode tests properly cover both cases where method information is available and when it's empty, verifying the appropriate output in each scenario.
259-363: LGTM!The
TestCliAppRunWakepyMethodsclass provides thorough testing of the methods subcommand with good coverage of both verbose and non-verbose output formats, plus injection testing viaprobe_runner.
365-404: LGTM!The
TestDisplayThemeandTestShouldUseAsciiOnlyclasses provide comprehensive coverage of theme creation and ASCII mode detection across different platforms.
407-519: LGTM!The
TestRenderingclass provides excellent coverage of all rendering functions, including edge cases like long name truncation and infinite spinner frame generation.
521-578: LGTM!The
TestMainandTestGetLoggingLevelclasses properly test the entry point and logging configuration with good use of mocking and parametrization.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
f95f174 to
d2b777f
Compare
Previously, running "wakepy -v" would just enable INFO level logging. Now it also prints a summary of used (and unused) methods just before the wakepy logo.
Previously it was possible to see the list of associated wakepy Methods only in case of failure. Now user may see them with the -v flag.
Example:
Closes: #550