-
Notifications
You must be signed in to change notification settings - Fork 3.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
test(types/tx/msgs.go): add unit tests #20199
Changes from 6 commits
7b3c92f
67d1f8f
e3687b5
ed96130
f2c2324
c961c3e
92cce5e
b381aa5
e416dfe
50f51da
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,182 @@ | ||
package tx_test | ||
|
||
import ( | ||
"encoding/json" | ||
"testing" | ||
|
||
"github.com/stretchr/testify/require" | ||
|
||
"github.com/cosmos/cosmos-sdk/codec" | ||
"github.com/cosmos/cosmos-sdk/codec/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/types/tx" | ||
) | ||
|
||
func Test_SetMsg(t *testing.T) { | ||
cases := map[string]struct { | ||
msg sdk.Msg | ||
msgTypeURL string | ||
expErr bool | ||
}{ | ||
"Set nil Msg": { | ||
msg: nil, | ||
expErr: true, | ||
}, | ||
"Set empty message": { | ||
msg: &DummyProtoMessage1{}, | ||
msgTypeURL: "/dummy.proto.message1", | ||
expErr: false, | ||
}, | ||
"Set a valid message": { | ||
msg: &DummyProtoMessage1{Name: "some-name"}, | ||
msgTypeURL: "/dummy.proto.message1", | ||
expErr: false, | ||
}, | ||
} | ||
|
||
for name, tc := range cases { | ||
t.Run(name, func(t *testing.T) { | ||
actual, err := tx.SetMsg(tc.msg) | ||
if tc.expErr { | ||
require.Error(t, err) | ||
return | ||
} | ||
require.NoError(t, err) | ||
|
||
b, err := json.Marshal(tc.msg) | ||
require.NoError(t, err) | ||
|
||
require.Equal(t, tc.msgTypeURL, actual.TypeUrl) | ||
require.Equal(t, b, actual.GetValue()) | ||
require.Equal(t, tc.msg, actual.GetCachedValue()) | ||
}) | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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 := map[string]struct { | ||
msgs []sdk.Msg | ||
msgTypeURL []string | ||
expErr bool | ||
}{ | ||
"Set nil slice of messages": { | ||
msgs: nil, | ||
expErr: false, | ||
}, | ||
"Set empty slice of messages": { | ||
msgs: []sdk.Msg{}, | ||
expErr: false, | ||
}, | ||
"Set nil message inside the slice of messages": { | ||
msgs: []sdk.Msg{nil}, | ||
expErr: true, | ||
}, | ||
"Set valid messages": { | ||
msgs: []sdk.Msg{&DummyProtoMessage1{Name: "name1"}, &DummyProtoMessage2{}}, | ||
msgTypeURL: []string{"/dummy.proto.message1", "/dummy.proto.message2"}, | ||
expErr: false, | ||
}, | ||
} | ||
|
||
for name, tc := range cases { | ||
t.Run(name, func(t *testing.T) { | ||
actual, err := tx.SetMsgs(tc.msgs) | ||
if tc.expErr { | ||
require.Error(t, err) | ||
return | ||
} | ||
require.NoError(t, err) | ||
require.Equal(t, len(tc.msgs), len(actual)) | ||
|
||
for i, msg := range tc.msgs { | ||
b, err := json.Marshal(msg) | ||
require.NoError(t, err) | ||
|
||
require.Equal(t, msg, actual[i].GetCachedValue()) | ||
require.Equal(t, tc.msgTypeURL[i], actual[i].GetTypeUrl()) | ||
require.Equal(t, b, actual[i].GetValue()) | ||
} | ||
}) | ||
} | ||
} | ||
|
||
func Test_GetMsgs(t *testing.T) { | ||
sdkMsgs := []sdk.Msg{&DummyProtoMessage1{}, &DummyProtoMessage2{}} | ||
anyMsg, err := tx.SetMsgs(sdkMsgs) | ||
require.NoError(t, err) | ||
|
||
cases := map[string]struct { | ||
msgs []*types.Any | ||
expected []sdk.Msg | ||
expErr bool | ||
}{ | ||
"GetMsgs from a nil slice of Any messages": { | ||
msgs: nil, | ||
expected: []sdk.Msg{}, | ||
expErr: false, | ||
}, | ||
"GetMsgs from empty slice of Any messages": { | ||
msgs: []*types.Any{}, | ||
expected: []sdk.Msg{}, | ||
expErr: false, | ||
}, | ||
"GetMsgs from a slice with valid Any messages": { | ||
msgs: anyMsg, | ||
expected: sdkMsgs, | ||
expErr: false, | ||
}, | ||
"GetMsgs from a slice that contains uncached Any message": { | ||
msgs: []*types.Any{{}}, | ||
expErr: true, | ||
}, | ||
} | ||
|
||
for name, tc := range cases { | ||
t.Run(name, func(t *testing.T) { | ||
actual, err := tx.GetMsgs(tc.msgs, "dummy") | ||
if tc.expErr { | ||
require.Error(t, err) | ||
return | ||
} | ||
|
||
require.NoError(t, err) | ||
require.Equal(t, tc.expected, actual) | ||
}) | ||
} | ||
} | ||
|
||
func TestTx_UnpackInterfaces(t *testing.T) { | ||
unpacker := codec.NewProtoCodec(types.NewInterfaceRegistry()) | ||
sdkMsgs := []sdk.Msg{&DummyProtoMessage1{}, &DummyProtoMessage2{}} | ||
anyMsg, err := tx.SetMsgs(sdkMsgs) | ||
require.NoError(t, err) | ||
|
||
cases := map[string]struct { | ||
msgs []*types.Any | ||
expErr bool | ||
}{ | ||
"Unpack nil slice messages": { | ||
msgs: nil, | ||
expErr: false, | ||
}, | ||
"Unpack empty slice of messages": { | ||
msgs: []*types.Any{}, | ||
expErr: false, | ||
}, | ||
"Unpack valid messages": { | ||
msgs: anyMsg, | ||
expErr: false, | ||
}, | ||
"Unpack uncashed message": { | ||
msgs: []*types.Any{{TypeUrl: "uncached"}}, | ||
expErr: true, | ||
}, | ||
} | ||
|
||
for name, tc := range cases { | ||
t.Run(name, func(t *testing.T) { | ||
err = tx.UnpackInterfaces(unpacker, tc.msgs) | ||
require.Equal(t, tc.expErr, err != nil) | ||
}) | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good test cases |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,8 @@ | ||
package tx | ||
package tx_test | ||
|
||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same as above There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. . |
||
import ( | ||
"encoding/hex" | ||
"encoding/json" | ||
"testing" | ||
|
||
"github.com/cosmos/gogoproto/proto" | ||
|
@@ -15,6 +16,7 @@ import ( | |
"github.com/cosmos/cosmos-sdk/codec/address" | ||
codectypes "github.com/cosmos/cosmos-sdk/codec/types" | ||
sdk "github.com/cosmos/cosmos-sdk/types" | ||
"github.com/cosmos/cosmos-sdk/types/tx" | ||
) | ||
|
||
func TestTx_GetMsgs(t *testing.T) { | ||
|
@@ -56,8 +58,8 @@ func TestTx_GetMsgs(t *testing.T) { | |
|
||
for _, tc := range cases { | ||
t.Run(tc.name, func(t *testing.T) { | ||
transaction := Tx{ | ||
Body: &TxBody{ | ||
transaction := tx.Tx{ | ||
Body: &tx.TxBody{ | ||
Messages: tc.msgs, | ||
Comment on lines
+61
to
+62
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider using |
||
}, | ||
} | ||
|
@@ -77,7 +79,7 @@ func TestTx_GetMsgs(t *testing.T) { | |
func TestTx_ValidateBasic(t *testing.T) { | ||
cases := []struct { | ||
name string | ||
transaction *Tx | ||
transaction *tx.Tx | ||
expErr bool | ||
}{ | ||
{ | ||
|
@@ -87,46 +89,46 @@ func TestTx_ValidateBasic(t *testing.T) { | |
}, | ||
{ | ||
name: "Tx without body", | ||
transaction: &Tx{}, | ||
transaction: &tx.Tx{}, | ||
expErr: true, | ||
}, | ||
{ | ||
name: "Tx without AuthInfo", | ||
transaction: &Tx{Body: &TxBody{}}, | ||
transaction: &tx.Tx{Body: &tx.TxBody{}}, | ||
expErr: true, | ||
}, | ||
{ | ||
name: "Tx without Fee", | ||
transaction: &Tx{Body: &TxBody{}, AuthInfo: &AuthInfo{}}, | ||
transaction: &tx.Tx{Body: &tx.TxBody{}, AuthInfo: &tx.AuthInfo{}}, | ||
expErr: true, | ||
}, | ||
{ | ||
name: "Tx with gas limit greater than Max gas wanted", | ||
transaction: &Tx{Body: &TxBody{}, AuthInfo: &AuthInfo{Fee: &Fee{GasLimit: MaxGasWanted + 1}}}, | ||
transaction: &tx.Tx{Body: &tx.TxBody{}, AuthInfo: &tx.AuthInfo{Fee: &tx.Fee{GasLimit: tx.MaxGasWanted + 1}}}, | ||
expErr: true, | ||
}, | ||
{ | ||
name: "Tx without Fee Amount", | ||
transaction: &Tx{Body: &TxBody{}, AuthInfo: &AuthInfo{Fee: &Fee{GasLimit: MaxGasWanted}}}, | ||
transaction: &tx.Tx{Body: &tx.TxBody{}, AuthInfo: &tx.AuthInfo{Fee: &tx.Fee{GasLimit: tx.MaxGasWanted}}}, | ||
expErr: true, | ||
}, | ||
{ | ||
name: "Tx with negative Fee Amount", | ||
transaction: &Tx{ | ||
Body: &TxBody{}, | ||
AuthInfo: &AuthInfo{ | ||
Fee: &Fee{GasLimit: MaxGasWanted, Amount: sdk.Coins{sdk.Coin{Amount: math.NewInt(-1)}}}, | ||
transaction: &tx.Tx{ | ||
Body: &tx.TxBody{}, | ||
AuthInfo: &tx.AuthInfo{ | ||
Fee: &tx.Fee{GasLimit: tx.MaxGasWanted, Amount: sdk.Coins{sdk.Coin{Amount: math.NewInt(-1)}}}, | ||
}, | ||
}, | ||
expErr: true, | ||
}, | ||
{ | ||
name: "Tx with invalid fee payer address", | ||
transaction: &Tx{ | ||
Body: &TxBody{}, | ||
AuthInfo: &AuthInfo{ | ||
Fee: &Fee{ | ||
GasLimit: MaxGasWanted, | ||
transaction: &tx.Tx{ | ||
Body: &tx.TxBody{}, | ||
AuthInfo: &tx.AuthInfo{ | ||
Fee: &tx.Fee{ | ||
GasLimit: tx.MaxGasWanted, | ||
Payer: "invalidPayerAddress", | ||
Amount: sdk.Coins{sdk.NewCoin("aaa", math.NewInt(10))}, | ||
}, | ||
|
@@ -136,11 +138,11 @@ func TestTx_ValidateBasic(t *testing.T) { | |
}, | ||
{ | ||
name: "Tx without signature", | ||
transaction: &Tx{ | ||
Body: &TxBody{}, | ||
AuthInfo: &AuthInfo{ | ||
Fee: &Fee{ | ||
GasLimit: MaxGasWanted, | ||
transaction: &tx.Tx{ | ||
Body: &tx.TxBody{}, | ||
AuthInfo: &tx.AuthInfo{ | ||
Fee: &tx.Fee{ | ||
GasLimit: tx.MaxGasWanted, | ||
Payer: "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs", | ||
Amount: sdk.Coins{sdk.NewCoin("aaa", math.NewInt(11))}, | ||
}, | ||
|
@@ -150,11 +152,11 @@ func TestTx_ValidateBasic(t *testing.T) { | |
}, | ||
{ | ||
name: "Tx is valid", | ||
transaction: &Tx{ | ||
Body: &TxBody{}, | ||
AuthInfo: &AuthInfo{ | ||
Fee: &Fee{ | ||
GasLimit: MaxGasWanted, | ||
transaction: &tx.Tx{ | ||
Body: &tx.TxBody{}, | ||
AuthInfo: &tx.AuthInfo{ | ||
Fee: &tx.Fee{ | ||
GasLimit: tx.MaxGasWanted, | ||
Payer: "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs", | ||
Amount: sdk.Coins{sdk.NewCoin("aaa", math.NewInt(11))}, | ||
}, | ||
|
@@ -174,11 +176,11 @@ func TestTx_ValidateBasic(t *testing.T) { | |
} | ||
|
||
func TestTx_GetSigners(t *testing.T) { | ||
transaction := &Tx{ | ||
Body: &TxBody{}, | ||
AuthInfo: &AuthInfo{ | ||
Fee: &Fee{ | ||
GasLimit: MaxGasWanted, | ||
transaction := &tx.Tx{ | ||
Body: &tx.TxBody{}, | ||
AuthInfo: &tx.AuthInfo{ | ||
Fee: &tx.Fee{ | ||
GasLimit: tx.MaxGasWanted, | ||
Payer: "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs", | ||
Comment on lines
+179
to
+183
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Consider adding more test cases to cover scenarios with multiple signers and invalid signer addresses to ensure comprehensive coverage of the |
||
Amount: sdk.Coins{sdk.NewCoin("aaa", math.NewInt(11))}, | ||
}, | ||
|
@@ -206,17 +208,23 @@ func TestTx_GetSigners(t *testing.T) { | |
require.Nil(t, err) | ||
} | ||
|
||
type DummyProtoMessage1 struct{} | ||
type DummyProtoMessage1 struct { | ||
Name string | ||
} | ||
|
||
func (d *DummyProtoMessage1) Reset() {} | ||
func (d *DummyProtoMessage1) String() string { return "/dummy.proto.message1" } | ||
func (d *DummyProtoMessage1) ProtoMessage() {} | ||
func (d *DummyProtoMessage1) Reset() {} | ||
func (d *DummyProtoMessage1) String() string { return "/dummy.proto.message1" } | ||
func (d *DummyProtoMessage1) ProtoMessage() {} | ||
func (d *DummyProtoMessage1) Marshal() ([]byte, error) { return json.Marshal(d) } | ||
func (d *DummyProtoMessage1) XXX_MessageName() string { return "dummy.proto.message1" } | ||
|
||
type DummyProtoMessage2 struct{} | ||
|
||
func (d *DummyProtoMessage2) Reset() {} | ||
func (d *DummyProtoMessage2) String() string { return "/dummy.proto.message2" } | ||
func (d *DummyProtoMessage2) ProtoMessage() {} | ||
func (d *DummyProtoMessage2) Reset() {} | ||
func (d *DummyProtoMessage2) String() string { return "/dummy.proto.message2" } | ||
func (d *DummyProtoMessage2) ProtoMessage() {} | ||
func (d *DummyProtoMessage2) Marshal() ([]byte, error) { return json.Marshal(d) } | ||
func (d *DummyProtoMessage2) XXX_MessageName() string { return "dummy.proto.message2" } | ||
|
||
type dummyAddressCodec struct{} | ||
|
||
|
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.