Skip to content

Commit

Permalink
(chore): use expected errors in 29fee (#7191)
Browse files Browse the repository at this point in the history
* chore: replace expected boolean errors with expected errors

* use errors rather than strings

* last tests

* linter

* introduce testing aux function

* Update modules/apps/29-fee/ibc_middleware_test.go

Co-authored-by: Carlos Rodriguez <[email protected]>

* Missing file

* mark function as helper

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
  • Loading branch information
bznein and crodriguezvega authored Aug 27, 2024
1 parent 4851524 commit 5db55dd
Show file tree
Hide file tree
Showing 10 changed files with 601 additions and 536 deletions.
190 changes: 96 additions & 94 deletions modules/apps/29-fee/ibc_middleware_test.go

Large diffs are not rendered by default.

246 changes: 134 additions & 112 deletions modules/apps/29-fee/keeper/grpc_query_test.go

Large diffs are not rendered by default.

255 changes: 131 additions & 124 deletions modules/apps/29-fee/keeper/msg_server_test.go

Large diffs are not rendered by default.

10 changes: 5 additions & 5 deletions modules/apps/29-fee/keeper/relay_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,20 +12,20 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
testCases := []struct {
name string
malleate func()
expPass bool
expErr error
}{
{
"success",
func() {
suite.chainB.GetSimApp().IBCFeeKeeper.SetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketID(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID, 1), suite.chainA.SenderAccount.GetAddress().String())
suite.chainB.GetSimApp().IBCFeeKeeper.SetCounterpartyPayeeAddress(suite.chainB.GetContext(), suite.chainA.SenderAccount.GetAddress().String(), suite.chainB.SenderAccount.GetAddress().String(), suite.path.EndpointB.ChannelID)
},
true,
nil,
},
{
"relayer address not set for async WriteAcknowledgement",
func() {},
false,
types.ErrRelayerNotFoundForAsyncAck,
},
}

Expand Down Expand Up @@ -61,7 +61,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {

err := suite.chainB.GetSimApp().IBCFeeKeeper.WriteAcknowledgement(suite.chainB.GetContext(), chanCap, packet, ack)

if tc.expPass {
if tc.expErr == nil {
suite.Require().NoError(err)
_, found := suite.chainB.GetSimApp().IBCFeeKeeper.GetRelayerAddressForAsyncAck(suite.chainB.GetContext(), channeltypes.NewPacketID(suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, 1))
suite.Require().False(found)
Expand All @@ -70,7 +70,7 @@ func (suite *KeeperTestSuite) TestWriteAcknowledgementAsync() {
committedAck, _ := suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.GetPacketAcknowledgement(suite.chainB.GetContext(), packet.DestinationPort, packet.DestinationChannel, 1)
suite.Require().Equal(committedAck, channeltypes.CommitAcknowledgement(expectedAck.Acknowledgement()))
} else {
suite.Require().Error(err)
suite.Require().ErrorIs(err, tc.expErr)
}
})
}
Expand Down
18 changes: 10 additions & 8 deletions modules/apps/29-fee/types/codec_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types_test

