From 3267c7a41001162279b1e7a02f2c68fbbc1ad7c5 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Fri, 25 Nov 2022 23:42:59 +0530 Subject: [PATCH 01/11] Update ibc_transfer event testing to use AssertEvents from common module --- .../apps/transfer/keeper/msg_server_test.go | 21 ++++++++-- testing/events.go | 42 +++++++++++++++++++ 2 files changed, 59 insertions(+), 4 deletions(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index 0fe65b4ebdc..7031b57b6a5 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -3,6 +3,7 @@ package keeper_test import ( sdk "github.com/cosmos/cosmos-sdk/types" banktypes "github.com/cosmos/cosmos-sdk/x/bank/types" + ibctesting "github.com/cosmos/ibc-go/v6/testing" "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" ) @@ -38,7 +39,10 @@ func (suite *KeeperTestSuite) assertTransferEvents( } func (suite *KeeperTestSuite) TestMsgTransfer() { - var msg *types.MsgTransfer + var ( + msg *types.MsgTransfer + expEvents map[string]map[string]string + ) testCases := []struct { name string @@ -127,18 +131,27 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(ctx), msg) + events := ctx.EventManager().Events() + expEvents = map[string]map[string]string{ + "ibc_transfer": { + "sender": suite.chainA.SenderAccount.GetAddress().String(), + "receiver": suite.chainB.SenderAccount.GetAddress().String(), + "amount": coin.Amount.String(), + "denom": coin.Denom, + "memo": "memo", + }, + } + if tc.expPass { suite.Require().NoError(err) suite.Require().NotNil(res) suite.Require().NotEqual(res.Sequence, uint64(0)) - events := ctx.EventManager().Events() - suite.assertTransferEvents(events, coin, "memo") + ibctesting.AssertEvents(suite.Suite, expEvents, events) } else { suite.Require().Error(err) suite.Require().Nil(res) - events := ctx.EventManager().Events() suite.Require().Len(events, 0) } }) diff --git a/testing/events.go b/testing/events.go index 46dfbcf987e..cb32121b46b 100644 --- a/testing/events.go +++ b/testing/events.go @@ -5,6 +5,7 @@ import ( "strconv" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/suite" clienttypes "github.com/cosmos/ibc-go/v6/modules/core/02-client/types" connectiontypes "github.com/cosmos/ibc-go/v6/modules/core/03-connection/types" @@ -129,3 +130,44 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { } return nil, fmt.Errorf("acknowledgement event attribute not found") } + +// AssertEvents asserts that expected events are present in the actual events +// Note: nil value and empty map is handled seperatly as follows +// 1. if expected is empty map, then assert actual events are empty +// 2. if expected is nil, skip the whole assert and return early +func AssertEvents( + suite suite.Suite, + expected map[string]map[string]string, + actual sdk.Events, +) { + if expected == nil { + suite.Require().True(true) + return + } + if len(expected) == 0 { + suite.Require().Len(actual, 0) + return + } + + hasEvents := make(map[string]bool) + for eventType := range expected { + hasEvents[eventType] = false + } + + for _, event := range actual { + expEvent, eventFound := expected[event.Type] + if eventFound { + hasEvents[event.Type] = true + suite.Require().Len(event.Attributes, len(expEvent)) + for _, attr := range event.Attributes { + expValue, found := expEvent[string(attr.Key)] + suite.Require().True(found) + suite.Require().Equal(expValue, string(attr.Value)) + } + } + } + + for eventName, hasEvent := range hasEvents { + suite.Require().True(hasEvent, "event: %s was not found in events", eventName) + } +} From 126b73e040979ca0497ae6d26b6cd3610e688cc2 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Sat, 26 Nov 2022 22:55:44 +0530 Subject: [PATCH 02/11] Update assertion of events --- .../apps/transfer/keeper/msg_server_test.go | 29 ++++++++++++------- 1 file changed, 18 insertions(+), 11 deletions(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index 7031b57b6a5..d2f6a7c6cec 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -41,7 +41,7 @@ func (suite *KeeperTestSuite) assertTransferEvents( func (suite *KeeperTestSuite) TestMsgTransfer() { var ( msg *types.MsgTransfer - expEvents map[string]map[string]string + hasEvents bool ) testCases := []struct { @@ -51,12 +51,15 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { }{ { "success", - func() {}, + func() { + hasEvents = true + }, true, }, { "bank send enabled for denom", func() { + hasEvents = true suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), banktypes.Params{ SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: true}}, @@ -113,6 +116,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { for _, tc := range testCases { suite.Run(tc.name, func() { suite.SetupTest() + hasEvents = false // must be explicitly set path := NewTransferPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) @@ -131,8 +135,18 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(ctx), msg) + if tc.expPass { + suite.Require().NoError(err) + suite.Require().NotNil(res) + suite.Require().NotEqual(res.Sequence, uint64(0)) + } else { + suite.Require().Error(err) + suite.Require().Nil(res) + } + + // Verify events events := ctx.EventManager().Events() - expEvents = map[string]map[string]string{ + expEvents := map[string]map[string]string{ "ibc_transfer": { "sender": suite.chainA.SenderAccount.GetAddress().String(), "receiver": suite.chainB.SenderAccount.GetAddress().String(), @@ -142,16 +156,9 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { }, } - if tc.expPass { - suite.Require().NoError(err) - suite.Require().NotNil(res) - suite.Require().NotEqual(res.Sequence, uint64(0)) - + if hasEvents { ibctesting.AssertEvents(suite.Suite, expEvents, events) } else { - suite.Require().Error(err) - suite.Require().Nil(res) - suite.Require().Len(events, 0) } }) From 517d00604c018bc69a17ebdef4a303184cfac970 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Sat, 26 Nov 2022 23:44:32 +0530 Subject: [PATCH 03/11] Add changelog, update AssertEvents to pass as pointer --- CHANGELOG.md | 1 + testing/events.go | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 08a7139458d..05f8c890d37 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -118,6 +118,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core/24-host) [\#2820](https://github.com/cosmos/ibc-go/pull/2820) Add `MustParseClientStatePath` which parses the clientID from a client state key path. * (apps/27-interchain-accounts) [\#2147](https://github.com/cosmos/ibc-go/pull/2147) Adding a `SubmitTx` gRPC endpoint for the ICS27 Controller module which allows owners of interchain accounts to submit transactions. This replaces the previously existing need for authentication modules to implement this standard functionality. * (testing/simapp) [\#2190](https://github.com/cosmos/ibc-go/pull/2190) Adding the new `x/group` cosmos-sdk module to simapp. +* (testing) [\#2829](https://github.com/cosmos/ibc-go/pull/2829) Add `AssertEvents` which asserts events against expected event map. ### Bug Fixes diff --git a/testing/events.go b/testing/events.go index cb32121b46b..b36fabeee53 100644 --- a/testing/events.go +++ b/testing/events.go @@ -136,7 +136,7 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { // 1. if expected is empty map, then assert actual events are empty // 2. if expected is nil, skip the whole assert and return early func AssertEvents( - suite suite.Suite, + suite *suite.Suite, expected map[string]map[string]string, actual sdk.Events, ) { From 3ce88e0baea0a90484988115c1cb5c2e9fabde80 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Sat, 26 Nov 2022 23:54:46 +0530 Subject: [PATCH 04/11] Pass suite by pointer to ibctest --- modules/apps/transfer/keeper/msg_server_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index d2f6a7c6cec..80f64286529 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -157,7 +157,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { } if hasEvents { - ibctesting.AssertEvents(suite.Suite, expEvents, events) + ibctesting.AssertEvents(&suite.Suite, expEvents, events) } else { suite.Require().Len(events, 0) } From 489d184e658e5e3773276d5501cba2b08c65d8ba Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Sun, 27 Nov 2022 20:25:56 +0530 Subject: [PATCH 05/11] Remove nil and zero values of expected map --- testing/events.go | 13 +------------ 1 file changed, 1 insertion(+), 12 deletions(-) diff --git a/testing/events.go b/testing/events.go index b36fabeee53..61957c2241a 100644 --- a/testing/events.go +++ b/testing/events.go @@ -132,23 +132,12 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { } // AssertEvents asserts that expected events are present in the actual events -// Note: nil value and empty map is handled seperatly as follows -// 1. if expected is empty map, then assert actual events are empty -// 2. if expected is nil, skip the whole assert and return early +// expected map needs to be a subset of actual events to pass func AssertEvents( suite *suite.Suite, expected map[string]map[string]string, actual sdk.Events, ) { - if expected == nil { - suite.Require().True(true) - return - } - if len(expected) == 0 { - suite.Require().Len(actual, 0) - return - } - hasEvents := make(map[string]bool) for eventType := range expected { hasEvents[eventType] = false From f507ef7f478d88649f4fe2426a028bc7e3ccaedb Mon Sep 17 00:00:00 2001 From: Anmol Date: Tue, 29 Nov 2022 14:49:49 +0530 Subject: [PATCH 06/11] Update testing/events.go Co-authored-by: Carlos Rodriguez --- testing/events.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/events.go b/testing/events.go index 61957c2241a..e0c23739572 100644 --- a/testing/events.go +++ b/testing/events.go @@ -131,7 +131,7 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { return nil, fmt.Errorf("acknowledgement event attribute not found") } -// AssertEvents asserts that expected events are present in the actual events +// AssertEvents asserts that expected events are present in the actual events. // expected map needs to be a subset of actual events to pass func AssertEvents( suite *suite.Suite, From 9991a425d655514b6e6d7cf0d0b62dddcd33cbe0 Mon Sep 17 00:00:00 2001 From: Anmol Date: Tue, 29 Nov 2022 14:49:58 +0530 Subject: [PATCH 07/11] Update testing/events.go Co-authored-by: Carlos Rodriguez --- testing/events.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/testing/events.go b/testing/events.go index e0c23739572..2bf332ed9c2 100644 --- a/testing/events.go +++ b/testing/events.go @@ -132,7 +132,7 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { } // AssertEvents asserts that expected events are present in the actual events. -// expected map needs to be a subset of actual events to pass +// Expected map needs to be a subset of actual events to pass. func AssertEvents( suite *suite.Suite, expected map[string]map[string]string, From 4283fd69abb10ebe1bc0798e64d05b6c3ec2f29a Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Tue, 29 Nov 2022 17:04:34 +0530 Subject: [PATCH 08/11] Add type to events map --- modules/apps/transfer/keeper/msg_server_test.go | 2 +- testing/events.go | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index 80f64286529..47f33b9aec1 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -146,7 +146,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { // Verify events events := ctx.EventManager().Events() - expEvents := map[string]map[string]string{ + expEvents := ibctesting.EventsMap{ "ibc_transfer": { "sender": suite.chainA.SenderAccount.GetAddress().String(), "receiver": suite.chainB.SenderAccount.GetAddress().String(), diff --git a/testing/events.go b/testing/events.go index 2bf332ed9c2..22f6b81aeda 100644 --- a/testing/events.go +++ b/testing/events.go @@ -12,6 +12,8 @@ import ( channeltypes "github.com/cosmos/ibc-go/v6/modules/core/04-channel/types" ) +type EventsMap map[string]map[string]string + // ParseClientIDFromEvents parses events emitted from a MsgCreateClient and returns the // client identifier. func ParseClientIDFromEvents(events sdk.Events) (string, error) { @@ -135,7 +137,7 @@ func ParseAckFromEvents(events sdk.Events) ([]byte, error) { // Expected map needs to be a subset of actual events to pass. func AssertEvents( suite *suite.Suite, - expected map[string]map[string]string, + expected EventsMap, actual sdk.Events, ) { hasEvents := make(map[string]bool) From 1585467466c0c1a467e0cb73ede954a0726380b6 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Sat, 10 Dec 2022 21:06:14 +0530 Subject: [PATCH 09/11] Use expPass instead of hasEvents for checking for events --- modules/apps/transfer/keeper/msg_server_test.go | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index 47f33b9aec1..4c636ae823d 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -40,8 +40,7 @@ func (suite *KeeperTestSuite) assertTransferEvents( func (suite *KeeperTestSuite) TestMsgTransfer() { var ( - msg *types.MsgTransfer - hasEvents bool + msg *types.MsgTransfer ) testCases := []struct { @@ -51,15 +50,12 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { }{ { "success", - func() { - hasEvents = true - }, + func() {}, true, }, { "bank send enabled for denom", func() { - hasEvents = true suite.chainA.GetSimApp().BankKeeper.SetParams(suite.chainA.GetContext(), banktypes.Params{ SendEnabled: []*banktypes.SendEnabled{{Denom: sdk.DefaultBondDenom, Enabled: true}}, @@ -116,7 +112,6 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { for _, tc := range testCases { suite.Run(tc.name, func() { suite.SetupTest() - hasEvents = false // must be explicitly set path := NewTransferPath(suite.chainA, suite.chainB) suite.coordinator.Setup(path) @@ -156,7 +151,7 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { }, } - if hasEvents { + if tc.expPass { ibctesting.AssertEvents(&suite.Suite, expEvents, events) } else { suite.Require().Len(events, 0) From 3cc23c4378e24ea451b58130a3ae8dfc69a9a0fc Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Sat, 10 Dec 2022 21:07:58 +0530 Subject: [PATCH 10/11] Combine the code blocks for verification of events --- modules/apps/transfer/keeper/msg_server_test.go | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index 4c636ae823d..fccc6a305ba 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -130,15 +130,6 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { ctx := suite.chainA.GetContext() res, err := suite.chainA.GetSimApp().TransferKeeper.Transfer(sdk.WrapSDKContext(ctx), msg) - if tc.expPass { - suite.Require().NoError(err) - suite.Require().NotNil(res) - suite.Require().NotEqual(res.Sequence, uint64(0)) - } else { - suite.Require().Error(err) - suite.Require().Nil(res) - } - // Verify events events := ctx.EventManager().Events() expEvents := ibctesting.EventsMap{ @@ -152,8 +143,13 @@ func (suite *KeeperTestSuite) TestMsgTransfer() { } if tc.expPass { + suite.Require().NoError(err) + suite.Require().NotNil(res) + suite.Require().NotEqual(res.Sequence, uint64(0)) ibctesting.AssertEvents(&suite.Suite, expEvents, events) } else { + suite.Require().Error(err) + suite.Require().Nil(res) suite.Require().Len(events, 0) } }) From 7ee6b8b8666991f9c092603576667d3d74563481 Mon Sep 17 00:00:00 2001 From: Anmol1696 Date: Mon, 12 Dec 2022 20:10:10 +0530 Subject: [PATCH 11/11] Remove unused function assertTransferEvents --- .../apps/transfer/keeper/msg_server_test.go | 30 ------------------- 1 file changed, 30 deletions(-) diff --git a/modules/apps/transfer/keeper/msg_server_test.go b/modules/apps/transfer/keeper/msg_server_test.go index fccc6a305ba..e457d3894cb 100644 --- a/modules/apps/transfer/keeper/msg_server_test.go +++ b/modules/apps/transfer/keeper/msg_server_test.go @@ -8,36 +8,6 @@ import ( "github.com/cosmos/ibc-go/v6/modules/apps/transfer/types" ) -func (suite *KeeperTestSuite) assertTransferEvents( - actualEvents sdk.Events, - coin sdk.Coin, - memo string, -) { - hasEvent := false - - expEvent := map[string]string{ - sdk.AttributeKeySender: suite.chainA.SenderAccount.GetAddress().String(), - types.AttributeKeyReceiver: suite.chainB.SenderAccount.GetAddress().String(), - types.AttributeKeyAmount: coin.Amount.String(), - types.AttributeKeyDenom: coin.Denom, - types.AttributeKeyMemo: memo, - } - - for _, event := range actualEvents { - if event.Type == types.EventTypeTransfer { - hasEvent = true - suite.Require().Len(event.Attributes, len(expEvent)) - for _, attr := range event.Attributes { - expValue, found := expEvent[string(attr.Key)] - suite.Require().True(found) - suite.Require().Equal(expValue, string(attr.Value)) - } - } - } - - suite.Require().True(hasEvent, "event: %s was not found in events", types.EventTypeTransfer) -} - func (suite *KeeperTestSuite) TestMsgTransfer() { var ( msg *types.MsgTransfer