Skip to content

Commit 97c7b5f

Browse files
MonaaEidMonaaEid
authored
chore: enhance unit and integration test review instructions (#1231)
Signed-off-by: MonaaEid <[email protected]>
1 parent bf6ee2d commit 97c7b5f

File tree

2 files changed

+107
-2
lines changed

2 files changed

+107
-2
lines changed

‎.coderabbit.yaml‎

Lines changed: 106 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,8 +19,8 @@ reviews:
1919
poem: false # Do not write a literal poem (spammy)
2020
enable_prompt_for_ai_agents: false # Disable prompts for AI agents (spammy)
2121

22-
# --- CUSTOM INSTRUCTIONS FOR EXAMPLES DIRECTORY ---
2322
path_instructions:
23+
# --- CUSTOM INSTRUCTIONS FOR EXAMPLES DIRECTORY ---
2424
- path: "examples/**/*"
2525
instructions: |
2626
You are acting as a senior maintainer reviewing SDK examples. Your goal is to ensure examples work verbatim for users who copy-paste them.
@@ -63,6 +63,111 @@ reviews:
6363
- Be concise, technical, and opinionated.
6464
- Flag out-of-scope improvements as potential new issues rather than blocking.
6565
66+
# --- UNIT TESTS REVIEW INSTRUCTIONS ---
67+
- path: "tests/unit/**/*"
68+
instructions: |
69+
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.
70+
71+
**CRITICAL PRINCIPLES - Tests Must Fail Loudly & Deterministically**:
72+
- Tests must provide useful error messages when they fail for future debugging.
73+
- No `print()` statements - use assertions with descriptive messages.
74+
- No timing-dependent or unseeded random assertions.
75+
- No network calls or external dependencies (unit tests are isolated).
76+
- No unjustified TODOs or skipped tests without tracking issues.
77+
78+
**PRIORITY 1 - Protect Against Breaking Changes**:
79+
- Assert public attributes exist (e.g., `assert hasattr(obj, 'account_id')`).
80+
- Assert return types where relevant (e.g., `assert isinstance(result, AccountId)`).
81+
- Assert fluent setters return `self` (e.g., `assert tx.set_memo("test") is tx`).
82+
- Assert backward-compatible defaults are maintained.
83+
- If a breaking change is introduced, tests must assert deprecation behavior and test old behavior until removal.
84+
85+
**PRIORITY 2 - Constructor & Setter Behavior**:
86+
- Test constructor behavior with valid inputs, edge cases, and invalid inputs.
87+
- Test setter behavior including method chaining (fluent interface).
88+
- Verify that setters validate input and raise appropriate exceptions.
89+
- Test that getters return expected values after construction/setting.
90+
91+
**PRIORITY 3 - Comprehensive Coverage**:
92+
- Unit tests should be extensive - test even if we don't expect users to use it currently.
93+
- Cover happy paths AND unhappy paths/edge cases.
94+
- Test boundary conditions, null/None values, empty collections, etc.
95+
- Avoid brittle ordering assertions unless order is part of the contract.
96+
97+
**PRIORITY 4 - No Mocks for Non-Existent Modules**:
98+
- All imports must reference actual SDK modules - no hallucinated paths.
99+
- Validate import paths against the actual `src/hiero_sdk_python` structure.
100+
- Mocks should only be used for external dependencies, not SDK internals.
101+
102+
**PRIORITY 5 - Test Framework Philosophy**:
103+
- Prefer repetitive but clear tests over abstracted helper functions.
104+
- Some core functionality may warrant helper files (considered an exception).
105+
- Discourage custom helper functions; prefer pytest fixtures when shared setup is needed.
106+
- Prefer testing real functionality over mocked behavior.
107+
108+
**AVOID**:
109+
- Linter or formatting feedback (leave that to ruff/pre-commit).
110+
- Nitpicking minor stylistic issues unless they impact maintainability.
111+
- Overly abstracted test helpers that obscure what's being tested.
112+
113+
**PHILOSOPHY**:
114+
- Unit tests protect our future selves - be defensive and forward-looking.
115+
- Tests should be readable by SDK developers: clear names, brief docstrings, key inline comments.
116+
- When tests fail, we should immediately know what broke and why.
117+
118+
# --- INTEGRATION TESTS REVIEW INSTRUCTIONS ---
119+
- path: "tests/integration/**/*"
120+
instructions: |
121+
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.
122+
123+
**CRITICAL PRINCIPLES - Safety & Diagnosability**:
124+
- **Prioritize safety**: No implicit or default mainnet usage.
125+
- Secrets and credentials must be injected safely (env vars, not hardcoded).
126+
- Test failures must be diagnosable with clear error messages.
127+
- Tests must assert observable network behavior, not just `SUCCESS`.
128+
- Failure-path tests must assert specific `ResponseCode` values (e.g., `TOKEN_HAS_NO_PAUSE_KEY`).
129+
130+
**PRIORITY 1 - End-to-End Behavior**:
131+
- Tests should be end-to-end: construct → freeze → sign → execute → verify.
132+
- Validate resulting balances, ownership, and state changes (not just transaction success).
133+
- Assert transaction receipts contain expected data (IDs, serial numbers, etc.).
134+
- Verify network state after operations (e.g., account balance changed, token transferred).
135+
136+
**PRIORITY 2 - Test Structure & Maintainability**:
137+
- One major behavior per test (clear focus).
138+
- Tests should be readable: clear names, brief docstrings, key inline comments.
139+
- Minimal abstraction layers - prefer clarity over DRY.
140+
- Is the file too monolithic? Flag if tests should be split into smaller modules.
141+
- Are helper functions good candidates for pytest fixtures or shared utilities?
142+
143+
**PRIORITY 3 - Isolation & Cleanup**:
144+
- Local account creation over operator reuse (avoid state pollution).
145+
- Are accounts, tokens, and allowances properly cleaned up to avoid state leakage?
146+
- Recommend teardown strategies or fixture scoping improvements.
147+
- Tests should not depend on execution order (avoid brittle assumptions).
148+
149+
**PRIORITY 4 - Assertions & Coverage**:
150+
- Do tests validate only success/failure, or also assert resulting state?
151+
- Suggest additional assertions that would strengthen correctness (balances, allowances, ownership).
152+
- Cover happy paths AND unhappy paths (e.g., invalid spender, revoked allowance, insufficient balance).
153+
- Avoid timing-based or flaky assumptions.
154+
155+
**PRIORITY 5 - Observability & Debugging**:
156+
- Could structured logging or transaction metadata improve debugging?
157+
- Suggest capturing allowance values, transaction IDs, and balances in logs.
158+
- Ensure error messages provide context for failure diagnosis.
159+
160+
**AVOID**:
161+
- Linter or formatting feedback.
162+
- Overly abstracted helpers that obscure what's being tested.
163+
- Timing-dependent assertions (use explicit waits or retries if needed).
164+
165+
**PHILOSOPHY**:
166+
- Integration tests validate real network behavior - they must be reliable and safe.
167+
- Tests should protect against regressions while being maintainable.
168+
- When tests fail in CI, developers should immediately understand what broke.
169+
- Redundant setup code should be refactored, but clarity trumps abstraction.
170+
66171
chat:
67172
art: false # Don't draw ASCII art (false)
68173
auto_reply: false # Don't allow bot to converse (spammy)

‎CHANGELOG.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ This changelog is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.
9999
- Transformed `examples\tokens\custom_royalty_fee.py` to be an end-to-end example, that interacts with the Hedera network, rather than a static object demo.
100100
- Refactored `examples/tokens/custom_royalty_fee.py` by splitting monolithic function custom_royalty_fee_example() into modular functions create_royalty_fee_object(), create_token_with_fee(), verify_token_fee(), and main() to improve readability, cleaned up setup_client() (#1169)
101101
- Added comprehensive unit tests for Timestamp class (#1158)
102-
102+
- Enhance unit and integration test review instructions for clarity and coverage `.coderabbit.yaml`.
103103

104104

105105
### Fixed

0 commit comments

Comments
 (0)