import (
"errors"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -10,38 +11,39 @@ import (

fee "github.com/cosmos/ibc-go/v9/modules/apps/29-fee"
"github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)

func TestCodecTypeRegistration(t *testing.T) {
testCases := []struct {
name string
typeURL string
expPass bool
expErr error
}{
{
"success: MsgPayPacketFee",
sdk.MsgTypeURL(&types.MsgPayPacketFee{}),
true,
nil,
},
{
"success: MsgPayPacketFeeAsync",
sdk.MsgTypeURL(&types.MsgPayPacketFeeAsync{}),
true,
nil,
},
{
"success: MsgRegisterPayee",
sdk.MsgTypeURL(&types.MsgRegisterPayee{}),
true,
nil,
},
{
"success: MsgRegisterCounterpartyPayee",
sdk.MsgTypeURL(&types.MsgRegisterCounterpartyPayee{}),
true,
nil,
},
{
"type not registered on codec",
"ibc.invalid.MsgTypeURL",
false,
errors.New("unable to resolve type URL ibc.invalid.MsgTypeURL"),
},
}

Expand All @@ -52,12 +54,12 @@ func TestCodecTypeRegistration(t *testing.T) {
encodingCfg := moduletestutil.MakeTestEncodingConfig(fee.AppModuleBasic{})
msg, err := encodingCfg.Codec.InterfaceRegistry().Resolve(tc.typeURL)

if tc.expPass {
if tc.expErr == nil {
require.NotNil(t, msg)
require.NoError(t, err)
} else {
require.Nil(t, msg)
require.Error(t, err)
ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error())
}
})
}
Expand Down
45 changes: 25 additions & 20 deletions modules/apps/29-fee/types/fee_test.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package types_test

import (
"errors"
"testing"

"github.com/stretchr/testify/require"
Expand All @@ -12,6 +13,8 @@ import (
"github.com/cometbft/cometbft/crypto/secp256k1"

"github.com/cosmos/ibc-go/v9/modules/apps/29-fee/types"
ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors"
ibctesting "github.com/cosmos/ibc-go/v9/testing"
)

var (
Expand Down Expand Up @@ -131,26 +134,26 @@ func TestPacketFeeValidation(t *testing.T) {
testCases := []struct {
name string
malleate func()
expPass bool
expErr error
}{
{
"success",
func() {},
true,
nil,
},
{
"success with empty slice for Relayers",
func() {
packetFee.Relayers = []string{}
},
true,
nil,
},
{
"should fail when refund address is invalid",
func() {
packetFee.RefundAddress = invalidAddress
},
false,
errors.New("failed to convert RefundAddress into sdk.AccAddress"),
},
{
"should fail when all fees are invalid",
Expand All @@ -159,37 +162,37 @@ func TestPacketFeeValidation(t *testing.T) {
packetFee.Fee.RecvFee = invalidFee
packetFee.Fee.TimeoutFee = invalidFee
},
false,
ibcerrors.ErrInvalidCoins,
},
{
"should fail with single invalid fee",
func() {
packetFee.Fee.AckFee = invalidFee
},
false,
ibcerrors.ErrInvalidCoins,
},
{
"should fail with two invalid fees",
func() {
packetFee.Fee.TimeoutFee = invalidFee
packetFee.Fee.AckFee = invalidFee
},
false,
ibcerrors.ErrInvalidCoins,
},
{
"should pass with two empty fees",
func() {
packetFee.Fee.TimeoutFee = sdk.Coins{}
packetFee.Fee.AckFee = sdk.Coins{}
},
true,
nil,
},
{
"should pass with one empty fee",
func() {
packetFee.Fee.TimeoutFee = sdk.Coins{}
},
true,
nil,
},
{
"should fail if all fees are empty",
Expand All @@ -198,31 +201,33 @@ func TestPacketFeeValidation(t *testing.T) {
packetFee.Fee.RecvFee = sdk.Coins{}
packetFee.Fee.TimeoutFee = sdk.Coins{}
},
false,
ibcerrors.ErrInvalidCoins,
},
{
"should fail with non empty Relayers",
func() {
packetFee.Relayers = []string{"relayer"}
},
false,
types.ErrRelayersNotEmpty,
},
}

for _, tc := range testCases {
tc := tc

fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
packetFee = types.NewPacketFee(fee, defaultAccAddress, nil)
t.Run(tc.name, func(t *testing.T) {
fee := types.NewFee(defaultRecvFee, defaultAckFee, defaultTimeoutFee)
packetFee = types.NewPacketFee(fee, defaultAccAddress, nil)

tc.malleate() // malleate mutates test data
tc.malleate() // malleate mutates test data

err := packetFee.Validate()
err := packetFee.Validate()

if tc.expPass {
require.NoError(t, err, tc.name)
} else {
require.Error(t, err, tc.name)
}
if tc.expErr == nil {
require.NoError(t, err, tc.name)
} else {
ibctesting.RequireErrorIsOrContains(t, err, tc.expErr, err.Error())
}
})
}
}
Loading

0 comments on commit 5db55dd

Please sign in to comment.