test(types/tx/msgs.go): add unit tests #20199
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||
|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,195 @@ | ||||||||
| package tx_test | ||||||||
|
|
||||||||
| import ( | ||||||||
| "testing" | ||||||||
|
|
||||||||
| "github.com/cosmos/gogoproto/proto" | ||||||||
| "github.com/stretchr/testify/require" | ||||||||
|
|
||||||||
| "github.com/cosmos/cosmos-sdk/codec" | ||||||||
| "github.com/cosmos/cosmos-sdk/codec/types" | ||||||||
| codectypes "github.com/cosmos/cosmos-sdk/codec/types" | ||||||||
|
There was a problem hiding this comment.
Suggested change
|
||||||||
| sdk "github.com/cosmos/cosmos-sdk/types" | ||||||||
| "github.com/cosmos/cosmos-sdk/types/tx" | ||||||||
| ) | ||||||||
|
|
||||||||
| func Test_SetMsg(t *testing.T) { | ||||||||
| cases := []struct { | ||||||||
| name string | ||||||||
| msg sdk.Msg | ||||||||
| expErr bool | ||||||||
| }{ | ||||||||
| { | ||||||||
| name: "Set nil Msg", | ||||||||
| msg: nil, | ||||||||
| expErr: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Set a valid message", | ||||||||
| msg: &DummyProtoMessage1{}, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| } | ||||||||
|
|
||||||||
| for _, tc := range cases { | ||||||||
| t.Run(tc.name, func(t *testing.T) { | ||||||||
| actual, err := tx.SetMsg(tc.msg) | ||||||||
| if tc.expErr { | ||||||||
| require.Error(t, err) | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| expected := mustAny(tc.msg) | ||||||||
|
There was a problem hiding this comment. This calls the same There was a problem hiding this comment. Agree. It will be better to test cache, url and bytes. Thank you for the suggestion Fixed. The function "mustAny" is removed |
||||||||
| require.Equal(t, expected, actual) | ||||||||
| require.Nil(t, err) | ||||||||
|
There was a problem hiding this comment. let's check this first to fail fast. |
||||||||
| }) | ||||||||
| } | ||||||||
| } | ||||||||
|
There was a problem hiding this comment. I find the map type easier to read and you can get rid of the name field. For example: specs := map[string]struct{
msg sdk.Msg
expErr bool
}{
"Set nil Msg": {
msg: nil,
expErr: true,
},
}
for name, spec := range specs {
t.Run(name, func(t *testing.T) {
})
}There was a problem hiding this comment. Thanks for the comment. I really like how it looks with a map. I will start using map[string]struct{} instead struct{}. Fixed |
||||||||
|
|
||||||||
| func Test_SetMsgs(t *testing.T) { | ||||||||
| cases := []struct { | ||||||||
| name string | ||||||||
| msgs []sdk.Msg | ||||||||
| expErr bool | ||||||||
| }{ | ||||||||
| { | ||||||||
| name: "Set nil slice of messages", | ||||||||
| msgs: nil, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Set empty slice of messages", | ||||||||
| msgs: []sdk.Msg{}, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Set nil message inside the slice of messages", | ||||||||
| msgs: []sdk.Msg{nil}, | ||||||||
| expErr: true, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Set valid messages", | ||||||||
| msgs: []sdk.Msg{&DummyProtoMessage1{}, &DummyProtoMessage2{}}, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| } | ||||||||
|
|
||||||||
| for _, tc := range cases { | ||||||||
| t.Run(tc.name, func(t *testing.T) { | ||||||||
| actual, err := tx.SetMsgs(tc.msgs) | ||||||||
| if tc.expErr { | ||||||||
| require.Error(t, err) | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| expected := make([]*types.Any, len(tc.msgs)) | ||||||||
| for i, msg := range tc.msgs { | ||||||||
| a := mustAny(msg) | ||||||||
| expected[i] = a | ||||||||
| } | ||||||||
|
|
||||||||
| require.Equal(t, expected, actual) | ||||||||
| require.Nil(t, err) | ||||||||
|
There was a problem hiding this comment. Same comments as above btw, personal preference: There was a problem hiding this comment. Your idea is to remove I think we should keep tx.SetMsg but we can simplified it from: func SetMsg(msg sdk.Msg) (*types.Any, error) {
any, err := types.NewAnyWithValue(msg)
if err != nil {
return nil, err
}
return any, nil
}to: func SetMsg(msg sdk.Msg) (*types.Any, error) {
return types.NewAnyWithValue(msg)
} |
||||||||
| }) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| func Test_GetMsgs(t *testing.T) { | ||||||||
| sdkMsgs := []sdk.Msg{&DummyProtoMessage1{}, &DummyProtoMessage2{}} | ||||||||
| anyMsg, err := tx.SetMsgs(sdkMsgs) | ||||||||
| require.Nil(t, err) | ||||||||
|
|
||||||||
| cases := []struct { | ||||||||
| name string | ||||||||
| msgs []*types.Any | ||||||||
| expected []sdk.Msg | ||||||||
| expErr bool | ||||||||
| }{ | ||||||||
| { | ||||||||
| name: "GetMsgs from a nil slice of Any messages", | ||||||||
| msgs: nil, | ||||||||
| expected: []sdk.Msg{}, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "GetMsgs from empty slice of Any messages", | ||||||||
| msgs: []*types.Any{}, | ||||||||
| expected: []sdk.Msg{}, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "GetMsgs from a slice with valid Any messages", | ||||||||
| msgs: anyMsg, | ||||||||
| expected: sdkMsgs, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "GetMsgs from a slice that contains uncached Any message", | ||||||||
| msgs: []*types.Any{{}}, | ||||||||
| expErr: true, | ||||||||
| }, | ||||||||
| } | ||||||||
|
|
||||||||
| for _, tc := range cases { | ||||||||
| t.Run(tc.name, func(t *testing.T) { | ||||||||
| actual, err := tx.GetMsgs(tc.msgs, "dummy") | ||||||||
| if tc.expErr { | ||||||||
| require.Error(t, err) | ||||||||
| return | ||||||||
| } | ||||||||
|
|
||||||||
| require.Equal(t, tc.expected, actual) | ||||||||
| require.Nil(t, err) | ||||||||
| }) | ||||||||
| } | ||||||||
| } | ||||||||
|
|
||||||||
| func TestTx_UnpackInterfaces(t *testing.T) { | ||||||||
| unpacker := codec.NewProtoCodec(codectypes.NewInterfaceRegistry()) | ||||||||
|
|
||||||||
| sdkMsgs := []sdk.Msg{&DummyProtoMessage1{}, &DummyProtoMessage2{}} | ||||||||
| anyMsg, err := tx.SetMsgs(sdkMsgs) | ||||||||
| require.Nil(t, err) | ||||||||
|
|
||||||||
| cases := []struct { | ||||||||
| name string | ||||||||
| msgs []*types.Any | ||||||||
| expErr bool | ||||||||
| }{ | ||||||||
| { | ||||||||
| name: "Unpack nil slice messages", | ||||||||
| msgs: nil, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Unpack empty slice of messages", | ||||||||
| msgs: []*types.Any{}, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Unpack valid messages", | ||||||||
| msgs: anyMsg, | ||||||||
| expErr: false, | ||||||||
| }, | ||||||||
| { | ||||||||
| name: "Unpack uncashed message", | ||||||||
| msgs: []*types.Any{{TypeUrl: "uncached"}}, | ||||||||
| expErr: true, | ||||||||
| }, | ||||||||
| } | ||||||||
|
|
||||||||
| for _, tc := range cases { | ||||||||
| t.Run(tc.name, func(t *testing.T) { | ||||||||
| err = tx.UnpackInterfaces(unpacker, tc.msgs) | ||||||||
| require.Equal(t, tc.expErr, err != nil) | ||||||||
| }) | ||||||||
| } | ||||||||
|
|
||||||||
| } | ||||||||
|
|
||||||||
| func mustAny(msg proto.Message) *types.Any { | ||||||||
| a, err := types.NewAnyWithValue(msg) | ||||||||
| if err != nil { | ||||||||
| panic(err) | ||||||||
| } | ||||||||
| return a | ||||||||
| } | ||||||||
GitHub Actions 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this? Unless you have to avoid cicular dependencies or want to call from the "outside", the _test package mostly just adds complexity, IMHO.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this practice because it improving package design since using the _test package forces you to test the package as if you are an external user, it encourages you to think more carefully about the design of your package’s API. You'll need to ensure that all necessary functionality is properly exposed through exported types and functions, which can lead to better, more user-friendly design. If you have problems with testing and using package probably the clients of the package will have the same problems.