Add a test case to verify correctness of EVM.dryCall & COA.dryCall
#782
Conversation
m-Peter
requested review from
janezpodhostnik,
peterargue and
zhangchiqing
as code owners
WalkthroughThis pull request updates several dependency versions in the Go module files and adds new test cases. The dependency updates affect packages from the onflow ecosystem, ensuring that the latest versions are used. Additionally, new tests have been introduced in both Go and JavaScript that focus on the dry call functionality of EVM and COA, verifying that state changes do not occur during these calls. Changes
Possibly related PRs
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (1)
🔇 Additional comments (1)
✨ Finishing Touches
🪧 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: 0
🧹 Nitpick comments (2)
tests/web3js/evm_dry_call_test.js (1)
1-57: Well-structured test for verifying dry calls don't generate transactions.The test effectively verifies that
EVM.dryCallandCOA.dryCalloperations don't generate transactions or change state by checking the block content and transaction count.Consider making the test more maintainable by reducing hardcoded values:
- assert.equal(receipt.contractAddress, '0x99A64c993965f8d69F985b5171bC20065Cc32fAB') + const expectedContractAddress = '0x99A64c993965f8d69F985b5171bC20065Cc32fAB' + assert.equal(receipt.contractAddress, expectedContractAddress)This approach would make it easier to update if contract addresses change, especially when used multiple times in the test.
tests/e2e_web3js_test.go (1)
238-345: Comprehensive test case for verifying dry call functionality.This test effectively verifies that both
EVM.dryCallandCOA.dryCalldon't modify state by checking the returned value remains unchanged after attempting to update it.Consider refactoring the duplicate code patterns for testing
EVM.dryCallandCOA.dryCall. Both sections have similar verification logic that could be extracted:+ func verifyDryCallDoesNotChangeState(t *testing.T, emu emulator.Emulator, script string, expectedValue int) { + res, err := flowSendTransaction(emu, script) + require.NoError(t, err) + require.NoError(t, res.Error) + } // Then call this helper function with appropriate scripts for EVM.dryCall and COA.dryCallThis would reduce duplication and make the test more maintainable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (2)
go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sum
📒 Files selected for processing (4)
go.mod(2 hunks)tests/e2e_web3js_test.go(1 hunks)tests/go.mod(3 hunks)tests/web3js/evm_dry_call_test.js(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: Lint
🔇 Additional comments (4)
tests/go.mod (1)
7-7: Dependency updates look good.The updated versions of key dependencies are necessary to support the new
EVM.dryCallandCOA.dryCallfunctionality being tested in this PR. The updated packages include Cadence, Flow-Go, Flow-Go-SDK, and various Flow core contracts.Also applies to: 11-12, 154-155, 160-160, 254-254
go.mod (1)
10-12: Dependency updates align with the testing requirements.The updates to Cadence, Flow-Go, Flow-Go-SDK and other dependencies are consistent with the changes in tests/go.mod and support the new functionality being tested.
Also applies to: 144-145, 150-150
tests/e2e_web3js_test.go (2)
294-314: Good assertion of state persistence after EVM.dryCall.The test clearly verifies that
EVM.dryCallto modify the contract state withstoreWithLog(1453)doesn't actually change the state, as the retrieved value remains 42.
316-336: Good verification of COA.dryCall behavior.The test effectively demonstrates that
COA.dryCallwithstoreWithLog(1515)doesn't change the contract state, maintaining the value at 42. Also correctly handles the resource cleanup withdestroy.
m-Peter
force-pushed
the
mpeter/test-evm-dry-call
branch
from
522a8da to
3af4f6a
Compare
m-Peter
force-pushed
the
mpeter/test-evm-dry-call
branch
from
3af4f6a to
053f977
Compare
Depends on: onflow/flow-emulator#803
Description
Updates the
flow-goto include the newly-addedEVM.dryCall&COA.dryCallfunctions, and adds an E2E test to verify they work as expected. That is, without creating any unnecessary EVM transactions and without committing any state changes.For contributor use:
masterbranchFiles changedin the Github PR explorerSummary by CodeRabbit
EVM.dryCallandCOA.dryCallfunctionalities, expanding coverage for contract interactions.EVM.dryCallandCOA.dryCallfunctions.EVM.dryCall & COA.dryCallto ensure correct contract interactions.