-
Notifications
You must be signed in to change notification settings - Fork 108
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(e2e): tolerate multiple runs of precompiled tests #3256
Conversation
📝 WalkthroughWalkthroughThe pull request introduces enhancements to end-to-end tests in the Changes
Assessment against linked issues
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #3256 +/- ##
========================================
Coverage 61.74% 61.75%
========================================
Files 431 431
Lines 30772 30772
========================================
+ Hits 19001 19004 +3
+ Misses 10914 10911 -3
Partials 857 857 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
cmd/zetae2e/local/local.go (1)
381-391
: Enhance Test Independence by Eliminating Block Height DependencyRelying on
e2eStartHeight < 100
to conditionally execute tests introduces external dependencies on the system state. Consider refactoring the tests to be idempotent and independent of the block height, ensuring they can run reliably in any environment.e2e/e2etests/test_precompiles_bank.go (1)
63-69
: Refactor Balance Initialization into Helper FunctionThe code for initializing and capturing starting balances is similar across multiple tests. Consider abstracting this into a helper function to reduce duplication and improve maintainability.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
cmd/zetae2e/local/local.go
(3 hunks)e2e/e2etests/helpers.go
(1 hunks)e2e/e2etests/test_precompiles_bank.go
(5 hunks)e2e/e2etests/test_precompiles_bank_through_contract.go
(5 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
e2e/e2etests/helpers.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_precompiles_bank.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
e2e/e2etests/test_precompiles_bank_through_contract.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
cmd/zetae2e/local/local.go (1)
Pattern **/*.go
: Review the Go code, point out issues relative to principles of clean code, expressiveness, and performance.
🔇 Additional comments (18)
cmd/zetae2e/local/local.go (2)
221-223
: Block Height Retrieval Implemented Correctly
The retrieval of e2eStartHeight
ensures that the current block height is captured before test execution. This is correctly implemented.
288-290
: Consistent Minting of ERC20 Tokens Before Tests
Calling MintERC20OnEvm(1e10)
prior to each test ensures a consistent state for ERC20 token availability.
e2e/e2etests/helpers.go (1)
189-197
: Utility Functions Improve Code Readability
The addition of bigAdd
and bigSub
functions provides clear and concise operations for big integer arithmetic, enhancing code readability.
e2e/e2etests/test_precompiles_bank_through_contract.go (8)
Line range hint 50-54
: Proper Reset of Allowance and Balance in Test Cleanup
Resetting the allowance and balance ensures that tests can run multiple times without interference from previous states, improving test reliability.
63-69
: Initialization of Starting Balances Enhances Test Accuracy
Capturing the starting balances for the spender and bank addresses allows for precise balance assertions throughout the test.
76-78
: Balance Assertions Use Starting Balances Appropriately
Using the starting balances in assertions ensures accuracy and adaptability, especially if the initial balances change.
89-91
: Balances Remain Unchanged After Failed Deposit Attempt
Verifying that balances remain the same after a failed operation confirms that the contract handles error conditions correctly.
102-104
: Correct Handling of Deposit Amount Exceeding Balance
The test accurately checks that depositing an amount higher than the available balance fails as expected.
111-117
: Successful Deposit Updates Balances Appropriately
After a successful deposit, the balances are updated correctly, reflecting the transfer of tokens.
127-137
: Withdrawal Amount Validation Works as Intended
Attempting to withdraw more than the available balance fails, indicating proper validation in the withdrawal logic.
144-146
: Balances Restored After Successful Withdrawal
Upon successful withdrawal, the balances return to their initial state, demonstrating correct function behavior.
e2e/e2etests/test_precompiles_bank.go (7)
49-54
: Ensuring Zero Allowance Before Test Execution
Resetting the allowance to zero before the test begins prevents interference from previous test runs and ensures a clean testing environment.
111-118
: Accurate Balance Update Verification After Deposit
The assertions confirm that balances are updated correctly following a deposit, ensuring the contract's deposit functionality works as intended.
123-128
: Maintaining Token Lock State Post-Failed Withdrawal
After a failed withdrawal attempt, the bank's token balance remains unchanged, indicating proper handling of unsuccessful operations.
140-145
: Successful Withdrawal Reflects Correct Balance Adjustments
Balances are correctly adjusted following a successful withdrawal, validating the withdrawal logic.
160-168
: Spender's Cosmos Balance Reset to Initial State
The test confirms that the spender's cosmos balance returns to the initial state after withdrawal, ensuring consistency.
170-178
: Spender's ZRC20 Balance Restored Post-Withdrawal
The spender's ZRC20 balance matches the starting balance after withdrawal, indicating that tokens are returned properly.
180-188
: Bank's ZRC20 Balance Correct After Transactions
The bank's ZRC20 balance reflects the correct amount after deposit and withdrawal operations, confirming accurate balance management.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Closes #3201
Use relative balances when possible in the precompiled contract tests to enable running multiple times.
The staking contracts cannot be run multiple times so disable them if the block height is greater than 100 (indicating an unclean test environment).
I need to add a CI check that verifies e2e can be run multiple times to prevent future regressions.
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Documentation