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

Improve CLI tests assertions #1558

Open
4 tasks
nullpointer0x00 opened this issue May 23, 2023 · 1 comment
Open
4 tasks

Improve CLI tests assertions #1558

nullpointer0x00 opened this issue May 23, 2023 · 1 comment
Labels
CLI Command line interface feature
Milestone

Comments

@nullpointer0x00
Copy link
Contributor

Summary

The cli tests assertions can be improved. We have this pattern all throughout our code and would be nice to have it all cleaned up at once to have consistency and a proper example to base new tests off of.

A few things:

testCases := []struct {
		name         string
		cmd          *cobra.Command
		args         []string
		expectErr    bool
		respType     proto.Message
		expectedCode uint32
	}

....

		s.Run(tc.name, func() {
			clientCtx := s.testnet.Validators[0].ClientCtx
			out, err := clitestutil.ExecTestCLICmd(clientCtx, tc.cmd, tc.args)

			if tc.expectErr {
				s.Require().Error(err)
			} else {
				s.Require().NoError(err)
				s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType), out.String())
				txResp := tc.respType.(*sdk.TxResponse)
				s.Require().Equal(tc.expectedCode, txResp.Code)
			}
		})
	}

1.) We have an expectedErr bool. This should be a string and and checked with EqualError call.
2.) Don't need the respType property, because the response is always sdk.TxResponse
3.) s.Require().NoError(clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType), out.String()) Fix output message.
4.) Change expected code checks to int32 instead of uint32 and add output message.

Problem Definition

Proposal


For Admin Use

  • Not duplicate issue
  • Appropriate labels applied
  • Appropriate contributors tagged
  • Contributor assigned/self-assigned
@iramiller iramiller added this to the v1.17.0 milestone Jun 1, 2023
@SpicyLemon SpicyLemon modified the milestones: v1.17.0, backlog Sep 6, 2023
@iramiller iramiller added the CLI Command line interface feature label Sep 6, 2023
@SpicyLemon
Copy link
Contributor

A couple other notes here:

  1. The out variable there (1st returned variable from ExecTestCLICmd) is a buffer, and calling either Bytes() or String() on it might clear it out so that the next time one of those are called, it returns empty. So it's better to call one of those and store it in a variable for use down the road. It's an easy cast to go between the two string and []byte types later.
  2. We now have the TxExecutor (defined in testutil/cli/exec.go) that we should consider switching to for these tests.

@SpicyLemon SpicyLemon moved this from Todo to Backlog in Provenance Core Protocol Team Nov 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLI Command line interface feature
Projects
Development

No branches or pull requests

3 participants