Use module accounts in MsgSend and MsgMultiSend tests in bank module #9075
Conversation
|
@cyberbono3 Is there any progress on this PR? Do you need any help? |
Codecov Report
@@ Coverage Diff @@
## master #9075 +/- ##
==========================================
+ Coverage 60.33% 60.37% +0.03%
==========================================
Files 585 585
Lines 36789 36851 +62
==========================================
+ Hits 22195 22247 +52
- Misses 12624 12630 +6
- Partials 1970 1974 +4
|
No, thanks. We had a pairing session with @anilcse , so I move forward. |
x/bank/simulation/operations_test.go
Outdated
| var addr sdk.AccAddress | ||
|
|
||
| switch { | ||
| case r.Int()%2 == 0: |
There was a problem hiding this comment.
Added for loop here for simplicity and pick ModuleName in GetModuleAddress at random, depending on moduleAccCount. I am not quite sure if it makes sense.
|
@anilcse Following our today discussion I tried to refactor SimulateMsgSend to handle both cases(recipient: ModuleAccount, Account). Unfortunately, my findings showed this would cause a lot of modifications in other modules. I have to add additional bool argument in Operation and apply these modifications in other modules which would take a lot of time. From my perspective, it does not make a lot of sense. What do u think? |
|
@anilcse so should I introduce |
x/bank/simulation/operations.go
Outdated
| accs []simtypes.Account, chainID string, | ||
| ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { | ||
| simAccount := accs[0] | ||
| toSimAcc := accs[1] |
There was a problem hiding this comment.
Who and where defines accs?
There was a problem hiding this comment.
setupAccounts defines accs.
cyberbono3
requested review from
aaronc,
alessio and
alexanderbez
as code owners
|
@robert-zaremba Would you have time to give another review to this PR? cc @anilcse @aleem1314 too, if you have bandwidth |
Co-authored-by: Robert Zaremba <[email protected]>
Co-authored-by: Robert Zaremba <[email protected]>
| return func( | ||
| r *rand.Rand, app *baseapp.BaseApp, ctx sdk.Context, | ||
| accs []simtypes.Account, chainID string, | ||
| ) (simtypes.OperationMsg, []simtypes.FutureOperation, error) { | ||
| from := accs[0] | ||
| to := accs[1] | ||
|
|
||
| to := getModuleAccounts(ak, ctx, moduleAccCount)[0] |
There was a problem hiding this comment.
@robert-zaremba I have implemented getModuleAccounts to generate module accounts. i hope it make sense for you.
Co-authored-by: Robert Zaremba <[email protected]>
|
Looks like there are some failing checks here @cyberbono3 |
Description
My idea is to implement two methods TestSimulateModuleAccountMsgSend and TestSimulateModuleAccountMsgMultiSend() .
Instead of using getTestingAccounts I use getModuleAccounts to generate multiple module accounts. Then I use them here and catch an non nil error .
closes: #8818
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerCodecov Reportin the comment section below once CI passes