feat: add contract_id support for CryptoGetAccountBalanceQuery #1389
Conversation
Signed-off-by: Antonio Ceppellini <[email protected]>
📝 WalkthroughWalkthroughAdds optional contract_id support to CryptoGetAccountBalanceQuery: constructor accepts contract_id, new set_contract_id() method, SDK enforces exactly one of account_id or contract_id before request construction, and request populates the protobuf field accountID or contractID accordingly. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Query as CryptoGetAccountBalanceQuery
participant Node as HederaNode
Client->>Query: construct (account_id? / contract_id?)
alt account_id set
Query->>Node: send CryptoGetAccountBalance (accountID)
else contract_id set
Query->>Node: send CryptoGetAccountBalance (contractID)
end
Node-->>Query: CryptoGetAccountBalanceResponse
Query-->>Client: parsed balance response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/hiero_sdk_python/query/account_balance_query.py (1)
25-33: Docstring missingcontract_idparameter documentation.The
__init__docstring documentsaccount_idbut omits the newcontract_idparameter.📝 Proposed fix
""" Initializes a new instance of the CryptoGetAccountBalanceQuery class. Args: account_id (AccountId, optional): The ID of the account to retrieve the balance for. + contract_id (ContractId, optional): The ID of the contract to retrieve the balance for. + Mutually exclusive with account_id. """
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
CHANGELOG.mdexamples/query/contract_balance_query.pysrc/hiero_sdk_python/query/account_balance_query.pytests/integration/account_balance_query_e2e_test.pytests/unit/account_balance_query_test.py
🧰 Additional context used
📓 Path-based instructions (3)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/query/contract_balance_query.py
tests/integration/**/*
⚙️ CodeRabbit configuration file
tests/integration/**/*: You are acting as a senior maintainer reviewing integration tests for the hiero-sdk-python project. Your goal is to ensure end-to-end tests validate real network behavior safely and deterministically.CRITICAL PRINCIPLES - Safety & Diagnosability:
- Prioritize safety: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just
SUCCESS.- Failure-path tests must assert specific
ResponseCodevalues (e.g.,TOKEN_HAS_NO_PAUSE_KEY).PRIORITY 1 - End-to-End Behavior:
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
- Validate resulting balances, ownership, and state changes (not just transaction success).
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
- Verify network state after operations (e.g., account balance changed, token transferred).
PRIORITY 2 - Test Structure & Maintainability:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller modules.
- Are helper functions good candidates for pytest fixtures or shared utilities?
PRIORITY 3 - Isolation & Cleanup:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle assumptions).
PRIORITY 4 - Assertions & Coverage:
- Do tests validate only success/failure, or also assert resulting state?
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
- Cover happy paths AND ...
Files:
tests/integration/account_balance_query_e2e_test.py
tests/unit/**/*
⚙️ CodeRabbit configuration file
tests/unit/**/*: You are acting as a senior maintainer reviewing unit tests for the hiero-sdk-python project. Your goal is to ensure tests are extensive, deterministic, and protect against breaking changes.CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically:
- Tests must provide useful error messages when they fail for future debugging.
- No
print()statements - use assertions with descriptive messages.- No timing-dependent or unseeded random assertions.
- No network calls or external dependencies (unit tests are isolated).
- No unjustified TODOs or skipped tests without tracking issues.
PRIORITY 1 - Protect Against Breaking Changes:
- Assert public attributes exist (e.g.,
assert hasattr(obj, 'account_id')).- Assert return types where relevant (e.g.,
assert isinstance(result, AccountId)).- Assert fluent setters return
self(e.g.,assert tx.set_memo("test") is tx).- Assert backward-compatible defaults are maintained.
- If a breaking change is introduced, tests must assert deprecation behavior and test old behavior until removal.
PRIORITY 2 - Constructor & Setter Behavior:
- Test constructor behavior with valid inputs, edge cases, and invalid inputs.
- Test setter behavior including method chaining (fluent interface).
- Verify that setters validate input and raise appropriate exceptions.
- Test that getters return expected values after construction/setting.
PRIORITY 3 - Comprehensive Coverage:
- Unit tests should be extensive - test even if we don't expect users to use it currently.
- Cover happy paths AND unhappy paths/edge cases.
- Test boundary conditions, null/None values, empty collections, etc.
- Avoid brittle ordering assertions unless order is part of the contract.
PRIORITY 4 - No Mocks for Non-Existent Modules:
- All imports must reference actual SDK modules - no hallucinated paths.
- Validate import paths against the actual
src/hiero_sdk_pythonstructure.- Mocks should only be used for external de...
Files:
tests/unit/account_balance_query_test.py
🧬 Code graph analysis (3)
examples/query/contract_balance_query.py (3)
src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/query/account_balance_query.py (3)
CryptoGetAccountBalanceQuery(12-175)set_contract_id(50-63)execute(126-149)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)
tests/integration/account_balance_query_e2e_test.py (2)
src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/query/account_balance_query.py (3)
CryptoGetAccountBalanceQuery(12-175)execute(126-149)set_contract_id(50-63)
tests/unit/account_balance_query_test.py (3)
src/hiero_sdk_python/response_code.py (1)
ResponseCode(4-387)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)
🪛 Ruff (0.14.10)
examples/query/contract_balance_query.py
43-43: Avoid specifying long messages outside the exception class
(TRY003)
79-79: Abstract raise to an inner function
(TRY301)
79-79: Avoid specifying long messages outside the exception class
(TRY003)
89-89: Do not catch blind exception: Exception
(BLE001)
tests/integration/account_balance_query_e2e_test.py
20-20: Undefined name os
(F821)
tests/unit/account_balance_query_test.py
57-57: Do not catch blind exception: Exception
(BLE001)
102-102: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
113-113: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
src/hiero_sdk_python/query/account_balance_query.py
80-80: Abstract raise to an inner function
(TRY301)
80-80: Avoid specifying long messages outside the exception class
(TRY003)
83-83: Abstract raise to an inner function
(TRY301)
83-83: Avoid specifying long messages outside the exception class
(TRY003)
98-100: Abstract raise to an inner function
(TRY301)
98-100: Avoid specifying long messages outside the exception class
(TRY003)
152-152: Dynamically typed expressions (typing.Any) are disallowed in response
(ANN401)
168-168: Missing return type annotation for private function _is_payment_required
Add return type annotation: bool
(ANN202)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (4)
tests/unit/account_balance_query_test.py (1)
67-130: Comprehensive test coverage for contract_id scenarios.The new tests thoroughly cover:
- Constructor behavior with contract_id
- Method chaining and mutual exclusivity enforcement via setters
- Validation errors when both or neither IDs are set
- Correct protobuf field population for contract_id path
This aligns well with the unit test guidelines to protect against breaking changes and test constructor/setter behavior.
examples/query/contract_balance_query.py (1)
1-95: Well-structured example demonstrating contract balance queries.The example correctly demonstrates:
- Standard example patterns (
main(),if __name__ == "__main__",load_dotenv())- Proper query lifecycle:
CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(client)- Valid import paths matching the SDK structure
- Clear documentation of required/optional environment variables
- Appropriate error handling with
sys.exit(1)The commented-out lines 83-85 helpfully show the mutual exclusivity constraint for educational purposes.
src/hiero_sdk_python/query/account_balance_query.py (2)
50-63: Well-implementedset_contract_idmethod.The setter correctly:
- Clears
account_idto enforce mutual exclusivity- Returns
selffor method chaining- Includes proper docstring
This aligns with the protobuf
oneof balanceSourcesemantics specified in the issue requirements.
78-94: Correct mutual exclusivity validation and conditional proto population.The implementation properly enforces the
oneofsemantics:
- Raises
ValueErrorif neither ID is set- Raises
ValueErrorif both IDs are set- Populates
accountIDwhenaccount_idis set, otherwisecontractIDThis matches the expected behavior from issue #1293.
|
Hi, this is WorkflowBot.
|
Signed-off-by: Antonio Ceppellini <[email protected]>
Codecov Report❌ Patch coverage is @@ Coverage Diff @@
## main #1389 +/- ##
==========================================
+ Coverage 92.44% 92.46% +0.01%
==========================================
Files 139 139
Lines 8528 8547 +19
==========================================
+ Hits 7884 7903 +19
Misses 644 644 🚀 New features to boost your workflow:
|
Signed-off-by: Antonio Ceppellini <[email protected]>
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (4)
CHANGELOG.mdexamples/query/contract_balance_query.pytests/integration/account_balance_query_e2e_test.pytests/unit/account_balance_query_test.py
🧰 Additional context used
📓 Path-based instructions (3)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/query/contract_balance_query.py
tests/unit/**/*
⚙️ CodeRabbit configuration file
tests/unit/**/*: You are acting as a senior maintainer reviewing unit tests for the hiero-sdk-python project. Your goal is to ensure tests are extensive, deterministic, and protect against breaking changes.CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically:
- Tests must provide useful error messages when they fail for future debugging.
- No
print()statements - use assertions with descriptive messages.- No timing-dependent or unseeded random assertions.
- No network calls or external dependencies (unit tests are isolated).
- No unjustified TODOs or skipped tests without tracking issues.
PRIORITY 1 - Protect Against Breaking Changes:
- Assert public attributes exist (e.g.,
assert hasattr(obj, 'account_id')).- Assert return types where relevant (e.g.,
assert isinstance(result, AccountId)).- Assert fluent setters return
self(e.g.,assert tx.set_memo("test") is tx).- Assert backward-compatible defaults are maintained.
- If a breaking change is introduced, tests must assert deprecation behavior and test old behavior until removal.
PRIORITY 2 - Constructor & Setter Behavior:
- Test constructor behavior with valid inputs, edge cases, and invalid inputs.
- Test setter behavior including method chaining (fluent interface).
- Verify that setters validate input and raise appropriate exceptions.
- Test that getters return expected values after construction/setting.
PRIORITY 3 - Comprehensive Coverage:
- Unit tests should be extensive - test even if we don't expect users to use it currently.
- Cover happy paths AND unhappy paths/edge cases.
- Test boundary conditions, null/None values, empty collections, etc.
- Avoid brittle ordering assertions unless order is part of the contract.
PRIORITY 4 - No Mocks for Non-Existent Modules:
- All imports must reference actual SDK modules - no hallucinated paths.
- Validate import paths against the actual
src/hiero_sdk_pythonstructure.- Mocks should only be used for external de...
Files:
tests/unit/account_balance_query_test.py
tests/integration/**/*
⚙️ CodeRabbit configuration file
tests/integration/**/*: You are acting as a senior maintainer reviewing integration tests for the hiero-sdk-python project. Your goal is to ensure end-to-end tests validate real network behavior safely and deterministically.CRITICAL PRINCIPLES - Safety & Diagnosability:
- Prioritize safety: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just
SUCCESS.- Failure-path tests must assert specific
ResponseCodevalues (e.g.,TOKEN_HAS_NO_PAUSE_KEY).PRIORITY 1 - End-to-End Behavior:
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
- Validate resulting balances, ownership, and state changes (not just transaction success).
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
- Verify network state after operations (e.g., account balance changed, token transferred).
PRIORITY 2 - Test Structure & Maintainability:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller modules.
- Are helper functions good candidates for pytest fixtures or shared utilities?
PRIORITY 3 - Isolation & Cleanup:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle assumptions).
PRIORITY 4 - Assertions & Coverage:
- Do tests validate only success/failure, or also assert resulting state?
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
- Cover happy paths AND ...
Files:
tests/integration/account_balance_query_e2e_test.py
🧬 Code graph analysis (2)
tests/unit/account_balance_query_test.py (3)
src/hiero_sdk_python/query/account_balance_query.py (6)
CryptoGetAccountBalanceQuery(12-175)set_account_id(35-48)execute(126-149)_is_payment_required(168-175)set_contract_id(50-63)_make_request(65-107)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)
tests/integration/account_balance_query_e2e_test.py (4)
src/hiero_sdk_python/query/account_balance_query.py (3)
CryptoGetAccountBalanceQuery(12-175)execute(126-149)set_contract_id(50-63)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)tests/integration/utils.py (2)
IntegrationTestEnv(34-96)env(23-27)src/hiero_sdk_python/hbar.py (1)
to_tinybars(63-65)
🪛 Ruff (0.14.10)
examples/query/contract_balance_query.py
43-43: Avoid specifying long messages outside the exception class
(TRY003)
79-79: Abstract raise to an inner function
(TRY301)
79-79: Avoid specifying long messages outside the exception class
(TRY003)
89-89: Do not catch blind exception: Exception
(BLE001)
tests/unit/account_balance_query_test.py
57-57: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.12)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (11)
CHANGELOG.md (1)
90-90: LGTM: Changelog entry correctly placed.The entry is properly positioned in the "Added" section for new functionality. The format follows Keep a Changelog conventions.
tests/integration/account_balance_query_e2e_test.py (3)
2-2: Import added to fix past review issue.The
osmodule import resolves the NameError flagged in the previous review.
17-38: Excellent integration test coverage.This test properly validates:
- Environment-based configuration with graceful skip
- Observable network behavior (balance retrieval)
- Balance structure and value assertions
- Proper resource cleanup
The assertions align with integration test guidelines by verifying actual state rather than just success.
9-15: Consider adding try block for consistency.The new test at lines 18-38 uses
try-finallyfor cleanup, but this test only has afinallyblock without an explicittry. While functionally equivalent (the entire function body acts as the try block), adding an explicittryblock would improve consistency and readability.♻️ Suggested pattern for consistency
@pytest.mark.integration def test_integration_account_balance_query_can_execute(): env = IntegrationTestEnv() - - try: + + try: CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) finally: env.close()Likely an incorrect or invalid review comment.
tests/unit/account_balance_query_test.py (4)
6-17: Import updates support contract_id functionality.The reorganized imports and addition of
ContractIdproperly support the new contract balance query tests.
67-72: Good constructor test coverage.This test validates that the query can be constructed with a
contract_idand properly initializes attributes, including the mutual exclusivity expectation (account_id should be None).
90-115: Excellent validation test coverage.These two tests properly verify the mutual exclusivity constraints:
- Both account_id and contract_id set → ValueError
- Neither set → ValueError
The use of raw strings with escaped metacharacters in regex patterns addresses the past review feedback.
118-130: Thorough proto population test.This test properly verifies that:
contractIDis populated with the correct protoaccountIDfield is not present (usingHasField)This level of detail protects against proto structure regressions.
examples/query/contract_balance_query.py (3)
31-50: Excellent client setup pattern.The
setup_client()function follows SDK example conventions:
- Clear role-based naming (
operator_id,operator_key)- Proper error handling with descriptive messages
- Environment-based configuration
- User-friendly status output
53-70: Clear demonstration of contract balance query.This function effectively demonstrates:
- Correct usage of
set_contract_id()method- Query execution pattern
- Result inspection and display
- Distinction between account_id and contract_id usage (lines 57-58 comments)
83-85: Helpful educational comment.The commented code demonstrating what would raise an error due to oneof constraints is an excellent teaching tool for users learning the API.
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/unit/account_balance_query_test.py
🧰 Additional context used
📓 Path-based instructions (1)
tests/unit/**/*
⚙️ CodeRabbit configuration file
tests/unit/**/*: You are acting as a senior maintainer reviewing unit tests for the hiero-sdk-python project. Your goal is to ensure tests are extensive, deterministic, and protect against breaking changes.CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically:
- Tests must provide useful error messages when they fail for future debugging.
- No
print()statements - use assertions with descriptive messages.- No timing-dependent or unseeded random assertions.
- No network calls or external dependencies (unit tests are isolated).
- No unjustified TODOs or skipped tests without tracking issues.
PRIORITY 1 - Protect Against Breaking Changes:
- Assert public attributes exist (e.g.,
assert hasattr(obj, 'account_id')).- Assert return types where relevant (e.g.,
assert isinstance(result, AccountId)).- Assert fluent setters return
self(e.g.,assert tx.set_memo("test") is tx).- Assert backward-compatible defaults are maintained.
- If a breaking change is introduced, tests must assert deprecation behavior and test old behavior until removal.
PRIORITY 2 - Constructor & Setter Behavior:
- Test constructor behavior with valid inputs, edge cases, and invalid inputs.
- Test setter behavior including method chaining (fluent interface).
- Verify that setters validate input and raise appropriate exceptions.
- Test that getters return expected values after construction/setting.
PRIORITY 3 - Comprehensive Coverage:
- Unit tests should be extensive - test even if we don't expect users to use it currently.
- Cover happy paths AND unhappy paths/edge cases.
- Test boundary conditions, null/None values, empty collections, etc.
- Avoid brittle ordering assertions unless order is part of the contract.
PRIORITY 4 - No Mocks for Non-Existent Modules:
- All imports must reference actual SDK modules - no hallucinated paths.
- Validate import paths against the actual
src/hiero_sdk_pythonstructure.- Mocks should only be used for external de...
Files:
tests/unit/account_balance_query_test.py
🧬 Code graph analysis (1)
tests/unit/account_balance_query_test.py (2)
src/hiero_sdk_python/query/account_balance_query.py (6)
CryptoGetAccountBalanceQuery(12-175)set_account_id(35-48)execute(126-149)_is_payment_required(168-175)set_contract_id(50-63)_make_request(65-107)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)
🪛 Ruff (0.14.10)
tests/unit/account_balance_query_test.py
57-57: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: run-examples
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.13)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (2)
tests/unit/account_balance_query_test.py (2)
90-115: LGTM! Comprehensive validation coverage.These tests properly verify the mutual exclusion validation logic and error messages. The use of raw strings for regex patterns addresses the previous review feedback.
118-144: LGTM! Protobuf population tests are thorough.These tests properly verify that:
- Only the appropriate protobuf field is populated based on which ID is set
- The other field remains unset (using
HasFieldchecks)- The proto objects match the expected values
The tests adequately protect against incorrect protobuf construction.
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
examples/query/contract_balance_query.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/query/contract_balance_query.py
🪛 Ruff (0.14.10)
examples/query/contract_balance_query.py
47-47: Avoid specifying long messages outside the exception class
(TRY003)
105-105: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (4)
examples/query/contract_balance_query.py (4)
39-54: LGTM!The client setup follows the standard pattern with proper environment variable validation, clear error messages, and appropriate operator configuration.
83-91: LGTM! Correctly demonstrates the new contract_id support.The query pattern correctly uses
CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(client), which is the core objective of this PR. Queries don't requirefreeze_with(), so the execution chain is appropriate.
94-108: LGTM!The main function follows the standard example pattern with appropriate top-level error handling and logical flow. The broad exception catch is acceptable for a main() entry point.
30-30: Both import paths are valid and correctly reference existing modules in the project:
- Line 30:
ContractIdcorrectly imported fromhiero_sdk_python.contract.contract_id- Line 33:
SIMPLE_CONTRACT_BYTECODEcorrectly imported fromexamples.contract.contractsNo changes needed for the imports.
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/hiero_sdk_python/query/account_balance_query.pytests/unit/account_balance_query_test.py
🧰 Additional context used
📓 Path-based instructions (1)
tests/unit/**/*
⚙️ CodeRabbit configuration file
tests/unit/**/*: You are acting as a senior maintainer reviewing unit tests for the hiero-sdk-python project. Your goal is to ensure tests are extensive, deterministic, and protect against breaking changes.CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically:
- Tests must provide useful error messages when they fail for future debugging.
- No
print()statements - use assertions with descriptive messages.- No timing-dependent or unseeded random assertions.
- No network calls or external dependencies (unit tests are isolated).
- No unjustified TODOs or skipped tests without tracking issues.
PRIORITY 1 - Protect Against Breaking Changes:
- Assert public attributes exist (e.g.,
assert hasattr(obj, 'account_id')).- Assert return types where relevant (e.g.,
assert isinstance(result, AccountId)).- Assert fluent setters return
self(e.g.,assert tx.set_memo("test") is tx).- Assert backward-compatible defaults are maintained.
- If a breaking change is introduced, tests must assert deprecation behavior and test old behavior until removal.
PRIORITY 2 - Constructor & Setter Behavior:
- Test constructor behavior with valid inputs, edge cases, and invalid inputs.
- Test setter behavior including method chaining (fluent interface).
- Verify that setters validate input and raise appropriate exceptions.
- Test that getters return expected values after construction/setting.
PRIORITY 3 - Comprehensive Coverage:
- Unit tests should be extensive - test even if we don't expect users to use it currently.
- Cover happy paths AND unhappy paths/edge cases.
- Test boundary conditions, null/None values, empty collections, etc.
- Avoid brittle ordering assertions unless order is part of the contract.
PRIORITY 4 - No Mocks for Non-Existent Modules:
- All imports must reference actual SDK modules - no hallucinated paths.
- Validate import paths against the actual
src/hiero_sdk_pythonstructure.- Mocks should only be used for external de...
Files:
tests/unit/account_balance_query_test.py
🧬 Code graph analysis (1)
tests/unit/account_balance_query_test.py (3)
src/hiero_sdk_python/query/account_balance_query.py (6)
CryptoGetAccountBalanceQuery(12-179)set_account_id(35-50)execute(130-153)_is_payment_required(172-179)set_contract_id(52-67)_make_request(69-111)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)
🪛 Ruff (0.14.10)
src/hiero_sdk_python/query/account_balance_query.py
47-47: Avoid specifying long messages outside the exception class
(TRY003)
64-64: Avoid specifying long messages outside the exception class
(TRY003)
84-84: Abstract raise to an inner function
(TRY301)
84-84: Avoid specifying long messages outside the exception class
(TRY003)
87-87: Abstract raise to an inner function
(TRY301)
87-87: Avoid specifying long messages outside the exception class
(TRY003)
102-104: Abstract raise to an inner function
(TRY301)
102-104: Avoid specifying long messages outside the exception class
(TRY003)
156-156: Dynamically typed expressions (typing.Any) are disallowed in response
(ANN401)
172-172: Missing return type annotation for private function _is_payment_required
Add return type annotation: bool
(ANN202)
tests/unit/account_balance_query_test.py
57-57: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (10)
tests/unit/account_balance_query_test.py (6)
67-71: LGTM! Good invalid input coverage.This test properly validates setter behavior with invalid input types, aligning with unit test guidelines PRIORITY 2 (constructor & setter behavior).
74-78: LGTM! Good invalid input coverage.This test properly validates setter behavior with invalid input types, aligning with unit test guidelines PRIORITY 2 (constructor & setter behavior).
104-118: LGTM! Proper mutual exclusivity validation.This test correctly verifies that
_make_request()raisesValueErrorwhen bothaccount_idandcontract_idare set, protecting against invalid query construction. The regex pattern is properly escaped.
121-129: LGTM! Proper requirement validation.This test correctly verifies that
_make_request()raisesValueErrorwhen neitheraccount_idnorcontract_idis set, ensuring at least one ID is required. The regex pattern is properly escaped.
132-144: LGTM! Thorough protobuf field population check.This test properly verifies that
_make_request()populates only thecontractIDfield whencontract_idis set, and correctly usesHasField()to confirmaccountIDis not set.
146-158: LGTM! Thorough protobuf field population check.This test properly verifies that
_make_request()populates only theaccountIDfield whenaccount_idis set, and correctly usesHasField()to confirmcontractIDis not set.src/hiero_sdk_python/query/account_balance_query.py (4)
20-33: LGTM! Clean constructor design.The constructor properly accepts both
account_idandcontract_idas optional parameters, storing them as instance attributes. Deferred validation in_make_request()allows flexible query construction while enforcing mutual exclusivity at request time.
35-50: LGTM! Robust setter with proper validation.The setter correctly:
- Validates input type with clear error message
- Resets
contract_idto maintain mutual exclusivity- Returns
selffor method chainingThe type validation protects against programming errors at the point of use.
52-67: LGTM! Symmetric setter design.The
set_contract_id()method mirrorsset_account_id()design:
- Validates input type with clear error message
- Resets
account_idto maintain mutual exclusivity- Returns
selffor method chainingThe symmetric design provides consistent API ergonomics.
69-111: LGTM! Correct validation and request population.The
_make_request()method properly:
- Validates exactly one ID is set (lines 83-87)
- Populates the appropriate protobuf field based on which ID is set (lines 95-98)
- Uses clear, descriptive error messages for validation failures
The else clause at line 98 is safe because the validation at lines 83-87 guarantees that if
account_idis not set, thencontract_idmust be set.
Signed-off-by: Antonio Ceppellini <[email protected]>
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
examples/contract/contract_balance_query.py
🧰 Additional context used
📓 Path-based instructions (1)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/contract/contract_balance_query.py
🧬 Code graph analysis (1)
examples/contract/contract_balance_query.py (3)
src/hiero_sdk_python/query/account_balance_query.py (1)
CryptoGetAccountBalanceQuery(12-179)src/hiero_sdk_python/hbar.py (2)
Hbar(18-213)to_tinybars(63-65)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)
🪛 Ruff (0.14.10)
examples/contract/contract_balance_query.py
47-47: Avoid specifying long messages outside the exception class
(TRY003)
71-71: Avoid specifying long messages outside the exception class
(TRY003)
91-91: Local variable initial_balance_tinybars is assigned to but never used
Remove assignment to unused variable initial_balance_tinybars
(F841)
97-97: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (1)
examples/contract/contract_balance_query.py (1)
76-84: LGTM! Correct demonstration of the new contract_id API.The
get_contract_balance()function correctly demonstrates the newset_contract_id()method onCryptoGetAccountBalanceQuery, which is the core objective of this example.
|
Hi, this is MergeConflictBot. Please resolve these conflicts locally and push the changes. To assist you, please read: Thank you for contributing! |
| from .contracts import SIMPLE_CONTRACT_BYTECODE | ||
| from hiero_sdk_python.response_code import ResponseCode | ||
|
|
||
| load_dotenv() |
There was a problem hiding this comment.
Please see the client class on how you can update line 35-55 this to like 1-3 lines of code :)
We have a new method
you can also search for:
def demonstrate_fast_setup():
| print(f"Querying balance for contract {contract_id} ...") | ||
| balance = CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(client) | ||
|
|
||
| print("✅Balance retrieved successfully!") |
There was a problem hiding this comment.
It is more accurate to check the response code,
e.g.
def response_name(receipt: "TransactionReceipt"):
status_code = ResponseCode(receipt.status)
status_name = status_code.name
print(f"The response name is {status_name}")
if status_name == ResponseCode.SUCCESS.name:
print("✅ Transaction succeeded!")
elif status_code.is_unknown:
print(f"❓ Unknown transaction status: {status_name}")
else:
print("❌ Transaction failed!") if receipt.status != ResponseCode.SUCCESS:
print(f"Hbar transfer failed with status: {ResponseCode(receipt.status).name}")
sys.exit(1)
print(
f"Successfully transferred {amount} from {owner_account_id} "
f"to {receiver_account_id} using allowance"
)| try: | ||
| client = setup_client() | ||
|
|
||
| initial_balance_tinybars = Hbar(1).to_tinybars() |
There was a problem hiding this comment.
can we pass hbars directly?
src/hiero_sdk_python/hbar_unit.py
src/hiero_sdk_python/hbar.py
or do we not yet support?
There was a problem hiding this comment.
we do not support yet, right now we need to pass int:
| def test_integration_contract_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
|
|
||
| contract_id_str = os.getenv("HEDERA_TEST_CONTRACT_ID") |
There was a problem hiding this comment.
can you use the contract utils in examples?
or update env? I don' tknow if we have this on solo
|
|
||
| from hiero_sdk_python.query.account_balance_query import CryptoGetAccountBalanceQuery | ||
| from hiero_sdk_python.contract.contract_id import ContractId | ||
| from tests.integration.utils import IntegrationTestEnv |
There was a problem hiding this comment.
I would want to see some unhappy path tests added, similar to the other query and contract tests
Signed-off-by: Antonio Ceppellini <[email protected]>
…countBalanceQuery Signed-off-by: AntonioCeppellini <[email protected]>
thank you @exploreriii :D |
There was a problem hiding this comment.
Actionable comments posted: 8
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (3)
CHANGELOG.mdexamples/contract/contract_balance_query.pytests/integration/account_balance_query_e2e_test.py
🧰 Additional context used
📓 Path-based instructions (2)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/contract/contract_balance_query.py
tests/integration/**/*
⚙️ CodeRabbit configuration file
tests/integration/**/*: You are acting as a senior maintainer reviewing integration tests for the hiero-sdk-python project. Your goal is to ensure end-to-end tests validate real network behavior safely and deterministically.CRITICAL PRINCIPLES - Safety & Diagnosability:
- Prioritize safety: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just
SUCCESS.- Failure-path tests must assert specific
ResponseCodevalues (e.g.,TOKEN_HAS_NO_PAUSE_KEY).PRIORITY 1 - End-to-End Behavior:
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
- Validate resulting balances, ownership, and state changes (not just transaction success).
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
- Verify network state after operations (e.g., account balance changed, token transferred).
PRIORITY 2 - Test Structure & Maintainability:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller modules.
- Are helper functions good candidates for pytest fixtures or shared utilities?
PRIORITY 3 - Isolation & Cleanup:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle assumptions).
PRIORITY 4 - Assertions & Coverage:
- Do tests validate only success/failure, or also assert resulting state?
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
- Cover happy paths AND ...
Files:
tests/integration/account_balance_query_e2e_test.py
🧠 Learnings (1)
📚 Learning: 2026-01-08T10:28:14.391Z
Learnt from: github-actions[bot]
Repo: hiero-ledger/hiero-sdk-python PR: 0
File: :0-0
Timestamp: 2026-01-08T10:28:14.391Z
Learning: In hiero-ledger/hiero-sdk-python, examples should prefer Client.from_env() (src/hiero_sdk_python/client/client.py) for client setup. For examples under examples/tokens, replace manual Network/AccountId/PrivateKey + load_dotenv() bootstrapping with Client.from_env(), and use client.operator_account_id and client.operator_private_key instead of passing operator_id/operator_key through functions.
Applied to files:
examples/contract/contract_balance_query.pyCHANGELOG.mdtests/integration/account_balance_query_e2e_test.py
🧬 Code graph analysis (1)
tests/integration/account_balance_query_e2e_test.py (7)
src/hiero_sdk_python/transaction/transaction_receipt.py (1)
account_id(88-100)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/query/account_balance_query.py (4)
CryptoGetAccountBalanceQuery(12-179)execute(130-153)set_contract_id(52-67)set_account_id(35-50)src/hiero_sdk_python/contract/contract_create_transaction.py (2)
ContractCreateTransaction(73-437)set_bytecode(133-149)src/hiero_sdk_python/response_code.py (1)
ResponseCode(4-387)tests/integration/utils.py (2)
IntegrationTestEnv(34-96)env(23-27)
🪛 Ruff (0.14.10)
examples/contract/contract_balance_query.py
40-40: Do not catch blind exception: Exception
(BLE001)
96-96: Do not catch blind exception: Exception
(BLE001)
tests/integration/account_balance_query_e2e_test.py
16-16: Missing return type annotation for private function _create_test_contract
(ANN202)
28-30: Avoid specifying long messages outside the exception class
(TRY003)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
65-65: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
80-80: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
92-92: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
104-104: Pattern passed to match= contains metacharacters but is neither escaped nor raw
(RUF043)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.13)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (5)
CHANGELOG.md (1)
91-91: LGTM!The changelog entry correctly documents the new
contract_idsupport feature with a proper issue reference link.examples/contract/contract_balance_query.py (2)
35-44: LGTM!The
setup_client()function follows the recommended pattern usingClient.from_env()with appropriate error handling and clear messaging.
51-58: No changes needed. The transaction lifecycle is correct forContractCreateTransaction.
ContractCreateTransactionauto-signs with the operator credentials duringexecute(client)and does not require explicitfreeze_with(client)before execution. The pattern of direct chaining toexecute(client)is correct and consistent across all contract examples in the codebase. Thefreeze_with(client)requirement applies only to transactions that use explicit.sign(private_key)calls, which this example does not have.Likely an incorrect or invalid review comment.
tests/integration/account_balance_query_e2e_test.py (2)
1-13: LGTM!Imports are well-organized and the module-level
pytestmarkcorrectly marks all tests as integration tests.
38-45: LGTM!Test correctly validates account balance query execution with appropriate cleanup via
try/finally.
| def test_integration_contract_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| contract_id = _create_test_contract(env) | ||
|
|
||
| balance = CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(env.client) | ||
|
|
||
| assert balance is not None | ||
| assert hasattr(balance, "hbars") | ||
| assert balance.hbars.to_tinybars() >= 0 | ||
| finally: | ||
| env.close() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider asserting exact expected balance.
The contract is created without an initial balance, so its balance should be exactly 0. A more precise assertion would strengthen this test.
♻️ Suggested enhancement
assert balance is not None
assert hasattr(balance, "hbars")
- assert balance.hbars.to_tinybars() >= 0
+ assert balance.hbars.to_tinybars() == 0, "Contract created without initial balance should have 0 hbars"Signed-off-by: Antonio Ceppellini <[email protected]>
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
examples/contract/contract_balance_query.pytests/integration/account_balance_query_e2e_test.py
🧰 Additional context used
📓 Path-based instructions (2)
examples/**/*
⚙️ CodeRabbit configuration file
examples/**/*: You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.Priority 1 - Correctness:
- Verify transaction lifecycle chain (construction -> freeze_with -> sign -> execute).
- Ensure
freeze_with(client)is called BEFORE signing.- Validate that methods referenced actually exist in the
hiero_sdk_pythoncodebase.- Ensure response validation checks
receipt.statusagainstResponseCodeenums (e.g.,ResponseCode.SUCCESS).Priority 2 - Transaction Lifecycle:
- Check method chaining logic.
- Verify correct signing order (especially for multi-sig).
- Ensure explicit
.execute(client)calls.- Verify response property extraction (e.g., using
.token_id,.account_id,.serial_numbers).- Ensure error handling uses
ResponseCode(receipt.status).namefor clarity.Priority 3 - Naming & Clarity:
- Enforce role-based naming:
operator_id/_key,treasury_account_id/_key,receiver_id/_key.- Use
_idsuffix for AccountId and_keysuffix for PrivateKey variables.- Validate negative examples explicitly check for failure codes (e.g.,
TOKEN_HAS_NO_PAUSE_KEY).- Ensure logical top-to-bottom flow without ambiguity.
Priority 4 - Consistency:
- Verify standard patterns:
def main(),if __name__ == "__main__":,load_dotenv().- IMPORT RULES:
- Accept both top-level imports (e.g.,
from hiero_sdk_python import PrivateKey) and fully qualified imports (e.g.,from hiero_sdk_python.crypto.private_key import PrivateKey).- STRICTLY validate that the import path actually exists in the project structure. Compare against other files in
/examplesor your knowledge of the SDK file tree.- Flag hallucinations immediately (e.g.,
hiero_sdk_python.keysdoes not exist).- Check for
try-exceptblocks withsys.exit(1)for critical failures.Priority 5 - User Experience:
- Ensure comments explain SDK usage patterns (for users,...
Files:
examples/contract/contract_balance_query.py
tests/integration/**/*
⚙️ CodeRabbit configuration file
tests/integration/**/*: You are acting as a senior maintainer reviewing integration tests for the hiero-sdk-python project. Your goal is to ensure end-to-end tests validate real network behavior safely and deterministically.CRITICAL PRINCIPLES - Safety & Diagnosability:
- Prioritize safety: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just
SUCCESS.- Failure-path tests must assert specific
ResponseCodevalues (e.g.,TOKEN_HAS_NO_PAUSE_KEY).PRIORITY 1 - End-to-End Behavior:
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
- Validate resulting balances, ownership, and state changes (not just transaction success).
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
- Verify network state after operations (e.g., account balance changed, token transferred).
PRIORITY 2 - Test Structure & Maintainability:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller modules.
- Are helper functions good candidates for pytest fixtures or shared utilities?
PRIORITY 3 - Isolation & Cleanup:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle assumptions).
PRIORITY 4 - Assertions & Coverage:
- Do tests validate only success/failure, or also assert resulting state?
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
- Cover happy paths AND ...
Files:
tests/integration/account_balance_query_e2e_test.py
🧠 Learnings (1)
📚 Learning: 2026-01-08T10:28:14.391Z
Learnt from: github-actions[bot]
Repo: hiero-ledger/hiero-sdk-python PR: 0
File: :0-0
Timestamp: 2026-01-08T10:28:14.391Z
Learning: In hiero-ledger/hiero-sdk-python, examples should prefer Client.from_env() (src/hiero_sdk_python/client/client.py) for client setup. For examples under examples/tokens, replace manual Network/AccountId/PrivateKey + load_dotenv() bootstrapping with Client.from_env(), and use client.operator_account_id and client.operator_private_key instead of passing operator_id/operator_key through functions.
Applied to files:
examples/contract/contract_balance_query.pytests/integration/account_balance_query_e2e_test.py
🧬 Code graph analysis (2)
examples/contract/contract_balance_query.py (5)
src/hiero_sdk_python/client/client.py (1)
Client(28-255)src/hiero_sdk_python/query/account_balance_query.py (1)
CryptoGetAccountBalanceQuery(12-179)src/hiero_sdk_python/hbar.py (2)
Hbar(18-213)to_tinybars(63-65)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/response_code.py (2)
ResponseCode(4-387)is_unknown(374-375)
tests/integration/account_balance_query_e2e_test.py (4)
src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/query/account_balance_query.py (4)
CryptoGetAccountBalanceQuery(12-179)execute(130-153)set_contract_id(52-67)set_account_id(35-50)tests/integration/utils.py (2)
IntegrationTestEnv(34-96)env(23-27)
🪛 Ruff (0.14.10)
examples/contract/contract_balance_query.py
39-39: Do not catch blind exception: Exception
(BLE001)
95-95: Do not catch blind exception: Exception
(BLE001)
tests/integration/account_balance_query_e2e_test.py
16-16: Missing return type annotation for private function _create_test_contract
(ANN202)
28-30: Avoid specifying long messages outside the exception class
(TRY003)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.11)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (1)
examples/contract/contract_balance_query.py (1)
19-26: Optional: Consider consistent import style for ContractId.Lines 19-24 use top-level imports from
hiero_sdk_python, while line 26 uses a fully qualified import forContractId. Both styles are acceptable, but for consistency you might consider importingContractIdfrom the top-level package if it's exported there (e.g.,from hiero_sdk_python import ContractId).⛔ Skipped due to learnings
Learnt from: github-actions[bot] Repo: hiero-ledger/hiero-sdk-python PR: 0 File: :0-0 Timestamp: 2026-01-08T10:28:14.391Z Learning: In hiero-ledger/hiero-sdk-python, examples should prefer Client.from_env() (src/hiero_sdk_python/client/client.py) for client setup. For examples under examples/tokens, replace manual Network/AccountId/PrivateKey + load_dotenv() bootstrapping with Client.from_env(), and use client.operator_account_id and client.operator_private_key instead of passing operator_id/operator_key through functions.
| def test_integration_balance_query_raises_when_both_sources_set(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| query = CryptoGetAccountBalanceQuery( | ||
| account_id=env.operator_id, | ||
| contract_id=ContractId(0, 0, 1234), | ||
| ) | ||
|
|
||
| with pytest.raises(ValueError, match=r"Specify either account_id or contract_id, not both\."): | ||
| query.execute(env.client) | ||
| finally: | ||
| env.close() |
There was a problem hiding this comment.
Fix the error message pattern to match actual ValueError.
The regex pattern on line 79 expects lowercase "account_id" and "contract_id", but the actual error message from account_balance_query.py line 91 uses "Account ID" and "Contract ID" with capital letters and spaces.
🐛 Proposed fix for the regex pattern
- with pytest.raises(ValueError, match=r"Specify either account_id or contract_id, not both\."):
+ with pytest.raises(ValueError, match=r"Specify either Account ID or Contract ID, not both\."):
query.execute(env.client)There was a problem hiding this comment.
Replace Specify either account_id or contract_id, not both\. with Specify either Account ID or Contract ID, not both\. this will fix test errors
Also I think it would be cleaner to instead change the source i.e (account_balance_query.py) error message to use the actual attribute names:
Specify either account_id or contract_id, not both.
Either account_id or contract_id must be set before making the request.
| """ | ||
| super().__init__() | ||
| self.account_id: Optional[AccountId] = account_id | ||
| self.contract_id: Optional[ContractId] = contract_id |
There was a problem hiding this comment.
To ensure that only one of account or contract id must be set we can use setters.
if account_id is not None:
self.set_account_id(account_id)
if contract_id is not None:
self.set_contact_id(contract_id).This guarantees that only one of the two fields is set at any time
There was a problem hiding this comment.
right now i'm doing the reset inside the setters, if we do like that in the
def __init__():
then the last setter used will win above the other because it will reset the other one (account_id vs contract_id).
what do you suggest to use?
I like your way and maybe could live with the reset inside the setters, thinking about that is probably safer, it will block always one of the two (acccount_id or contract_id) when both are passed
| def test_integration_balance_query_with_invalid_account_id_raises(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(ValueError, match=r"account_id must be an AccountId\."): |
There was a problem hiding this comment.
This test currently asserting ValueError but set_account_id() raises a TypeError on invalid AccountId
| def test_integration_balance_query_with_invalid_contract_id_raises(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(ValueError, match=r"contract_id must be a ContractId\."): |
There was a problem hiding this comment.
Same with this test currently asserting ValueError but set_contract_id() raises a TypeError on invalid ContractId
manishdait
left a comment
manishdait
left a comment
There was a problem hiding this comment.
Hi @AntonioCeppellini, great work! I’ve left a suggestions and small change requests.
aceppaluni
left a comment
aceppaluni
left a comment
There was a problem hiding this comment.
@AntonioCeppellini This is looking great 👍
I concur with @manishdait and @exploreriii on the suggested changes.
If you need assistance or have any questions please let us know!
Signed-off-by: Antonio Ceppellini <[email protected]>
|
thank you very much @manishdait @aceppaluni |
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
tests/unit/account_balance_query_test.py (1)
25-29: Add breaking change protection assertions.Per coding guidelines PRIORITY 1, add assertions to protect against breaking changes to public attributes and types.
✨ Suggested enhancement for breaking change protection
def test_build_account_balance_query(mock_account_ids): """Test building a CryptoGetAccountBalanceQuery with a valid account ID.""" account_id_sender, *_ = mock_account_ids query = CryptoGetAccountBalanceQuery(account_id=account_id_sender) assert query.account_id == account_id_sender + assert isinstance(query.account_id, AccountId) + assert query.contract_id is None + assert hasattr(query, 'account_id') + assert hasattr(query, 'contract_id')Based on coding guidelines: "Assert public attributes exist" and "Assert return types where relevant" to protect against breaking changes.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
src/hiero_sdk_python/query/account_balance_query.pytests/unit/account_balance_query_test.py
🧰 Additional context used
📓 Path-based instructions (1)
tests/unit/**/*
⚙️ CodeRabbit configuration file
tests/unit/**/*: You are acting as a senior maintainer reviewing unit tests for the hiero-sdk-python project. Your goal is to ensure tests are extensive, deterministic, and protect against breaking changes.CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically:
- Tests must provide useful error messages when they fail for future debugging.
- No
print()statements - use assertions with descriptive messages.- No timing-dependent or unseeded random assertions.
- No network calls or external dependencies (unit tests are isolated).
- No unjustified TODOs or skipped tests without tracking issues.
PRIORITY 1 - Protect Against Breaking Changes:
- Assert public attributes exist (e.g.,
assert hasattr(obj, 'account_id')).- Assert return types where relevant (e.g.,
assert isinstance(result, AccountId)).- Assert fluent setters return
self(e.g.,assert tx.set_memo("test") is tx).- Assert backward-compatible defaults are maintained.
- If a breaking change is introduced, tests must assert deprecation behavior and test old behavior until removal.
PRIORITY 2 - Constructor & Setter Behavior:
- Test constructor behavior with valid inputs, edge cases, and invalid inputs.
- Test setter behavior including method chaining (fluent interface).
- Verify that setters validate input and raise appropriate exceptions.
- Test that getters return expected values after construction/setting.
PRIORITY 3 - Comprehensive Coverage:
- Unit tests should be extensive - test even if we don't expect users to use it currently.
- Cover happy paths AND unhappy paths/edge cases.
- Test boundary conditions, null/None values, empty collections, etc.
- Avoid brittle ordering assertions unless order is part of the contract.
PRIORITY 4 - No Mocks for Non-Existent Modules:
- All imports must reference actual SDK modules - no hallucinated paths.
- Validate import paths against the actual
src/hiero_sdk_pythonstructure.- Mocks should only be used for external de...
Files:
tests/unit/account_balance_query_test.py
🧬 Code graph analysis (2)
src/hiero_sdk_python/query/account_balance_query.py (2)
src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/query/query.py (2)
Query(29-376)_get_query_response(61-77)
tests/unit/account_balance_query_test.py (3)
src/hiero_sdk_python/query/account_balance_query.py (6)
CryptoGetAccountBalanceQuery(12-184)set_account_id(40-55)execute(135-158)_is_payment_required(177-184)set_contract_id(57-72)_make_request(74-116)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)
🪛 Ruff (0.14.10)
src/hiero_sdk_python/query/account_balance_query.py
52-52: Avoid specifying long messages outside the exception class
(TRY003)
69-69: Avoid specifying long messages outside the exception class
(TRY003)
89-89: Abstract raise to an inner function
(TRY301)
89-89: Avoid specifying long messages outside the exception class
(TRY003)
92-92: Abstract raise to an inner function
(TRY301)
92-92: Avoid specifying long messages outside the exception class
(TRY003)
107-109: Abstract raise to an inner function
(TRY301)
107-109: Avoid specifying long messages outside the exception class
(TRY003)
161-161: Dynamically typed expressions (typing.Any) are disallowed in response
(ANN401)
177-177: Missing return type annotation for private function _is_payment_required
Add return type annotation: bool
(ANN202)
tests/unit/account_balance_query_test.py
57-57: Do not catch blind exception: Exception
(BLE001)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.11)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (9)
tests/unit/account_balance_query_test.py (4)
6-17: LGTM: Import organization is clean.The imports are properly organized with grouped multiline imports from
hapi.servicesand appropriate additions forCryptoGetAccountBalanceResponseandContractId.
32-58: LGTM: Execution test with appropriate exception handling.The test correctly verifies that the query executes without raising exceptions. The broad
Exceptioncatch flagged by static analysis (line 57) is appropriate in this test context, as the intent is to catch and fail on any unexpected error during execution.
119-127: LGTM: Proper validation test for missing identifiers.Correctly verifies that
_make_request()raises a clearValueErrorwhen neitheraccount_idnorcontract_idis set, with an appropriate error message.
130-156: Excellent protobuf field population tests.These tests thoroughly verify the correct protobuf wiring:
- Correct field is populated (
contractIDvsaccountID)- Mutual exclusivity is maintained (other field is unset)
- Proper use of
HasField()to verify field presence- Correct comparison using
_to_proto()methodsThis provides strong protection against regression in the request construction logic.
src/hiero_sdk_python/query/account_balance_query.py (5)
9-9: LGTM: Correct import addition.The
ContractIdimport is properly added to support the new contract-based balance query functionality.
40-55: LGTM: Type validation and mutual exclusivity enforcement.The method correctly:
- Validates that
account_idis anAccountIdinstance- Resets
contract_idtoNoneto enforce mutual exclusivity- Returns
selffor method chaining- Updates docstring to document the reset behavior
The static analysis warning (TRY003) about the error message length is a minor style issue and acceptable in this context.
57-72: LGTM: Well-implemented set_contract_id method.The new method follows the same pattern as
set_account_id:
- Type validation with descriptive error
- Mutual exclusivity by resetting
account_id- Fluent interface (returns
self)- Clear docstring documenting the reset behavior
Good consistency in API design.
74-116: LGTM: Robust validation with good defensive programming.The
_make_request()method correctly:
- Validates that at least one ID is set (line 88-89)
- Includes defensive validation that both aren't set (lines 91-92)
- Populates the appropriate protobuf field based on which ID is set (lines 100-103)
Note on defensive validation: The check for both IDs being set (lines 91-92) is technically unreachable through the public API since the setters enforce mutual exclusivity. However, this is good defensive programming that protects against future changes or direct attribute manipulation.
The static analysis warnings (TRY301, TRY003) are minor style issues and the current approach is clear and maintainable.
160-175: LGTM: Explicit return type annotation.The method signature is now explicit about the return type, which improves code clarity and type checking. The
Anytype forresponseparameter is acceptable as it matches the base class pattern and the actual response type varies by query.The static analysis warning (ANN401) about
Anyis acceptable in this context.
| def __init__( | ||
| self, | ||
| account_id: Optional[AccountId] = None, | ||
| contract_id: Optional[ContractId] = None, | ||
| ) -> None: | ||
| """ | ||
| Initializes a new instance of the CryptoGetAccountBalanceQuery class. | ||
| Args: | ||
| account_id (AccountId, optional): The ID of the account to retrieve the balance for. | ||
| """ | ||
| super().__init__() | ||
| self.account_id: Optional[AccountId] = account_id | ||
| self.account_id: Optional[AccountId] = None | ||
| self.contract_id: Optional[ContractId] = None | ||
|
|
||
| if account_id is not None: | ||
| self.set_account_id(account_id) | ||
| if contract_id is not None: | ||
| self.set_contract_id(contract_id) |
There was a problem hiding this comment.
Update docstring to document contract_id parameter.
The __init__ method now accepts a contract_id parameter, but the docstring only documents account_id.
📝 Update docstring
def __init__(
self,
account_id: Optional[AccountId] = None,
contract_id: Optional[ContractId] = None,
) -> None:
"""
Initializes a new instance of the CryptoGetAccountBalanceQuery class.
Args:
account_id (AccountId, optional): The ID of the account to retrieve the balance for.
+ contract_id (ContractId, optional): The ID of the contract to retrieve the balance for.
+
+ Note:
+ If both account_id and contract_id are provided, contract_id takes precedence
+ due to the order of setter invocation.
"""| def _is_payment_required(self): | ||
| """ | ||
| Account balance query does not require payment. | ||
| Returns: | ||
| bool: False | ||
| """ | ||
| return False | ||
| return False |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider adding return type annotation.
The method logic is correct. Static analysis suggests adding an explicit return type annotation for consistency.
♻️ Add return type annotation
- def _is_payment_required(self):
+ def _is_payment_required(self) -> bool:
"""
Account balance query does not require payment.
Returns:
bool: False
"""
return FalseThis is a minor improvement for type safety and code clarity.
🧰 Tools
🪛 Ruff (0.14.10)
177-177: Missing return type annotation for private function _is_payment_required
Add return type annotation: bool
(ANN202)
| def test_set_account_id_with_invalid_type_raises(): | ||
| query = CryptoGetAccountBalanceQuery() | ||
|
|
||
| with pytest.raises(TypeError, match=r"account_id must be an AccountId\."): | ||
| query.set_account_id("ciao") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing fluent interface assertion for set_account_id.
Per coding guidelines PRIORITY 1, fluent setters must be tested to ensure they return self for method chaining. This test validates type checking but doesn't verify the return value.
✨ Add fluent interface test
Consider adding a complementary test:
def test_set_account_id_returns_self_for_chaining():
"""set_account_id should return self to enable method chaining."""
query = CryptoGetAccountBalanceQuery()
account_id = AccountId(0, 0, 1800)
result = query.set_account_id(account_id)
assert result is query
assert isinstance(result, CryptoGetAccountBalanceQuery)Based on coding guidelines: "Assert fluent setters return self" for breaking change protection.
| def test_set_contract_id_with_invalid_type_raises(): | ||
| query = CryptoGetAccountBalanceQuery() | ||
|
|
||
| with pytest.raises(TypeError, match=r"contract_id must be a ContractId\."): | ||
| query.set_contract_id("ciao") |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing fluent interface assertion for set_contract_id.
Similar to set_account_id, this test validates type checking but doesn't verify the fluent interface contract.
✨ Add fluent interface test
Consider adding a complementary test:
def test_set_contract_id_returns_self_for_chaining():
"""set_contract_id should return self to enable method chaining."""
query = CryptoGetAccountBalanceQuery()
contract_id = ContractId(0, 0, 1234)
result = query.set_contract_id(contract_id)
assert result is query
assert isinstance(result, CryptoGetAccountBalanceQuery)Based on coding guidelines: "Assert fluent setters return self" for breaking change protection.
| def test_build_account_balance_query_with_contract_id(): | ||
| """Test building a CryptoGetAccountBalanceQuery with a valid contract ID.""" | ||
| contract_id = ContractId(0, 0, 1234) | ||
| query = CryptoGetAccountBalanceQuery(contract_id=contract_id) | ||
| assert query.contract_id == contract_id | ||
| assert query.account_id is None |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Add type assertions for breaking change protection.
The test correctly verifies the values but should also assert types to protect against breaking changes.
✨ Add type and attribute assertions
def test_build_account_balance_query_with_contract_id():
"""Test building a CryptoGetAccountBalanceQuery with a valid contract ID."""
contract_id = ContractId(0, 0, 1234)
query = CryptoGetAccountBalanceQuery(contract_id=contract_id)
assert query.contract_id == contract_id
+ assert isinstance(query.contract_id, ContractId)
assert query.account_id is None
+ assert hasattr(query, 'contract_id')
+ assert hasattr(query, 'account_id')Based on coding guidelines: "Assert return types where relevant" and "Assert public attributes exist."
| def test_set_contract_id_method_chaining_resets_account_id(mock_account_ids): | ||
| """set_contract_id should support chaining and reset account_id.""" | ||
| account_id_sender, *_ = mock_account_ids | ||
| contract_id = ContractId(0, 0, 1234) | ||
|
|
||
| query = ( | ||
| CryptoGetAccountBalanceQuery() | ||
| .set_account_id(account_id_sender) | ||
| .set_contract_id(contract_id) | ||
| ) | ||
|
|
||
| assert query.contract_id == contract_id | ||
| assert query.account_id is None |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Consider explicit fluent interface assertion.
The test demonstrates method chaining works (implicitly testing that setters return self), but could be more explicit per PRIORITY 1 guidelines.
♻️ Optional: Make fluent interface explicit
def test_set_contract_id_method_chaining_resets_account_id(mock_account_ids):
"""set_contract_id should support chaining and reset account_id."""
account_id_sender, *_ = mock_account_ids
contract_id = ContractId(0, 0, 1234)
- query = (
+ query_obj = CryptoGetAccountBalanceQuery()
+ query = (
+ query_obj
- CryptoGetAccountBalanceQuery()
.set_account_id(account_id_sender)
.set_contract_id(contract_id)
)
+ assert query is query_obj # Verify fluent interface
assert query.contract_id == contract_id
assert query.account_id is NoneBased on coding guidelines: "Assert fluent setters return self" for explicit breaking change protection.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_set_contract_id_method_chaining_resets_account_id(mock_account_ids): | |
| """set_contract_id should support chaining and reset account_id.""" | |
| account_id_sender, *_ = mock_account_ids | |
| contract_id = ContractId(0, 0, 1234) | |
| query = ( | |
| CryptoGetAccountBalanceQuery() | |
| .set_account_id(account_id_sender) | |
| .set_contract_id(contract_id) | |
| ) | |
| assert query.contract_id == contract_id | |
| assert query.account_id is None | |
| def test_set_contract_id_method_chaining_resets_account_id(mock_account_ids): | |
| """set_contract_id should support chaining and reset account_id.""" | |
| account_id_sender, *_ = mock_account_ids | |
| contract_id = ContractId(0, 0, 1234) | |
| query_obj = CryptoGetAccountBalanceQuery() | |
| query = ( | |
| query_obj | |
| .set_account_id(account_id_sender) | |
| .set_contract_id(contract_id) | |
| ) | |
| assert query is query_obj # Verify fluent interface | |
| assert query.contract_id == contract_id | |
| assert query.account_id is None |
| def test_last_wins_when_both_account_id_and_contract_id_are_set( | ||
| mock_account_ids, | ||
| ): | ||
| """_make_request should raise if both account_id and contract_id are set.""" | ||
| account_id_sender, *_ = mock_account_ids | ||
| contract_id = ContractId(0, 0, 1234) | ||
|
|
||
| query = CryptoGetAccountBalanceQuery( | ||
| account_id=account_id_sender, contract_id=contract_id | ||
| ) | ||
|
|
||
| assert query.contract_id == contract_id | ||
| assert query.account_id is None |
There was a problem hiding this comment.
Fix incorrect docstring that contradicts test behavior.
The docstring states "_make_request should raise if both account_id and contract_id are set" but the test never calls _make_request() and doesn't test for exceptions. The test name correctly describes the behavior (last wins), but the docstring is misleading.
📝 Correct the docstring
def test_last_wins_when_both_account_id_and_contract_id_are_set(
mock_account_ids,
):
- """_make_request should raise if both account_id and contract_id are set."""
+ """When both IDs are passed to constructor, contract_id wins (processed last)."""
account_id_sender, *_ = mock_account_ids
contract_id = ContractId(0, 0, 1234)
query = CryptoGetAccountBalanceQuery(
account_id=account_id_sender, contract_id=contract_id
)
assert query.contract_id == contract_id
assert query.account_id is NoneNote: This test verifies the constructor's parameter processing order (contract_id setter is called after account_id setter, so contract_id "wins"). The actual validation that prevents both from being set simultaneously is tested in test_make_request_raises_when_neither_account_id_nor_contract_id_is_set.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| def test_last_wins_when_both_account_id_and_contract_id_are_set( | |
| mock_account_ids, | |
| ): | |
| """_make_request should raise if both account_id and contract_id are set.""" | |
| account_id_sender, *_ = mock_account_ids | |
| contract_id = ContractId(0, 0, 1234) | |
| query = CryptoGetAccountBalanceQuery( | |
| account_id=account_id_sender, contract_id=contract_id | |
| ) | |
| assert query.contract_id == contract_id | |
| assert query.account_id is None | |
| def test_last_wins_when_both_account_id_and_contract_id_are_set( | |
| mock_account_ids, | |
| ): | |
| """When both IDs are passed to constructor, contract_id wins (processed last).""" | |
| account_id_sender, *_ = mock_account_ids | |
| contract_id = ContractId(0, 0, 1234) | |
| query = CryptoGetAccountBalanceQuery( | |
| account_id=account_id_sender, contract_id=contract_id | |
| ) | |
| assert query.contract_id == contract_id | |
| assert query.account_id is None |
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 3
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/integration/account_balance_query_e2e_test.py
🧰 Additional context used
📓 Path-based instructions (1)
tests/integration/**/*
⚙️ CodeRabbit configuration file
tests/integration/**/*: You are acting as a senior maintainer reviewing integration tests for the hiero-sdk-python project. Your goal is to ensure end-to-end tests validate real network behavior safely and deterministically.CRITICAL PRINCIPLES - Safety & Diagnosability:
- Prioritize safety: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just
SUCCESS.- Failure-path tests must assert specific
ResponseCodevalues (e.g.,TOKEN_HAS_NO_PAUSE_KEY).PRIORITY 1 - End-to-End Behavior:
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
- Validate resulting balances, ownership, and state changes (not just transaction success).
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
- Verify network state after operations (e.g., account balance changed, token transferred).
PRIORITY 2 - Test Structure & Maintainability:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller modules.
- Are helper functions good candidates for pytest fixtures or shared utilities?
PRIORITY 3 - Isolation & Cleanup:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle assumptions).
PRIORITY 4 - Assertions & Coverage:
- Do tests validate only success/failure, or also assert resulting state?
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
- Cover happy paths AND ...
Files:
tests/integration/account_balance_query_e2e_test.py
🧠 Learnings (1)
📚 Learning: 2026-01-08T10:28:14.391Z
Learnt from: github-actions[bot]
Repo: hiero-ledger/hiero-sdk-python PR: 0
File: :0-0
Timestamp: 2026-01-08T10:28:14.391Z
Learning: In hiero-ledger/hiero-sdk-python, examples should prefer Client.from_env() (src/hiero_sdk_python/client/client.py) for client setup. For examples under examples/tokens, replace manual Network/AccountId/PrivateKey + load_dotenv() bootstrapping with Client.from_env(), and use client.operator_account_id and client.operator_private_key instead of passing operator_id/operator_key through functions.
Applied to files:
tests/integration/account_balance_query_e2e_test.py
🧬 Code graph analysis (1)
tests/integration/account_balance_query_e2e_test.py (5)
src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/query/account_balance_query.py (4)
CryptoGetAccountBalanceQuery(12-184)execute(135-158)set_contract_id(57-72)set_account_id(40-55)src/hiero_sdk_python/contract/contract_create_transaction.py (2)
ContractCreateTransaction(73-437)set_bytecode(133-149)src/hiero_sdk_python/response_code.py (1)
ResponseCode(4-387)
🪛 Ruff (0.14.10)
tests/integration/account_balance_query_e2e_test.py
16-16: Missing return type annotation for private function _create_test_contract
(ANN202)
28-30: Avoid specifying long messages outside the exception class
(TRY003)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.11)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.10)
- GitHub Check: run-examples
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (1)
tests/integration/account_balance_query_e2e_test.py (1)
48-59: LGTM: Contract balance query test validates end-to-end behavior.The test properly:
- Creates a contract via helper
- Queries balance using the new
set_contract_id()method- Validates balance existence and non-negativity
Good end-to-end coverage of the contract balance query feature.
Note: Contract cleanup issue is already flagged in the
_create_test_contracthelper review.
| def test_integration_account_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
|
|
||
| try: | ||
| CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) | ||
| balance = CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) | ||
| assert balance is not None | ||
| assert hasattr(balance, "hbars") | ||
| finally: | ||
| env.close() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strengthen assertions: validate balance values, not just attribute existence.
The test uses assert hasattr(balance, "hbars") which only checks that the attribute exists. Per coding guidelines (PRIORITY 4): tests should validate resulting state, not just success.
💪 Strengthen assertions
balance = CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client)
assert balance is not None
- assert hasattr(balance, "hbars")
+ assert balance.hbars is not None
+ assert balance.hbars.to_tinybars() >= 0This validates that:
- The balance object has a valid
hbarsvalue (not None) - The balance is non-negative (sanity check for network response)
Signed-off-by: Antonio Ceppellini <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 5
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
tests/integration/account_balance_query_e2e_test.py
🧰 Additional context used
📓 Path-based instructions (1)
tests/integration/**/*
⚙️ CodeRabbit configuration file
tests/integration/**/*: You are acting as a senior maintainer reviewing integration tests for the hiero-sdk-python project. Your goal is to ensure end-to-end tests validate real network behavior safely and deterministically.CRITICAL PRINCIPLES - Safety & Diagnosability:
- Prioritize safety: No implicit or default mainnet usage.
- Secrets and credentials must be injected safely (env vars, not hardcoded).
- Test failures must be diagnosable with clear error messages.
- Tests must assert observable network behavior, not just
SUCCESS.- Failure-path tests must assert specific
ResponseCodevalues (e.g.,TOKEN_HAS_NO_PAUSE_KEY).PRIORITY 1 - End-to-End Behavior:
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
- Validate resulting balances, ownership, and state changes (not just transaction success).
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
- Verify network state after operations (e.g., account balance changed, token transferred).
PRIORITY 2 - Test Structure & Maintainability:
- One major behavior per test (clear focus).
- Tests should be readable: clear names, brief docstrings, key inline comments.
- Minimal abstraction layers - prefer clarity over DRY.
- Is the file too monolithic? Flag if tests should be split into smaller modules.
- Are helper functions good candidates for pytest fixtures or shared utilities?
PRIORITY 3 - Isolation & Cleanup:
- Local account creation over operator reuse (avoid state pollution).
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
- Recommend teardown strategies or fixture scoping improvements.
- Tests should not depend on execution order (avoid brittle assumptions).
PRIORITY 4 - Assertions & Coverage:
- Do tests validate only success/failure, or also assert resulting state?
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
- Cover happy paths AND ...
Files:
tests/integration/account_balance_query_e2e_test.py
🧠 Learnings (1)
📚 Learning: 2026-01-08T10:28:14.391Z
Learnt from: github-actions[bot]
Repo: hiero-ledger/hiero-sdk-python PR: 0
File: :0-0
Timestamp: 2026-01-08T10:28:14.391Z
Learning: In hiero-ledger/hiero-sdk-python, examples should prefer Client.from_env() (src/hiero_sdk_python/client/client.py) for client setup. For examples under examples/tokens, replace manual Network/AccountId/PrivateKey + load_dotenv() bootstrapping with Client.from_env(), and use client.operator_account_id and client.operator_private_key instead of passing operator_id/operator_key through functions.
Applied to files:
tests/integration/account_balance_query_e2e_test.py
🧬 Code graph analysis (1)
tests/integration/account_balance_query_e2e_test.py (5)
src/hiero_sdk_python/account/account_id.py (1)
AccountId(21-198)src/hiero_sdk_python/contract/contract_id.py (1)
ContractId(26-224)src/hiero_sdk_python/query/account_balance_query.py (4)
CryptoGetAccountBalanceQuery(12-184)execute(135-158)set_contract_id(57-72)set_account_id(40-55)src/hiero_sdk_python/response_code.py (1)
ResponseCode(4-387)tests/integration/utils.py (2)
IntegrationTestEnv(34-96)env(23-27)
🪛 Ruff (0.14.10)
tests/integration/account_balance_query_e2e_test.py
16-16: Missing return type annotation for private function _create_test_contract
(ANN202)
28-30: Avoid specifying long messages outside the exception class
(TRY003)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: Codacy Static Code Analysis
- GitHub Check: run-examples
- GitHub Check: build-and-test (3.10)
- GitHub Check: build-and-test (3.13)
- GitHub Check: build-and-test (3.12)
- GitHub Check: build-and-test (3.11)
- GitHub Check: StepSecurity Harden-Runner
🔇 Additional comments (1)
tests/integration/account_balance_query_e2e_test.py (1)
1-13: LGTM!Imports are appropriate for the integration test functionality, and the module marker is correctly applied.
| import pytest | ||
|
|
||
| from hiero_sdk_python.account.account_id import AccountId | ||
| from hiero_sdk_python.contract.contract_id import ContractId | ||
| from hiero_sdk_python.query.account_balance_query import CryptoGetAccountBalanceQuery | ||
| from hiero_sdk_python.contract.contract_create_transaction import ContractCreateTransaction | ||
| from hiero_sdk_python.response_code import ResponseCode | ||
| from tests.integration.utils import IntegrationTestEnv | ||
|
|
||
| from examples.contract.contracts import SIMPLE_CONTRACT_BYTECODE | ||
|
|
||
|
|
||
| pytestmark = pytest.mark.integration | ||
|
|
||
|
|
||
| def _create_test_contract(env: IntegrationTestEnv): | ||
| bytecode = bytes.fromhex(SIMPLE_CONTRACT_BYTECODE) | ||
|
|
||
| receipt = ( | ||
| ContractCreateTransaction() | ||
| .set_bytecode(bytecode) | ||
| .set_gas(2_000_000) | ||
| .set_contract_memo("integration test: contract balance query") | ||
| .execute(env.client) | ||
| ) | ||
|
|
||
| if ResponseCode(receipt.status) != ResponseCode.SUCCESS: | ||
| raise RuntimeError( | ||
| f"ContractCreateTransaction failed with status: {ResponseCode(receipt.status).name}" | ||
| ) | ||
|
|
||
| if receipt.contract_id is None: | ||
| raise RuntimeError("ContractCreateTransaction succeeded but receipt.contract_id is None") | ||
|
|
||
| return receipt.contract_id | ||
|
|
||
|
|
||
| @pytest.mark.integration | ||
| def test_integration_account_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
|
|
||
| try: | ||
| CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) | ||
| balance = CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) | ||
| assert balance is not None | ||
| assert hasattr(balance, "hbars") | ||
| finally: | ||
| env.close() | ||
|
|
||
|
|
||
| def test_integration_contract_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| contract_id = _create_test_contract(env) | ||
|
|
||
| balance = CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(env.client) | ||
|
|
||
| assert balance is not None | ||
| assert hasattr(balance, "hbars") | ||
| assert balance.hbars.to_tinybars() >= 0 | ||
| finally: | ||
| env.close() | ||
|
|
||
|
|
||
| def test_integration_balance_query_raises_when_neither_source_set(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(ValueError, match=r"Either account_id or contract_id must be set before making the request\."): | ||
| CryptoGetAccountBalanceQuery().execute(env.client) | ||
| finally: | ||
| env.close() | ||
|
|
||
|
|
||
| def test_integration_balance_query_with_invalid_account_id_raises(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(TypeError, match=r"account_id must be an AccountId\."): | ||
| CryptoGetAccountBalanceQuery().set_account_id("0.0.12345").execute(env.client) | ||
| finally: | ||
| env.close() | ||
|
|
||
|
|
||
| def test_integration_balance_query_with_invalid_contract_id_raises(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(TypeError, match=r"contract_id must be a ContractId\."): | ||
| CryptoGetAccountBalanceQuery().set_contract_id("0.0.12345").execute(env.client) | ||
| finally: | ||
| env.close() |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Missing test case: verify error when both account_id and contract_id are set.
The AI summary mentions a test test_integration_balance_query_raises_when_both_sources_set that validates the error when both account_id and contract_id are provided, but this test is not present in the code. The SDK's oneof validation enforces "exactly one" source, but only the "neither set" case is tested.
➕ Add missing test case
Add the following test (ideally as a unit test per the previous comment):
def test_integration_balance_query_raises_when_both_sources_set():
"""Verify that setting both account_id and contract_id raises ValueError."""
env = IntegrationTestEnv()
try:
account_id = AccountId(0, 0, 12345)
contract_id = ContractId(0, 0, 67890)
with pytest.raises(ValueError, match=r"Specify either account_id or contract_id, not both"):
(CryptoGetAccountBalanceQuery()
.set_account_id(account_id)
.set_contract_id(contract_id)
.execute(env.client))
finally:
env.close()This ensures complete coverage of the oneof validation logic described in the PR objectives.
🧰 Tools
🪛 Ruff (0.14.10)
16-16: Missing return type annotation for private function _create_test_contract
(ANN202)
28-30: Avoid specifying long messages outside the exception class
(TRY003)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
| def _create_test_contract(env: IntegrationTestEnv): | ||
| bytecode = bytes.fromhex(SIMPLE_CONTRACT_BYTECODE) | ||
|
|
||
| receipt = ( | ||
| ContractCreateTransaction() | ||
| .set_bytecode(bytecode) | ||
| .set_gas(2_000_000) | ||
| .set_contract_memo("integration test: contract balance query") | ||
| .execute(env.client) | ||
| ) | ||
|
|
||
| if ResponseCode(receipt.status) != ResponseCode.SUCCESS: | ||
| raise RuntimeError( | ||
| f"ContractCreateTransaction failed with status: {ResponseCode(receipt.status).name}" | ||
| ) | ||
|
|
||
| if receipt.contract_id is None: | ||
| raise RuntimeError("ContractCreateTransaction succeeded but receipt.contract_id is None") | ||
|
|
||
| return receipt.contract_id |
There was a problem hiding this comment.
Contract cleanup missing - state leakage on network.
The helper creates a contract but never deletes it, causing persistent state pollution. Integration tests should clean up all created resources.
Additionally, the function is missing a return type annotation.
♻️ Proposed improvements
1. Add return type annotation:
-def _create_test_contract(env: IntegrationTestEnv):
+def _create_test_contract(env: IntegrationTestEnv) -> ContractId:2. Consider a cleanup strategy:
Option A: Return contract_id and document that callers should delete it in their finally block (if ContractDeleteTransaction is available).
Option B: Use a pytest fixture with proper teardown to manage contract lifecycle:
@pytest.fixture
def test_contract(integration_env):
contract_id = _create_test_contract(integration_env)
yield contract_id
# Cleanup: delete contract if SDK supports ContractDeleteTransactionAs per coding guidelines: Tests should clean up created resources to avoid state pollution and leakage between test runs.
🧰 Tools
🪛 Ruff (0.14.10)
16-16: Missing return type annotation for private function _create_test_contract
(ANN202)
28-30: Avoid specifying long messages outside the exception class
(TRY003)
33-33: Avoid specifying long messages outside the exception class
(TRY003)
| def test_integration_account_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
|
|
||
| try: | ||
| CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) | ||
| balance = CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client) | ||
| assert balance is not None | ||
| assert hasattr(balance, "hbars") | ||
| finally: | ||
| env.close() | ||
|
|
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Strengthen assertions to validate observable state.
The test only verifies that balance exists and has an hbars attribute but doesn't validate the actual balance value. Consider adding an assertion like balance.hbars.to_tinybars() >= 0 (as done in test_integration_contract_balance_query_can_execute at line 57) to verify observable network state.
♻️ Proposed improvement
def test_integration_account_balance_query_can_execute():
env = IntegrationTestEnv()
try:
balance = CryptoGetAccountBalanceQuery(account_id=env.operator_id).execute(env.client)
assert balance is not None
assert hasattr(balance, "hbars")
+ assert balance.hbars.to_tinybars() >= 0
finally:
env.close()As per coding guidelines: Integration tests should validate observable network behavior, not just object existence.
| def test_integration_contract_balance_query_can_execute(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| contract_id = _create_test_contract(env) | ||
|
|
||
| balance = CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(env.client) | ||
|
|
||
| assert balance is not None | ||
| assert hasattr(balance, "hbars") | ||
| assert balance.hbars.to_tinybars() >= 0 | ||
| finally: | ||
| env.close() | ||
|
|
There was a problem hiding this comment.
Contract cleanup missing in test.
The contract created by _create_test_contract is never deleted, causing state leakage. Consider adding cleanup in the finally block if ContractDeleteTransaction is available in the SDK.
Additionally, you could strengthen the test by verifying that a newly created contract has the expected balance (likely 0 or close to the creation cost).
♻️ Proposed improvement
If ContractDeleteTransaction is available:
def test_integration_contract_balance_query_can_execute():
env = IntegrationTestEnv()
+ contract_id = None
try:
contract_id = _create_test_contract(env)
balance = CryptoGetAccountBalanceQuery().set_contract_id(contract_id).execute(env.client)
assert balance is not None
assert hasattr(balance, "hbars")
assert balance.hbars.to_tinybars() >= 0
finally:
+ if contract_id is not None:
+ # Delete contract to avoid state leakage
+ # ContractDeleteTransaction().set_contract_id(contract_id).execute(env.client)
+ pass
env.close()As per coding guidelines: Integration tests should clean up created resources to prevent state pollution between test runs.
Committable suggestion skipped: line range outside the PR's diff.
| def test_integration_balance_query_raises_when_neither_source_set(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(ValueError, match=r"Either account_id or contract_id must be set before making the request\."): | ||
| CryptoGetAccountBalanceQuery().execute(env.client) | ||
| finally: | ||
| env.close() | ||
|
|
||
|
|
||
| def test_integration_balance_query_with_invalid_account_id_raises(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(TypeError, match=r"account_id must be an AccountId\."): | ||
| CryptoGetAccountBalanceQuery().set_account_id("0.0.12345").execute(env.client) | ||
| finally: | ||
| env.close() | ||
|
|
||
|
|
||
| def test_integration_balance_query_with_invalid_contract_id_raises(): | ||
| env = IntegrationTestEnv() | ||
| try: | ||
| with pytest.raises(TypeError, match=r"contract_id must be a ContractId\."): | ||
| CryptoGetAccountBalanceQuery().set_contract_id("0.0.12345").execute(env.client) | ||
| finally: | ||
| env.close() |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
These tests validate SDK logic, not network behavior.
Tests test_integration_balance_query_raises_when_neither_source_set, test_integration_balance_query_with_invalid_account_id_raises, and test_integration_balance_query_with_invalid_contract_id_raises validate SDK-level validation (TypeError/ValueError) before any network interaction occurs.
These are more appropriate as unit tests rather than integration tests. Creating IntegrationTestEnv (which sets up a network client) is unnecessary overhead since no network call is made.
♻️ Recommended approach
Option 1 (preferred): Move to unit tests
Create a separate unit test file (e.g., tests/unit/test_account_balance_query.py) without network setup:
import pytest
from hiero_sdk_python.query.account_balance_query import CryptoGetAccountBalanceQuery
from hiero_sdk_python.account.account_id import AccountId
from hiero_sdk_python.contract.contract_id import ContractId
def test_balance_query_raises_when_neither_source_set():
with pytest.raises(ValueError, match=r"Either account_id or contract_id must be set"):
query = CryptoGetAccountBalanceQuery()
query._make_request() # or whatever method triggers validation
def test_balance_query_with_invalid_account_id_raises():
with pytest.raises(TypeError, match=r"account_id must be an AccountId"):
CryptoGetAccountBalanceQuery().set_account_id("0.0.12345")
def test_balance_query_with_invalid_contract_id_raises():
with pytest.raises(TypeError, match=r"contract_id must be a ContractId"):
CryptoGetAccountBalanceQuery().set_contract_id("0.0.12345")Option 2: If keeping in integration tests, use a lighter fixture
@pytest.fixture
def mock_client():
"""Lightweight mock client for validation-only tests."""
return None # or minimal mock
def test_integration_balance_query_raises_when_neither_source_set(mock_client):
with pytest.raises(ValueError, match=r"Either account_id or contract_id must be set"):
CryptoGetAccountBalanceQuery().execute(mock_client)As per coding guidelines: Integration tests should validate real network behavior end-to-end. SDK validation logic belongs in unit tests.
4 participants
Description:
This PR adds
contract_idsupport forCryptoGetAccountBalanceQuery.Additons:
_make_request()-->
Related issue(s):
Fixes #1293
Checklist