Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test: cleanup ics29 types tests #1006

Merged
merged 2 commits into from
Feb 25, 2022
Merged

test: cleanup ics29 types tests #1006

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1cf5425
refactoring ics29 types tests
damiannolan Feb 24, 2022
7d147cf
Merge branch 'ics29-fee-middleware' into damian/980-update-ics29-tests
damiannolan Feb 25, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
84 changes: 50 additions & 34 deletions modules/apps/29-fee/types/fee_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,30 +1,42 @@
package types
package types_test

import (
"testing"

sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/stretchr/testify/require"
"github.com/tendermint/tendermint/crypto/secp256k1"

"github.com/cosmos/ibc-go/v3/modules/apps/29-fee/types"
)

var (
// defaultRecvFee is the default packet receive fee used for testing purposes
defaultRecvFee = sdk.NewCoins(sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)})

// defaultAckFee is the default packet acknowledgement fee used for testing purposes
defaultAckFee = sdk.NewCoins(sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(200)})

// defaultTimeoutFee is the default packet timeout fee used for testing purposes
defaultTimeoutFee = sdk.NewCoins(sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(300)})

// invalidFee is an invalid coin set used to trigger error cases for testing purposes
invalidFee = sdk.Coins{sdk.Coin{Denom: "invalid-denom", Amount: sdk.NewInt(-2)}}

// defaultAccAddress is the default account used for testing purposes
defaultAccAddress = sdk.AccAddress(secp256k1.GenPrivKey().PubKey().Address()).String()
)

func TestFeeTotal(t *testing.T) {
fee := Fee{
AckFee: sdk.NewCoins(sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}),
RecvFee: sdk.NewCoins(sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}),
TimeoutFee: sdk.NewCoins(sdk.Coin{Denom: sdk.DefaultBondDenom, Amount: sdk.NewInt(100)}),
}
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)

total := fee.Total()
require.Equal(t, sdk.NewInt(300), total.AmountOf(sdk.DefaultBondDenom))
require.Equal(t, sdk.NewInt(600), total.AmountOf(sdk.DefaultBondDenom))
}

// TestFeeValidation tests Validate
func TestFeeValidation(t *testing.T) {
func TestPacketFeeValidation(t *testing.T) {
var (
fee Fee
ackFee sdk.Coins
receiveFee sdk.Coins
timeoutFee sdk.Coins
packetFee types.PacketFee
)

testCases := []struct {
Expand All @@ -37,66 +49,70 @@ func TestFeeValidation(t *testing.T) {
func() {},
true,
},
{
"should fail when refund address is invalid",
func() {
packetFee.RefundAddress = "invalid-address"
},
false,
},
{
"should fail when all fees are invalid",
func() {
ackFee = invalidCoins
receiveFee = invalidCoins
timeoutFee = invalidCoins
packetFee.Fee.AckFee = invalidFee
packetFee.Fee.RecvFee = invalidFee
packetFee.Fee.TimeoutFee = invalidFee
},
false,
},
{
"should fail with single invalid fee",
func() {
ackFee = invalidCoins
packetFee.Fee.AckFee = invalidFee
},
false,
},
{
"should fail with two invalid fees",
func() {
timeoutFee = invalidCoins
ackFee = invalidCoins
packetFee.Fee.TimeoutFee = invalidFee
packetFee.Fee.AckFee = invalidFee
},
false,
},
{
"should pass with two empty fees",
func() {
timeoutFee = sdk.Coins{}
ackFee = sdk.Coins{}
packetFee.Fee.TimeoutFee = sdk.Coins{}
packetFee.Fee.AckFee = sdk.Coins{}
},
true,
},
{
"should pass with one empty fee",
func() {
timeoutFee = sdk.Coins{}
packetFee.Fee.TimeoutFee = sdk.Coins{}
},
true,
},
{
"should fail if all fees are empty",
func() {
ackFee = sdk.Coins{}
receiveFee = sdk.Coins{}
timeoutFee = sdk.Coins{}
packetFee.Fee.AckFee = sdk.Coins{}
packetFee.Fee.RecvFee = sdk.Coins{}
packetFee.Fee.TimeoutFee = sdk.Coins{}
},
false,
},
}

for _, tc := range testCases {
// build message
ackFee = validCoins
receiveFee = validCoins
timeoutFee = validCoins

// malleate
tc.malleate()
fee = Fee{receiveFee, ackFee, timeoutFee}
err := fee.Validate()
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
packetFee = types.NewPacketFee(fee, defaultAccAddress, nil)
Comment on lines +110 to +111
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why don't you create a defaultPacketFee

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This packetFee is mutated by malleate() in each of the testcases, so creating a defaultPacketFee as a global test variable might not be great for reusing in other tests, as whatever its state is after the last case would carry forward to the next test. So you'd have to be careful resetting it..etc

I think here, because the packetFee is the subject under test it makes more sense to not be using a global var. The fields defined globally are fairly "static" for lack of a better word and more so boilerplate for not over bloating the test cases with initialisation code.


tc.malleate() // malleate mutates test data

err := packetFee.Validate()

if tc.expPass {
require.NoError(t, err)
Expand Down
Loading