Skip to content
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

fix: add restrictions on the number of args in the CLIs #734

Merged
merged 6 commits into from
Oct 19, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (crypto) [\#731](https://github.com/line/lbm-sdk/pull/731) remove VRFProve function
* (x/foundation) [\#732](https://github.com/line/lbm-sdk/pull/732) add verification on accounts into x/foundation Grants cli
* (x/foundation) [\#730](https://github.com/line/lbm-sdk/pull/730) prune stale x/foundation proposals at voting period end
* (cli) [\#734](https://github.com/line/lbm-sdk/pull/734) add restrictions on the number of args in the CLIs

### Breaking Changes
* (proto) [\#564](https://github.com/line/lbm-sdk/pull/564) change gRPC path to original cosmos path
Expand Down
2 changes: 2 additions & 0 deletions x/auth/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ func GetAccountCmd() *cobra.Command {
func GetAccountsCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "accounts",
Args: cobra.NoArgs,
Short: "Query all the accounts",
RunE: func(cmd *cobra.Command, args []string) error {
clientCtx, err := client.GetClientQueryContext(cmd)
Expand Down Expand Up @@ -147,6 +148,7 @@ func GetAccountsCmd() *cobra.Command {
func QueryTxsByEventsCmd() *cobra.Command {
cmd := &cobra.Command{
Use: "txs",
Args: cobra.NoArgs,
Short: "Query for paginated transactions that match a set of events",
Long: strings.TrimSpace(
fmt.Sprintf(`
Expand Down
57 changes: 50 additions & 7 deletions x/auth/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,7 @@ func (s *IntegrationTestSuite) TestCLIQueryTxsCmdByEvents() {
testCases := []struct {
name string
args []string
expectErr bool
expectEmpty bool
}{
{
Expand All @@ -463,6 +464,7 @@ func (s *IntegrationTestSuite) TestCLIQueryTxsCmdByEvents() {
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
},
false,
false,
},
{
"no matching fee event",
Expand All @@ -471,6 +473,16 @@ func (s *IntegrationTestSuite) TestCLIQueryTxsCmdByEvents() {
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(0))).String()),
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
},
false,
true,
},
{
"wrong number of arguments",
[]string{
"extra",
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
},
true,
true,
},
}
Expand All @@ -482,6 +494,10 @@ func (s *IntegrationTestSuite) TestCLIQueryTxsCmdByEvents() {
clientCtx := val.ClientCtx

out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, tc.args)
if tc.expectErr {
s.Require().Error(err)
return
}
s.Require().NoError(err)

var result sdk.SearchTxsResult
Expand Down Expand Up @@ -1122,16 +1138,43 @@ func (s *IntegrationTestSuite) TestGetAccountCmd() {

func (s *IntegrationTestSuite) TestGetAccountsCmd() {
val := s.network.Validators[0]
clientCtx := val.ClientCtx

out, err := clitestutil.ExecTestCLICmd(clientCtx, authcli.GetAccountsCmd(), []string{
commonArgs := []string{
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
})
s.Require().NoError(err)
}

var res authtypes.QueryAccountsResponse
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &res))
s.Require().NotEmpty(res.Accounts)
testCases := map[string]struct {
args []string
valid bool
}{
"valid request": {
valid: true,
},
"wrong number of args": {
args: []string{
"extra",
},
},
}

for name, tc := range testCases {
tc := tc
s.Run(name, func() {
cmd := authcli.GetAccountsCmd()
clientCtx := val.ClientCtx

out, err := clitestutil.ExecTestCLICmd(clientCtx, cmd, append(tc.args, commonArgs...))
if !tc.valid {
s.Require().Error(err)
return
}
s.Require().NoError(err)

var res authtypes.QueryAccountsResponse
s.Require().NoError(val.ClientCtx.Codec.UnmarshalJSON(out.Bytes(), &res))
s.Require().NotEmpty(res.Accounts)
})
}
}

func TestGetBroadcastCommandOfflineFlag(t *testing.T) {
Expand Down
2 changes: 2 additions & 0 deletions x/bank/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ Example:
func GetCmdDenomsMetadata() *cobra.Command {
cmd := &cobra.Command{
Use: "denom-metadata",
Args: cobra.NoArgs,
0Tech marked this conversation as resolved.
Show resolved Hide resolved
Short: "Query the client metadata for coin denominations",
Long: strings.TrimSpace(
fmt.Sprintf(`Query the client metadata for all the registered coin denominations
Expand Down Expand Up @@ -158,6 +159,7 @@ To query for the client metadata of a specific coin denomination use:
func GetCmdQueryTotalSupply() *cobra.Command {
cmd := &cobra.Command{
Use: "total",
Args: cobra.NoArgs,
Short: "Query the total supply of coins of the chain",
Long: strings.TrimSpace(
fmt.Sprintf(`Query total supply of coins that are held by accounts in the chain.
Expand Down
24 changes: 24 additions & 0 deletions x/bank/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -214,6 +214,15 @@ func (s *IntegrationTestSuite) TestGetCmdQueryTotalSupply() {
Amount: sdk.ZeroInt(),
},
},
{
name: "wrong number of arguments",
args: []string{
"extra",
fmt.Sprintf("--%s=1", flags.FlagHeight),
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
},
expectErr: true,
},
}

for _, tc := range testCases {
Expand Down Expand Up @@ -341,6 +350,21 @@ func (s *IntegrationTestSuite) TestGetCmdQueryDenomsMetadata() {
},
},
},
{
name: "wrong number of arguments",
args: []string{
"extra",
fmt.Sprintf("--%s=1", flags.FlagHeight),
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
},
expectErr: true,
respType: &types.QueryDenomMetadataResponse{},
expected: &types.QueryDenomMetadataResponse{
Metadata: types.Metadata{
DenomUnits: []*types.DenomUnit{},
},
},
},
}

for _, tc := range testCases {
Expand Down
1 change: 1 addition & 0 deletions x/gov/client/cli/query.go
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ $ %s query gov proposal 1
func GetCmdQueryProposals() *cobra.Command {
cmd := &cobra.Command{
Use: "proposals",
Args: cobra.NoArgs,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you know why this line is not covered?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the current testing structure for the CLIs, codecov cannot detect that the code is covered. So I cannot increase the coverage right now. But, actually, the tests DOES cover the CLIs.

Short: "Query proposals with optional filters",
Long: strings.TrimSpace(
fmt.Sprintf(`Query for a all paginated proposals that match optional filters:
Expand Down
8 changes: 8 additions & 0 deletions x/gov/client/testutil/suite.go
Original file line number Diff line number Diff line change
Expand Up @@ -435,6 +435,14 @@ func (s *IntegrationTestSuite) TestCmdGetProposals() {
},
true,
},
{
"wrong number of arguments",
[]string{
"extra",
fmt.Sprintf("--%s=json", ostcli.OutputFlag),
},
true,
},
}

for _, tc := range testCases {
Expand Down