(chore): use expected errors in 29fee (#7191)

* 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]>
cosmos · Aug 27, 2024 · 5db55dd · 5db55dd
1 parent 4851524
commit 5db55dd
Show file tree

Hide file tree

Showing 10 changed files with 601 additions and 536 deletions.
diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go
diff --git a/modules/apps/29-fee/keeper/grpc_query_test.go b/modules/apps/29-fee/keeper/grpc_query_test.go
diff --git a/modules/apps/29-fee/keeper/msg_server_test.go b/modules/apps/29-fee/keeper/msg_server_test.go
diff --git a/modules/apps/29-fee/keeper/relay_test.go b/modules/apps/29-fee/keeper/relay_test.go
@@ -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,
 		},
 	}
 
@@ -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)
@@ -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)
 			}
 		})
 	}

diff --git a/modules/apps/29-fee/types/codec_test.go b/modules/apps/29-fee/types/codec_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -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"),
 		},
 	}
 
@@ -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())
 			}
 		})
 	}

diff --git a/modules/apps/29-fee/types/fee_test.go b/modules/apps/29-fee/types/fee_test.go
@@ -1,6 +1,7 @@
 package types_test
 
 import (
+	"errors"
 	"testing"
 
 	"github.com/stretchr/testify/require"
@@ -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 (
@@ -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",
@@ -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",
@@ -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())
+			}
+		})
 	}
 }