-
Notifications
You must be signed in to change notification settings - Fork 185
Add more commands tests #500
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds comprehensive integration test coverage for command-line interface commands across wallet, validator, pool, backup, and BTC teleport modules. The tests use mocking extensively to isolate command behavior and verify correct interactions with core components.
Key changes:
- Added 8 new test files covering wallet, validator, pool (single/nominator/general), backup, and BTC teleport commands
- Improved test infrastructure with helper functions and fixture enhancements
- Enhanced production code with type hints, better exception handling, and code cleanup
- Updated CI workflow to test against Python 3.8-3.13 and added Ruff linting
Reviewed changes
Copilot reviewed 21 out of 21 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/integration/test_wallet_commands.py | Tests for wallet operations (create, activate, list, import, export, delete, transfer) |
| tests/integration/test_validator_commands.py | Tests for validator operations (voting, efficiency checking, collator management) |
| tests/integration/test_single_pool_commands.py | Tests for single nominator pool commands (create, activate, withdraw) |
| tests/integration/test_pool_commands.py | Tests for general pool commands (list, delete, import) |
| tests/integration/test_nominator_pool_commands.py | Tests for nominator pool commands (create, activate, deposit, withdraw) |
| tests/integration/test_btc_teleport_commands.py | Tests for BTC teleport removal command |
| tests/integration/test_backup_commands.py | Tests for backup creation and restoration |
| tests/integration/test_basic_commands.py | Updated to use package resource path helper |
| tests/helpers.py | Added helper function for creating pool files in tests |
| tests/conftest.py | Modified CLI fixture setup with monkeypatching |
| setup.py | Excluded tests from package, bumped minimum Python to 3.8, removed Python 3.7 classifier |
| mytonctrl/utils.py | Added type hints, improved exception handling |
| mytonctrl/scripts/xrestart.py | Removed entire deprecated script |
| mytonctrl/scripts/etabar.py | Improved code style (None comparison) |
| mytonctrl/progressbar.py | Improved code style (boolean comparison) |
| mytonctrl/mytonctrl.py | Multiple improvements: removed unused imports/code, better exception handling, type hints, f-string fixes |
| mytoncore/mytoncore.py | Added type hints, removed Init() call from constructor |
| mytoncore/functions.py | Removed unused auto_vote_offers cycle |
| modules/btc_teleport.py | Removed unused offer voting methods, added type hints |
| modules/backups.py | Added type hints, improved error handling |
| .github/workflows/tests.yml | Added Python version matrix (3.8-3.13), Ruff linting, and build step |
Comments suppressed due to low confidence (1)
tests/conftest.py:34
- This comment appears to contain commented-out code.
# def write_db(self, data):
# self.buffer.old_db = Dict(self.db)
#
# def load_db(self, db_path=False):
# self.set_default_config()
# return True
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| account.status = "empty" | ||
| with pytest.raises(Exception) as e: | ||
| cli.execute(f"activate_pool {pool_name}") | ||
| assert "account status is empty" in str(e) |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception assertion is checking 'str(e)' which captures the ExceptionInfo object representation, not the actual exception message. It should be 'str(e.value)' to check the actual exception message.
| assert "account status is empty" in str(e) | |
| assert "account status is empty" in str(e.value) |
| with pytest.raises(Exception) as e: | ||
| cli.execute(f"new_single_pool pool2 {owner_address}") | ||
| assert os.path.isfile(addr_file) | ||
| assert 'Pool with the same parameters already exists' in str(e) |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception assertion is checking 'str(e)' which captures the ExceptionInfo object representation, not the actual exception message. It should be 'str(e.value)' to check the actual exception message.
| assert 'Pool with the same parameters already exists' in str(e) | |
| assert 'Pool with the same parameters already exists' in str(e.value) |
| # def write_db(self, data): | ||
| # self.buffer.old_db = Dict(self.db) | ||
| # | ||
| # def load_db(self, db_path=False): | ||
| # self.set_default_config() | ||
| # return True |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented-out code should be removed rather than left in the codebase. If these methods are no longer needed, they should be deleted entirely. If they might be needed in the future, consider documenting why they're disabled.
| import typing | ||
| from typing import Optional |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The 'typing' module is imported on line 5, but Optional is imported from typing on line 6. The 'typing' import on line 5 should be removed since it's redundant - only the specific Optional type is needed.
| return subprocess.run(["bash", backup_script_path, "-u", user] + args, timeout=5) | ||
|
|
||
| def create_backup(self, args): | ||
| def create_backup(self, args: list) -> typing.Union[int, None]: |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The return type annotation 'typing.Union[int, None]' should be simplified to 'Optional[int]' for consistency with the rest of the codebase. The Optional type is already imported on line 6.
| def create_backup(self, args: list) -> typing.Union[int, None]: | |
| def create_backup(self, args: list) -> Optional[int]: |
| with pytest.raises(Exception) as e: | ||
| cli.execute(f"new_pool pool2 5.5 10 10000 1000") | ||
| assert os.path.isfile(addr_file) | ||
| assert 'Pool with the same parameters already exists' in str(e) |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exception assertion is checking 'str(e)' which captures the ExceptionInfo object representation, not the actual exception message. It should be 'str(e.value)' to check the actual exception message.
| assert 'Pool with the same parameters already exists' in str(e) | |
| assert 'Pool with the same parameters already exists' in str(e.value) |
| @@ -0,0 +1,131 @@ | |||
| import os | |||
| import struct | |||
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'struct' is not used.
| import struct |
|
|
||
| import pytest | ||
| from pytest_mock import MockerFixture | ||
| from mypylib import Dict |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'Dict' is not used.
| from mypylib import Dict |
| from pytest_mock import MockerFixture | ||
| from mypylib import Dict | ||
| from mytoncore.mytoncore import MyTonCore | ||
| from modules.single_pool import SingleNominatorModule |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Import of 'SingleNominatorModule' is not used.
| from modules.single_pool import SingleNominatorModule |
| except Exception: | ||
| pass |
Copilot
AI
Dec 22, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | |
| pass | |
| except Exception as exc: | |
| # Log the error but continue with ADNL connection check | |
| local.add_log(f"Failed to get validator config for ADNL check: {exc}", "warning") |
No description provided.