cli

cli copied to clipboard

Is your feature request related to a problem? Please describe. The current testdata used to test protoanalysis methods is the entire liquidity module proto sources. Any specific reason to have this test data? The sources are big and it makes it difficult to maintain the tests when a new feature must be added. For example, the format of the Parse method is modified

Describe the solution you'd like We should simplify a lot this testdata and still make it reliable for tests. For example, keeping only 3 txs, 3 queries and 3 messages in liquidity.proto should be already sufficient to test Parse method effectively.

The liquidity proto files are written in a different way and we needed to make sure that code gen works with this style of writing proto files. Right now we can basically clone gaia and generate code from there.

I agree on Denis, I think it's pretty important to keep a full test that checks everything for complex modules. Otherwise, we'll be basically lowering the test coverage.

We should detect what are all the edge cases in proto to handle them and have 100% coverage.

liquidity.proto has 10 messages with many many fields. Those messages are most of them very similar in some ways. The current assertion for messages for Parse is

Messages: []Message{
	{Name: "PoolRecord", Path: "testdata/liquidity/genesis.proto"},
	{Name: "GenesisState", Path: "testdata/liquidity/genesis.proto"},
	{Name: "PoolType", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "Params", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "Pool", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "PoolMetadata", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "PoolMetadataResponse", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "PoolBatch", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "PoolBatchResponse", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "DepositMsgState", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "WithdrawMsgState", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "SwapMsgState", Path: "testdata/liquidity/liquidity.proto"},
	{Name: "QueryLiquidityPoolRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryLiquidityPoolResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryLiquidityPoolBatchRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryLiquidityPoolBatchResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryLiquidityPoolsRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryLiquidityPoolsResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryParamsRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryParamsResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchSwapMsgsRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchSwapMsgRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchSwapMsgsResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchSwapMsgResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchDepositMsgsRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchDepositMsgRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchDepositMsgsResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchDepositMsgResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchWithdrawMsgsRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchWithdrawMsgRequest", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchWithdrawMsgsResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "QueryPoolBatchWithdrawMsgResponse", Path: "testdata/liquidity/query.proto"},
	{Name: "MsgCreatePool", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgCreatePoolRequest", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgCreatePoolResponse", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgDepositWithinBatch", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgDepositWithinBatchRequest", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgDepositWithinBatchResponse", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgWithdrawWithinBatch", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgWithdrawWithinBatchRequest", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgWithdrawWithinBatchResponse", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgSwapWithinBatch", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgSwapWithinBatchRequest", Path: "testdata/liquidity/tx.proto"},
	{Name: "MsgSwapWithinBatchResponse", Path: "testdata/liquidity/tx.proto"},
	{Name: "BaseReq", Path: "testdata/liquidity/tx.proto"},
	{Name: "Fee", Path: "testdata/liquidity/tx.proto"},
	{Name: "PubKey", Path: "testdata/liquidity/tx.proto"},
	{Name: "Signature", Path: "testdata/liquidity/tx.proto"},
	{Name: "StdTx", Path: "testdata/liquidity/tx.proto"},
},

It's unnecessarily too complex to maintain if we want to extend Parse like parsing fields for messages for example

Yes, that also makes sense as long as we cover all the edge cases in liquidity.

I think we can wait for now, it's not 100% sure we will need to extend those capabilities but it could be if we want to move to scaffolder that doesn't depend on any placeholder in proto

Testdata