Integraton tests for dynamic price changes #406
Conversation
- changed max bridge amount on safe - changed aggregation fee
…ce-changes # Conflicts: # executors/multiversx/module/scCallsModule.go # integrationTests/relayers/slowTests/edgeCases_test.go
# Conflicts: # executors/multiversx/module/scCallsModule.go # integrationTests/relayers/slowTests/framework/testSetup.go
| @@ -35,22 +34,22 @@ type scCallsModule struct { | |||
| } | |||
|
|
|||
| // NewScCallsModule creates a starts a new scCallsModule instance | |||
| func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger) (*scCallsModule, error) { | |||
| func NewScCallsModule(cfg config.ScCallsModuleConfig, proxy multiversx.Proxy, log logger.Logger) (*scCallsModule, error) { | |||
There was a problem hiding this comment.
extract parameters into a Arg structure
There was a problem hiding this comment.
also missing check for nil log instance
There was a problem hiding this comment.
I guess is my fault by not checking the log instance for nil :(
| @@ -236,4 +218,4 @@ func (module *scCallsModule) Close() error { | |||
| } | |||
|
|
|||
| return lastError | |||
| } | |||
| } | |||
| func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger) (*scCallsModule, error) { | ||
| func NewScCallsModule(cfg config.ScCallsModuleConfig, proxy multiversx.Proxy, log logger.Logger) (*scCallsModule, error) { | ||
| if check.IfNil(proxy) { | ||
| return nil, errNilProxy //TODO: add unit test for this |
| assert.NotNil(t, err) | ||
| assert.Contains(t, err.Error(), "nil proxy") |
There was a problem hiding this comment.
maybe consider exporting the error and check here against error?
There was a problem hiding this comment.
The check should be made on the existing errNilProxy error. The error, however, does not necessarily need to be exported since we are using the same package for production code & test code
|
|
||
| module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) | ||
| module, err := NewScCallsModule(cfg, proxy, &testsCommon.LoggerStub{}) |
There was a problem hiding this comment.
use a mocked component instead of a real proxy
There was a problem hiding this comment.
agree, we should use a stub proxy instance for these tests
| assert.NotNil(t, err) | ||
| assert.Contains(t, err.Error(), "invalid caching duration, provided: 0s, minimum: 1s") | ||
| assert.Nil(t, module) | ||
| assert.Nil(t, proxy) |
There was a problem hiding this comment.
all proxy tests should be removed now from this component
| logger "github.com/multiversx/mx-chain-logger-go" | ||
| "github.com/stretchr/testify/require" | ||
| ) | ||
|
|
||
| const ScCallsDeltaLimit = 10 |
There was a problem hiding this comment.
I think this can be unexported
| }) | ||
| } | ||
|
|
||
| func testRelayersWithChainSimulatorAndTokensForChangedAggregationFee(tb testing.TB, manualStopChan chan error, tokens ...framework.TestTokenParams) *framework.TestSetup { |
There was a problem hiding this comment.
this is very similar to testRelayersWithChainSimulatorAndTokensForChangedMaxBridgeAmount.. can we remove the duplicated code?
There was a problem hiding this comment.
maybe provide the handler as a parameter? The one given to the RegisterBeforeTransactionSendHandler function
| @@ -766,7 +766,7 @@ func (handler *MultiversxHandler) setPairDecimalsOnAggregator(ctx context.Contex | |||
| log.Info("setPairDecimals tx executed", "hash", hash, "status", txResult.Status) | |||
| } | |||
|
|
|||
| func (handler *MultiversxHandler) setMaxBridgeAmountOnSafe(ctx context.Context, params IssueTokenParams) { | |||
| func (handler *MultiversxHandler) SetMaxBridgeAmountOnSafe(ctx context.Context, params IssueTokenParams, maxBridgedAmountForToken string) { | |||
There was a problem hiding this comment.
is this needed as exported? if so, missing comment
| @@ -35,22 +34,22 @@ type scCallsModule struct { | |||
| } | |||
|
|
|||
| // NewScCallsModule creates a starts a new scCallsModule instance | |||
| func NewScCallsModule(cfg config.ScCallsModuleConfig, log logger.Logger) (*scCallsModule, error) { | |||
| func NewScCallsModule(cfg config.ScCallsModuleConfig, proxy multiversx.Proxy, log logger.Logger) (*scCallsModule, error) { | |||
There was a problem hiding this comment.
I guess is my fault by not checking the log instance for nil :(
| @@ -1,7 +1,10 @@ | |||
| package module | |||
|
|
|||
| import ( | |||
| "github.com/multiversx/mx-sdk-go/blockchain" | |||
| assert.NotNil(t, err) | ||
| assert.Contains(t, err.Error(), "nil proxy") |
There was a problem hiding this comment.
The check should be made on the existing errNilProxy error. The error, however, does not necessarily need to be exported since we are using the same package for production code & test code
|
|
||
| module, err := NewScCallsModule(cfg, &testsCommon.LoggerStub{}) | ||
| module, err := NewScCallsModule(cfg, proxy, &testsCommon.LoggerStub{}) |
There was a problem hiding this comment.
agree, we should use a stub proxy instance for these tests
| }) | ||
| } | ||
|
|
||
| func testRelayersWithChainSimulatorAndTokensForChangedAggregationFee(tb testing.TB, manualStopChan chan error, tokens ...framework.TestTokenParams) *framework.TestSetup { |
There was a problem hiding this comment.
maybe provide the handler as a parameter? The one given to the RegisterBeforeTransactionSendHandler function
| if firstProcessRun { | ||
| firstProcessRun = false | ||
|
|
||
| //TODO: remove this example |
There was a problem hiding this comment.
remove TODO & this example?
No description provided.