ccip - new integration tests #15473
ccip - new integration tests #15473
Conversation
AER Report: CI Coreaer_workflow , commit , Scheduled Run Frequency , Clean Go Tidy & Generate , Detect Changes , Flakeguard Root Project / Get Tests To Run , lint , Core Tests (go_core_tests) , Core Tests (go_core_tests_integration) , Core Tests (go_core_ccip_deployment_tests) , Core Tests (go_core_race_tests) , Core Tests (go_core_fuzz) , Flakeguard Root Project / Run Tests , Flakeguard Deployment Project / Get Tests To Run , Flakeguard Root Project / Report , Flakeguard Deployment Project / Run Tests , Flakeguard Deployment Project / Report , Flakey Test Detection , SonarQube Scan 1. Missing module error: buildSource of Error:Why: The error occurred because the 'requests' module is not installed in the environment where the build job is running. Suggested fix: Add 'requests' to the dependencies list in the requirements file or install it directly in the build job script. 2. Syntax error in script: testSource of Error:Why: There is a syntax error in one of the scripts being executed during the test step. This could be due to a typo or incorrect code structure. Suggested fix: Review the script for syntax errors and correct any typos or structural issues in the code. 3. Permission denied: deploySource of Error:Why: The error occurred because the script does not have the necessary permissions to access or modify the specified file or directory. Suggested fix: Ensure that the script has the appropriate permissions to access the file or directory by modifying the permissions or running the script with elevated privileges. AER Report: Operator UI CI ran successfully ✅ |
dimkouv
requested review from
winder,
rstout,
asoliman92,
makramkd,
mateusz-sekara,
0xAustinWang and
0xnogo
| E2E_TEST_SELECTED_NETWORK: SIMULATED_1,SIMULATED_2,SIMULATED_3 | ||
| E2E_JD_VERSION: 0.6.0 | ||
|
|
||
| - id: smoke/ccip/ccip_token_price_updates_test.go:* |
There was a problem hiding this comment.
No need to define 3 chains if you need only 2 of them. Let's keep only as many chains as we need for tests, that will make them faster
There was a problem hiding this comment.
I guess same applied to other tests
| feeQuoter2 := state.Chains[sourceChain2].FeeQuoter | ||
|
|
||
| // get initial chain fees | ||
| chainFee2, err := feeQuoter1.GetDestinationChainGasPrice(&bind.CallOpts{Context: ctx}, sourceChain2) |
There was a problem hiding this comment.
nit, but it seems we use &bind.CallOpts{Context: ctx} multiple times, might be worth extracting to some variable or func
| return false | ||
| } | ||
|
|
||
| chainFee2Now, err := feeQuoter1.GetDestinationChainGasPrice(&bind.CallOpts{Context: ctx}, sourceChain2) |
There was a problem hiding this comment.
Just an idea, but maybe it would be easier to understand the code if we name these as sourceChain and destChain. I think it would be easier to read it than chain1 and chain2
There was a problem hiding this comment.
it's not really true though, because lanes are bidirectional so both are source/dest
| // get initial chain fees | ||
| chainFee2, err := feeQuoter1.GetDestinationChainGasPrice(&bind.CallOpts{Context: ctx}, sourceChain2) | ||
| require.NoError(t, err) | ||
| chainFee1, err := feeQuoter2.GetDestinationChainGasPrice(&bind.CallOpts{Context: ctx}, sourceChain1) |
There was a problem hiding this comment.
maybe something like initialChainFee and updatedChainFee after the price updates?
| assert.GreaterOrEqual(t, len(allChainSelectors), 2, "test requires at least 2 chains") | ||
|
|
||
| sourceChain1 := allChainSelectors[0] | ||
| //sourceChain2 := allChainSelectors[1] |
There was a problem hiding this comment.
Remove if not needed
| }, tests.WaitTimeout(t), 500*time.Millisecond) | ||
|
|
||
| // disable oracles to prevent price updates while we manually edit token prices | ||
| var disabledNodes []string |
There was a problem hiding this comment.
disabling oracles could be extracted to a separate function, it would make the test easier to read
| }, tests.WaitTimeout(t), 200*time.Millisecond) | ||
|
|
||
| // re-enable the oracles | ||
| for _, n := range disabledNodes { |
There was a problem hiding this comment.
Similarly, probably you could extract disabling/enabling to separate functions
| ExtraArgs: changeset.MakeEVMExtraArgsV2(uint64(chain0DestConfig.MaxPerMsgGasLimit), true), | ||
| }, | ||
| }, | ||
| //{ // TODO: exec plugin never executed this message. |
There was a problem hiding this comment.
Please link the ticket in the comment
Add integration tests covering: