Improve tests for batched transactions #239
Conversation
WalkthroughThe recent updates focus on enhancing batch processing capabilities in Ethereum-related tests. Changes include the introduction of new imports, modifications to transaction handling to support batch operations, and simplification of test setup functions. This streamlines the testing process and expands the test coverage for batch transactions in a blockchain context. Changes
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.com/onflow/go-ethereum/commonis necessary for Ethereum common utilities, which are essential for address handling in the tests.
8-8: Import ofgithub.com/onflow/go-ethereum/cryptois crucial for cryptographic functions like signing transactions, which is used extensively in the test setup.
9-9: Import ofgithub.com/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.
| 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, block.number) |
There was a problem hiding this comment.
| let tx = await web3.eth.getTransactionFromBlock(latestHeight, block.number) | |
| let tx = await web3.eth.getTransactionFromBlock(latestHeight, i) |
| assert.equal(tx.blockNumber, block.number, "wrong block number") | ||
| assert.equal(tx.blockHash, block.hash, "wrong block hash") | ||
| assert.equal(tx.type, 0, "wrong type") | ||
| //assert.equal(tx.transactionIndex, i, "wrong index") |
There was a problem hiding this comment.
| //assert.equal(tx.transactionIndex, i, "wrong index") | |
| assert.equal(tx.transactionIndex, i, "wrong index") |
| for (let i = 0; i < block.transactions.length; i++) { | ||
| let tx = await web3.eth.getTransactionFromBlock(latestHeight, block.number) | ||
| console.log(tx) | ||
| console.log("-----") |
There was a problem hiding this comment.
I believe we can remove these 2 console.log statements.
| @@ -37,40 +42,48 @@ func TestWeb3_E2E(t *testing.T) { | |||
| }) | |||
|
|
|||
| t.Run("batch run transactions", func(t *testing.T) { | |||
| runWeb3TestWithSetup(t, "eth_batch_retrieval_test", func(emu emulator.Emulator) error { | |||
| tx1, err := cadence.NewString("f9015880808301e8488080b901086060604052341561000f57600080fd5b60eb8061001d6000396000f300606060405260043610603f576000357c0100000000000000000000000000000000000000000000000000000000900463ffffffff168063c6888fa1146044575b600080fd5b3415604e57600080fd5b606260048080359060200190919050506078565b6040518082815260200191505060405180910390f35b60007f24abdb5865df5079dcc5ac590ff6f01d5c16edbc5fab4e195d9febd1114503da600783026040518082815260200191505060405180910390a16007820290509190505600a165627a7a7230582040383f19d9f65246752244189b02f56e8d0980ed44e7a56c0b200458caad20bb002982052fa09c05a7389284dc02b356ec7dee8a023c5efd3a9d844fa3c481882684b0640866a057e96d0a71a857ed509bb2b7333e78b2408574b8cc7f51238f25c58812662653") | |||
There was a problem hiding this comment.
I know that it's impossible to understand what these 2 transactions did 😅
I believe it was a good test case scenario though, it was basically a contract deployment, followed by a contract function call. But I suppose that such logic must already have been tested against in the flow-go repository.
There was a problem hiding this comment.
I plan to add more complex transactions as well, but I prefer actually building them not just passing payload, that way is clear what they do and it's easier to change.
Description
Improve testing of batched transactions.
For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit