Skip to content

Commit

Permalink
chore: expect specific errors in apps/transfer (#7205)
Browse files Browse the repository at this point in the history 
* chore: expect specific errors in apps/transfer

* revert changed line

* mark function as helper

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
  • Loading branch information
@bznein @crodriguezvega
bznein and crodriguezvega authored Aug 28, 2024
1 parent 30371fa commit 1af0bfc
Show file tree
Hide file tree
Showing 8 changed files with 122 additions and 113 deletions.
76 changes: 39 additions & 37 deletions modules/apps/transfer/keeper/grpc_query_test.go
View file
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package keeper_test

import (
"errors"
"fmt"

sdkmath "cosmossdk.io/math"
Expand All @@ -21,7 +22,7 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
testCases := []struct {
msg string
malleate func()
expPass bool
expErr error
}{
{
"success: correct ibc denom",
Expand All @@ -37,7 +38,7 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
Hash: expDenom.IBCDenom(),
}
},
true,
nil,
},
{
"success: correct hex hash",
Expand All @@ -53,7 +54,7 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
Hash: expDenom.Hash().String(),
}
},
true,
nil,
},
{
"failure: invalid hash",
Expand All @@ -62,7 +63,7 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
Hash: "!@#!@#!",
}
},
false,
errors.New("invalid denom trace hash"),
},
{
"failure: not found denom trace",
Expand All @@ -77,7 +78,7 @@ func (suite *KeeperTestSuite) TestQueryDenom() {
Hash: expDenom.IBCDenom(),
}
},
false,
errors.New("denomination not found"),
},
}

Expand All @@ -91,12 +92,12 @@ func (suite *KeeperTestSuite) TestQueryDenom() {

res, err := suite.chainA.GetSimApp().TransferKeeper.Denom(ctx, req)

if tc.expPass {
if tc.expErr == nil {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(&expDenom, res.Denom)
} else {
suite.Require().Error(err)
ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error())
}
})
}
Expand All @@ -111,14 +112,14 @@ func (suite *KeeperTestSuite) TestQueryDenoms() {
testCases := []struct {
msg string
malleate func()
expPass bool
expErr error
}{
{
"empty pagination",
func() {
req = &types.QueryDenomsRequest{}
},
true,
nil,
},
{
"success",
Expand All @@ -138,7 +139,7 @@ func (suite *KeeperTestSuite) TestQueryDenoms() {
},
}
},
true,
nil,
},
}

Expand All @@ -152,12 +153,12 @@ func (suite *KeeperTestSuite) TestQueryDenoms() {

res, err := suite.chainA.GetSimApp().TransferKeeper.Denoms(ctx, req)

if tc.expPass {
if tc.expErr == nil {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(expDenoms.Sort(), res.Denoms)
} else {
suite.Require().Error(err)
ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error())
}
})
}
Expand All @@ -181,16 +182,21 @@ func (suite *KeeperTestSuite) TestQueryDenomHash() {
testCases := []struct {
msg string
malleate func()
expPass bool
expErr error
}{
{
"success",
func() {},
nil,
},
{
"invalid trace",
func() {
req = &types.QueryDenomHashRequest{
Trace: "transfer/channelToA/transfer/",
Trace: "transfer%%/channel-1/transfer/channel-1/uatom",
}
},
false,
errors.New("invalid trace"),
},
{
"not found denom trace",
Expand All @@ -199,12 +205,7 @@ func (suite *KeeperTestSuite) TestQueryDenomHash() {
Trace: "transfer/channelToC/uatom",
}
},
false,
},
{
"success",
func() {},
true,
errors.New("denomination not found"),
},
}

Expand All @@ -223,12 +224,12 @@ func (suite *KeeperTestSuite) TestQueryDenomHash() {

res, err := suite.chainA.GetSimApp().TransferKeeper.DenomHash(ctx, req)

if tc.expPass {
if tc.expErr == nil {
suite.Require().NoError(err)
suite.Require().NotNil(res)
suite.Require().Equal(expHash, res.Hash)
} else {
suite.Require().Error(err)
ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error())
}
})
}
Expand All @@ -241,7 +242,7 @@ func (suite *KeeperTestSuite) TestEscrowAddress() {
testCases := []struct {
msg string
malleate func()
expPass bool
expErr error
}{
{
"success",
Expand All @@ -251,7 +252,7 @@ func (suite *KeeperTestSuite) TestEscrowAddress() {
ChannelId: path.EndpointA.ChannelID,
}
},
true,
nil,
},
{
"failure - channel not found",
Expand All @@ -261,7 +262,7 @@ func (suite *KeeperTestSuite) TestEscrowAddress() {
ChannelId: ibctesting.FirstChannelID,
}
},
false,
errors.New("channel not found"),
},
{
"failure - empty channelID",
Expand All @@ -271,7 +272,7 @@ func (suite *KeeperTestSuite) TestEscrowAddress() {
ChannelId: "",
}
},
false,
errors.New("identifier cannot be blank"),
},
{
"failure - empty portID",
Expand All @@ -281,7 +282,7 @@ func (suite *KeeperTestSuite) TestEscrowAddress() {
ChannelId: ibctesting.FirstChannelID,
}
},
false,
errors.New("identifier cannot be blank"),
},
}

Expand All @@ -297,12 +298,12 @@ func (suite *KeeperTestSuite) TestEscrowAddress() {

res, err := suite.chainA.GetSimApp().TransferKeeper.EscrowAddress(ctx, req)

if tc.expPass {
if tc.expErr == nil {
suite.Require().NoError(err)
expected := types.GetEscrowAddress(ibctesting.TransferPort, path.EndpointA.ChannelID).String()
suite.Require().Equal(expected, res.EscrowAddress)
} else {
suite.Require().Error(err)
ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error())
}
})
}
Expand All @@ -317,7 +318,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
testCases := []struct {
msg string
malleate func()
expPass bool
expErr error
}{
{
"valid native denom with escrow amount < 2^63",
Expand All @@ -329,7 +330,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
expEscrowAmount = sdkmath.NewInt(100)
suite.chainA.GetSimApp().TransferKeeper.SetTotalEscrowForDenom(suite.chainA.GetContext(), sdk.NewCoin(sdk.DefaultBondDenom, expEscrowAmount))
},
true,
nil,
},
{
"valid ibc denom with escrow amount > 2^63",
Expand All @@ -345,7 +346,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
Denom: denom.IBCDenom(),
}
},
true,
nil,
},
{
"valid ibc denom treated as native denom",
Expand All @@ -356,7 +357,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
Denom: denom.IBCDenom(),
}
},
true, // denom trace is not found, thus the denom is considered a native token
nil, // denom trace is not found, thus the denom is considered a native token
},
{
"invalid ibc denom treated as valid native denom",
Expand All @@ -365,7 +366,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
Denom: "ibc/123",
}
},
true, // the ibc denom does not contain a valid hash, thus the denom is considered a native token
nil, // the ibc denom does not contain a valid hash, thus the denom is considered a native token
},
{
"invalid denom",
Expand All @@ -374,7 +375,7 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {
Denom: "??𓃠🐾??",
}
},
false,
errors.New("invalid denom"),
},
}

Expand All @@ -389,10 +390,11 @@ func (suite *KeeperTestSuite) TestTotalEscrowForDenom() {

res, err := suite.chainA.GetSimApp().TransferKeeper.TotalEscrowForDenom(ctx, req)

if tc.expPass {
if tc.expErr == nil {
suite.Require().NoError(err)
suite.Require().Equal(expEscrowAmount, res.Amount.Amount)
} else {
ibctesting.RequireErrorIsOrContains(suite.T(), err, tc.expErr, err.Error())
suite.Require().Error(err)
}
})
Expand Down
31 changes: 16 additions & 15 deletions modules/apps/transfer/keeper/keeper_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
testCases := []struct {
name string
instantiateFn func()
expPass bool
panicMsg string
}{
{"success", func() {
keeper.NewKeeper(
Expand All @@ -68,7 +68,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().ScopedTransferKeeper,
suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
)
}, true},
}, ""},
{"failure: transfer module account does not exist", func() {
keeper.NewKeeper(
suite.chainA.GetSimApp().AppCodec(),
Expand All @@ -82,7 +82,7 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().ScopedTransferKeeper,
suite.chainA.GetSimApp().ICAControllerKeeper.GetAuthority(),
)
}, false},
}, "the IBC transfer module account has not been set"},
{"failure: empty authority", func() {
keeper.NewKeeper(
suite.chainA.GetSimApp().AppCodec(),
Expand All @@ -96,20 +96,21 @@ func (suite *KeeperTestSuite) TestNewKeeper() {
suite.chainA.GetSimApp().ScopedTransferKeeper,
"", // authority
)
}, false},
}, "authority must be non-empty"},
}

for _, tc := range testCases {
tc := tc
suite.SetupTest()

suite.Run(tc.name, func() {
if tc.expPass {
if tc.panicMsg == "" {
suite.Require().NotPanics(
tc.instantiateFn,
)
} else {
suite.Require().Panics(
suite.Require().PanicsWithError(
tc.panicMsg,
tc.instantiateFn,
)
}
Expand Down Expand Up @@ -326,15 +327,15 @@ func (suite *KeeperTestSuite) TestGetAllForwardedPackets() {

func (suite *KeeperTestSuite) TestParams() {
testCases := []struct {
name string
input types.Params
expPass bool
name string
input types.Params
panicMsg string
}{
// it is not possible to set invalid booleans
{"success: set params false-false", types.NewParams(false, false), true},
{"success: set params false-true", types.NewParams(false, true), true},
{"success: set params true-false", types.NewParams(true, false), true},
{"success: set params true-true", types.NewParams(true, true), true},
{"success: set params false-false", types.NewParams(false, false), ""},
{"success: set params false-true", types.NewParams(false, true), ""},
{"success: set params true-false", types.NewParams(true, false), ""},
{"success: set params true-true", types.NewParams(true, true), ""},
}

for _, tc := range testCases {
Expand All @@ -343,13 +344,13 @@ func (suite *KeeperTestSuite) TestParams() {
suite.Run(tc.name, func() {
suite.SetupTest() // reset
ctx := suite.chainA.GetContext()
if tc.expPass {
if tc.panicMsg == "" {
suite.chainA.GetSimApp().TransferKeeper.SetParams(ctx, tc.input)
expected := tc.input
p := suite.chainA.GetSimApp().TransferKeeper.GetParams(ctx)
suite.Require().Equal(expected, p)
} else {
suite.Require().Panics(func() {
suite.Require().PanicsWithError(tc.panicMsg, func() {
suite.chainA.GetSimApp().TransferKeeper.SetParams(ctx, tc.input)
})
}
Expand Down
Loading

0 comments on commit 1af0bfc

Please sign in to comment.