Bank: unit tests for SDK v0.44.3 Bump #339
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
feat: add authz and feegrant modules to simapp fix: replace InitializeandSeal with Seal in app
fix: update upgrade handler template
* fix some bugs & structs. upgrade to sdk 0.44.3 * remove authz and feegrant modules * revert banktypes to sdkbanktypes * rename Codec to Marshaler, and fix simulations * fix gov tally logic * add evidence to exportgenesis * refactor voteoptions handling
* add dependencies requirement to lint * update golang version * add timeout config to lint
|
Please make sure to remove all conflicts. And I think some files were added by mistake. |
x/bank/keeper/keeper_test.go
Outdated
|
|
||
| queryHelper := baseapp.NewQueryServerTestHelper(suite.ctx, suite.app.InterfaceRegistry()) | ||
| types.RegisterMsgServer(queryHelper, suite.queryClient) | ||
| suite.queryClient = &types.UnimplementedMsgServer{} |
There was a problem hiding this comment.
Shouldn't we initialize with the module's query client here?
x/bank/keeper/keeper_test.go
Outdated
| args args | ||
| errArgs errArgs | ||
| }{ | ||
| {"Operator(1) Create: first send test case if coins is not greater than total amount", |
There was a problem hiding this comment.
what does Operator Create mean here
x/bank/keeper/keeper_test.go
Outdated
| args args | ||
| errArgs errArgs | ||
| }{ | ||
| {"Operator(1) Create: first send test case if coins is not greater than total amount", |
There was a problem hiding this comment.
also let's make the test names a complete sentence or at least have correct grammar
x/bank/keeper/keeper_test.go
Outdated
| suite.Run(tc.name, func() { | ||
| suite.SetupTest() | ||
| inputs := []bankTypes.Input{ | ||
| {Address: tc.args.Addr1.String(), Coins: sdk.Coins{sdk.NewInt64Coin("ctk", tc.args.amount)}}, |
There was a problem hiding this comment.
you shouldn't be using ctk. it should be common.microctkdenom
x/bank/types/types_test.go
Outdated
| fromAddr: suite.address[0], | ||
| toAddr: suite.address[1], | ||
| unlockerAddress: suite.address[2], | ||
| res: "[696E70757431999]", |
There was a problem hiding this comment.
what does this mean?
yoongbok-lee
requested changes
Jan 31, 2022
| @@ -168,7 +168,6 @@ jobs: | |||
| export BUILD_TAGS="netgo ledger" | |||
| export GOSUM=$(sha256sum go.sum | cut -d ' ' -f1) | |||
| curl -sL https://git.io/goreleaser | bash | |||
|
|
|||
There was a problem hiding this comment.
why delete these lines?
x/bank/keeper/keeper_test.go
Outdated
| if tc.errArgs.shouldPass { | ||
| suite.Require().NoError(err, tc.name) | ||
| suite.Require().NotEqual(tc.args.sendAmt, balance) | ||
| suite.Require().Equal(sdk.Coins{sdk.NewInt64Coin("uctk", tc.args.accBalance)}, balance) |
There was a problem hiding this comment.
use common.MicroCTKDenom
x/bank/keeper/keeper_test.go
Outdated
Comment on lines
202
to
203
| {Address: tc.args.Addr1.String(), Coins: sdk.Coins{sdk.NewInt64Coin("uctk", tc.args.amount)}}, | ||
| {Address: tc.args.Addr1.String(), Coins: sdk.Coins{sdk.NewInt64Coin("uctk", tc.args.amount)}}, |
x/bank/keeper/keeper_test.go
Outdated
Comment on lines
206
to
207
| {Address: tc.args.Addr2.String(), Coins: sdk.Coins{sdk.NewInt64Coin("uctk", tc.args.amount)}}, | ||
| {Address: tc.args.Addr3.String(), Coins: sdk.Coins{sdk.NewInt64Coin("uctk", tc.args.amount)}}, |
There was a problem hiding this comment.
never use hard coded "uctk" in the rest of the codebase
x/bank/keeper/keeper_test.go
Outdated
| suite.Require().NotNil(baseAcc) | ||
| suite.Require().NoError(err) | ||
| suite.Require().NotNil(toAcc) | ||
| suite.Require().Equal(balances, sdk.Coins{sdk.NewInt64Coin("uctk", tc.args.accBalance)}) |
There was a problem hiding this comment.
use NewCoins() instead of direct struct
CHANGELOG.md
Outdated
| ### Bug Fixes | ||
| ### Bug Fixes |
There was a problem hiding this comment.
what is this change for?
app/params/proto.go
Outdated
| } | ||
| } |
There was a problem hiding this comment.
revert this if you can
uwezukwechibuzor
requested review from
hacheigriega and
yoongbok-lee
and removed request for
hacheigriega and
yoongbok-lee
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #XXX
Related: #XXX
Description
For contributor use:
docs/) or specification (x/<module>/spec/)godoccomments.Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorerFor admin use:
WIP,R4R,docs, etc)