Improve tests for batched transactions #239
Conversation
|
Warning Rate limit exceeded@m-Peter has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 34 minutes and 41 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates enhance the testing framework for Ethereum transactions by incorporating batch processing capabilities. The primary changes involve creating, encoding, and sending multiple transactions in batches within the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant T as Test Function
participant E as Ethereum Library
participant B as Batch Processor
participant A as Assertions
T->>E: Create multiple transactions
E->>B: Encode transactions
B->>T: Return encoded transactions
T->>B: Send batch transactions
B->>T: Confirm transactions sent
T->>A: Perform assertions on batch transactions
A->>T: Return results
Possibly related issues
Poem
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- tests/e2e_web3js_test.go (2 hunks)
- tests/helpers.go (1 hunks)
- tests/web3js/eth_batch_retrieval_test.js (1 hunks)
Files skipped from review due to trivial changes (1)
- tests/web3js/eth_batch_retrieval_test.js
Additional comments not posted (7)
tests/e2e_web3js_test.go (6)
4-4: Import ofencoding/hexis appropriate for handling hexadecimal encoding required for transaction data.
7-7: Import ofgithub.aaakk.us.kg/onflow/go-ethereum/commonis necessary for Ethereum common utilities, which are essential for address handling in the tests.
8-8: Import ofgithub.aaakk.us.kg/onflow/go-ethereum/cryptois crucial for cryptographic functions like signing transactions, which is used extensively in the test setup.
9-9: Import ofgithub.aaakk.us.kg/stretchr/testify/requireis used for assertions within the test functions, ensuring that the tests fail fast on errors.
10-10: Import ofmath/bigis used for handling large integers in Ethereum transactions, which is necessary for the test scenarios.
45-86: The modifications inTestWeb3_E2Eto handle batch transactions are well-implemented. The setup for creating multiple transactions and the subsequent assertions are correctly structured to ensure that each transaction in the batch is processed successfully.tests/helpers.go (1)
111-114: The modification inrunWeb3TestWithSetupto remove the error return from the setup function parameter simplifies the test setup process. This change likely reduces boilerplate error handling and makes the function easier to use in test scenarios.
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (1)
- tests/web3js/eth_batch_retrieval_test.js (1 hunks)
Additional context used
Biome
tests/web3js/eth_batch_retrieval_test.js
[error] 9-9: This let declares a variable that is only assigned once.
[error] 10-10: This let declares a variable that is only assigned once.
[error] 12-12: This let declares a variable that is only assigned once.
[error] 16-16: This let declares a variable that is only assigned once.
Additional comments not posted (1)
tests/web3js/eth_batch_retrieval_test.js (1)
6-23: The logic and assertions in the test function appear correct and effectively test the batch transaction retrieval.
m-Peter
force-pushed
the
gregor/batched-transactions-testing
branch
from
1b2318e to
608059e
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (3)
- tests/e2e_web3js_test.go (2 hunks)
- tests/helpers.go (1 hunks)
- tests/web3js/eth_batch_retrieval_test.js (1 hunks)
Additional context used
Biome
tests/web3js/eth_batch_retrieval_test.js
[error] 8-8: This let declares a variable that is only assigned once.
[error] 9-9: This let declares a variable that is only assigned once.
[error] 11-11: This let declares a variable that is only assigned once.
[error] 15-15: This let declares a variable that is only assigned once.
Additional comments not posted (2)
tests/e2e_web3js_test.go (1)
50-91: The refactoring ofrunWeb3TestWithSetupto handle batch transactions is well done. The use ofrequire.NoErrorensures that any errors are caught immediately, which is crucial for transaction handling.tests/helpers.go (1)
112-115: The simplification of thesetupFuncparameter inrunWeb3TestWithSetupis a good improvement. Removing the error return type simplifies the function signature and error handling.
| let latestHeight = await web3.eth.getBlockNumber() | ||
| let block = await web3.eth.getBlock(latestHeight) | ||
| assert.lengthOf(block.transactions, 2) | ||
| let latestHeight = await web3.eth.getBlockNumber() |
There was a problem hiding this comment.
Consider using const for latestHeight as it is assigned only once.
- let latestHeight = await web3.eth.getBlockNumber()
+ const latestHeight = await web3.eth.getBlockNumber()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.
| let latestHeight = await web3.eth.getBlockNumber() | |
| const latestHeight = await web3.eth.getBlockNumber() |
| assert.equal(block.hash, deployTx.blockHash) | ||
| assert.equal(0, deployTx.type) | ||
| assert.equal(0, deployTx.transactionIndex) | ||
| let batchSize = 10 |
There was a problem hiding this comment.
Consider using const for batchSize as it is assigned only once.
- let batchSize = 10
+ const batchSize = 10Committable 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.
| let batchSize = 10 | |
| const batchSize = 10 |
| let block = await web3.eth.getBlock(latestHeight) | ||
| assert.lengthOf(block.transactions, 2) | ||
| let latestHeight = await web3.eth.getBlockNumber() | ||
| let block = await web3.eth.getBlock(latestHeight) |
There was a problem hiding this comment.
Consider using const for block as it is assigned only once.
- let block = await web3.eth.getBlock(latestHeight)
+ const block = await web3.eth.getBlock(latestHeight)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.
| let block = await web3.eth.getBlock(latestHeight) | |
| const block = await web3.eth.getBlock(latestHeight) |
| assert.equal(0, callTx.type) | ||
| assert.equal(1, callTx.transactionIndex) | ||
| for (let i = 0; i < block.transactions.length; i++) { | ||
| let tx = await web3.eth.getTransactionFromBlock(latestHeight, i) |
There was a problem hiding this comment.
Consider using const for tx as it is assigned only once.
- let tx = await web3.eth.getTransactionFromBlock(latestHeight, i)
+ const tx = await web3.eth.getTransactionFromBlock(latestHeight, i)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.
| let tx = await web3.eth.getTransactionFromBlock(latestHeight, i) | |
| const tx = await web3.eth.getTransactionFromBlock(latestHeight, i) |
Description
Improve testing of batched transactions.
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
New Features
Tests
These changes improve the robustness and efficiency of our testing processes, ensuring better performance and reliability.