auto_load_commands now defaults to False#1520
Conversation
Also: - The `tests_isolated` directory has been deleted and all tests underneath there have been moved under `tests`
|
🤖 Hi @tleonhardt, I've received your request, and I'm working on it now! You can track my progress in the logs for more details. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #1520 +/- ##
==========================================
+ Coverage 98.90% 98.92% +0.02%
==========================================
Files 21 21
Lines 4933 4933
==========================================
+ Hits 4879 4880 +1
+ Misses 54 53 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
📋 Review Summary
This pull request updates the default behavior of auto_load_commands to False, which is a significant but well-documented change. The consolidation of tests from tests_isolated into the tests directory is a great improvement for maintainability.
🔍 General Feedback
- The changes are well-documented in the
CHANGELOG.mdanddocs/upgrades.mdfiles. - The tests have been updated to reflect the new default behavior of
auto_load_commands. - The removal of the
tests_isolateddirectory simplifies the project structure.
The
auto_load_commandsargument to thecmd2.Cmd.__init__initializer now defaults toFalse.Also:
tests_isolateddirectory has been deleted and all tests underneath there have been moved undertestsSwitching the default value for
auto_load_commandswas required in order to be able to run all of the tests in the same suite. Alternatively, we would have had to change every singlecmd2.Cmdinstance in the main tests and setauto_load_commands=False.