test(evm): e2e tests configuration enhancements #2184
Conversation
|
Caution Review failedThe pull request is closed. 📝 WalkthroughWalkthroughThis pull request introduces configuration enhancements for end-to-end (e2e) tests in the Nibiru EVM repository. The changes primarily focus on adding configurable timeout constants ( Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (11)
✨ Finishing Touches
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
CodeRabbit Configuration 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: 1
🧹 Nitpick comments (6)
evm-e2e/.env_sample (1)
3-4: LGTM! Consider adding comments to document the timeout units.The timeout values look reasonable. Consider adding comments to clarify that these values are in milliseconds.
-TEST_TIMEOUT=15000 -TX_WAIT_TIMEOUT=5000 +# Test case timeout in milliseconds +TEST_TIMEOUT=15000 +# Transaction wait timeout in milliseconds +TX_WAIT_TIMEOUT=5000evm-e2e/test/setup.ts (1)
11-11: Fix the export statement formatting.There's a missing space before the closing curly brace.
-export { account, provider, TEST_TIMEOUT, TX_WAIT_TIMEOUT}; +export { account, provider, TEST_TIMEOUT, TX_WAIT_TIMEOUT };evm-e2e/test/contract_send_nibi.test.ts (1)
14-14: LGTM! Consider documenting the timeout relationship.Good use of
TX_WAIT_TIMEOUTfor transaction waits and a sensible approach to set test timeout asTX_WAIT_TIMEOUT * 2. Consider adding a comment explaining why the test timeout is set to double the transaction wait timeout.- const TIMEOUT_MS = TX_WAIT_TIMEOUT * 2; + // Double the transaction wait timeout to account for contract deployment and transaction execution + const TIMEOUT_MS = TX_WAIT_TIMEOUT * 2;Also applies to: 26-26, 55-55
evm-e2e/test/utils.ts (2)
15-17: Consider adding input validation to hexify function.The function should validate that the input is non-negative.
export const hexify = (x: number): string => { + if (x < 0) throw new Error('Input must be non-negative'); return '0x' + x.toString(16); };
57-65: Consider adding error handling to sendTestNibi.The function should handle potential transaction failures.
export const sendTestNibi = async () => { const transaction: TransactionRequest = { gasLimit: toBigInt(100e3), to: alice, value: parseEther('0.01'), }; - const txResponse = await account.sendTransaction(transaction); - await txResponse.wait(1, TX_WAIT_TIMEOUT); - console.log(txResponse); + try { + const txResponse = await account.sendTransaction(transaction); + await txResponse.wait(1, TX_WAIT_TIMEOUT); + console.log(txResponse); + return txResponse; + } catch (error) { + console.error('Transaction failed:', error); + throw error; + } - return txResponse; };evm-e2e/test/debug_queries.test.ts (1)
3-3: LGTM! Good improvement in test configurability.The changes effectively centralize timeout configuration by replacing hardcoded values with environment-configurable constants. This enhances maintainability and flexibility of the test suite.
Consider adding a comment explaining the purpose of these timeouts:
import {provider, TEST_TIMEOUT, TX_WAIT_TIMEOUT} from './setup'; +// TEST_TIMEOUT: Maximum time allowed for test execution +// TX_WAIT_TIMEOUT: Maximum time to wait for transaction confirmationAlso applies to: 21-21, 28-28
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (12)
CHANGELOG.md(1 hunks)evm-e2e/.env_sample(1 hunks)evm-e2e/test/contract_infinite_loop_gas.test.ts(2 hunks)evm-e2e/test/contract_send_nibi.test.ts(3 hunks)evm-e2e/test/debug_queries.test.ts(2 hunks)evm-e2e/test/erc20.test.ts(2 hunks)evm-e2e/test/eth_queries.test.ts(7 hunks)evm-e2e/test/native_transfer.test.ts(3 hunks)evm-e2e/test/nibiru_oracle.test.ts(2 hunks)evm-e2e/test/setup.ts(1 hunks)evm-e2e/test/tx_receipt.test.ts(2 hunks)evm-e2e/test/utils.ts(2 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
evm-e2e/test/utils.ts
[error] 81-81: Don't use 'Number' as a type.
Use lowercase primitives for consistency.
Safe fix: Use 'number' instead
(lint/complexity/noBannedTypes)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: e2e-evm
🔇 Additional comments (11)
evm-e2e/test/setup.ts (1)
8-9: LGTM! The implementation looks correct.Good use of Number() for safe type conversion and appropriate default values matching .env_sample.
evm-e2e/test/contract_infinite_loop_gas.test.ts (2)
10-10: LGTM! Good improvement in promise handling.The change to use
resolvesfor promise assertions is more correct and explicit.Also applies to: 20-20
21-21: LGTM! Appropriate timeout adjustment.Using
TEST_TIMEOUT * 2makes sense for this infinite loop test case, as it may require more time than standard tests.evm-e2e/test/erc20.test.ts (1)
3-3: LGTM! Clean implementation of the timeout configuration.The changes are consistent with the project-wide timeout standardization effort.
Also applies to: 24-24
evm-e2e/test/nibiru_oracle.test.ts (1)
4-4: LGTM! Standardized test timeout configuration.The addition of
TEST_TIMEOUTfrom the centralized setup improves test configuration management.Also applies to: 41-41
evm-e2e/test/native_transfer.test.ts (1)
3-3: LGTM! Consistent timeout configuration applied.Good use of both
TEST_TIMEOUTandTX_WAIT_TIMEOUTconstants for improved test configurability.Also applies to: 21-21, 44-44
evm-e2e/test/tx_receipt.test.ts (1)
3-4: LGTM! Good cleanup and standardization.The changes improve the code by:
- Using the standardized TEST_TIMEOUT constant
- Simplifying contract deployment by removing the unnecessary maxFeePerGas parameter
Also applies to: 8-8, 27-27
evm-e2e/test/eth_queries.test.ts (3)
2-3: LGTM! Good standardization of timeouts.The changes properly integrate the standardized timeout constants.
Also applies to: 14-14
57-57: Good improvement in block number handling.Replacing hardcoded block numbers with "latest" makes the tests more reliable and maintainable.
Also applies to: 69-69, 76-76
89-94: Good enhancement in filter setup.The changes improve filter reliability by:
- Retrieving the current block number before creating filters
- Using numberToHex for proper block number formatting
Also applies to: 118-123, 144-149
CHANGELOG.md (1)
93-93: LGTM! Proper changelog entry.The changelog entry is well-formatted and accurately describes the e2e tests configuration enhancements.
No description provided.