test(devgas-cli): CLI tests for devgas txs #1728
Conversation
|
Warning Rate Limit Exceeded@Unique-Divine has exceeded the limit for the number of files or commits that can be reviewed per hour. Please wait 6 minutes and 57 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the @coderabbitai review command. Alternatively, push new commits to this PR. We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. WalkthroughThe latest update introduces a suite of CLI tests for the Changes
Assessment against linked issues
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChat with CodeRabbit Bot (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 0
Configuration used: CodeRabbit UI
Files selected for processing (2)
- CHANGELOG.md (1 hunks)
- x/devgas/v1/client/cli/cli_test.go (1 hunks)
Additional comments: 8
x/devgas/v1/client/cli/cli_test.go (8)
1-26: The import section includes packages from both the Cosmos SDK and the CometBFT, which suggests that the project is using a fork of the Tendermint consensus engine. It's important to ensure that the CometBFT fork is compatible with the Cosmos SDK version used in the project.
28-42: The
CLITestSuiteis well-structured, providing a clear setup for CLI tests. It includes aSetupSuitemethod to initialize the necessary components for the tests, which is a good practice for maintainability and reusability.71-78: The
commonTxArgsfunction provides a centralized way to define common flags for CLI commands, which is a good practice for consistency and reduces the risk of errors when constructing CLI commands in tests.81-90: The
TestCasestruct and its methodNewCtxare well-designed, encapsulating the data needed for a test case and providing a method to generate a new context. This design promotes test readability and maintainability.92-110: The
Runmethod in theTestCasestruct is a good example of encapsulating the execution logic of a test case. It uses thes.Runmethod from thesuitepackage to run the subtests, which is a standard practice in Go for structuring table-driven tests.112-152: The
TestCmdRegisterFeeSharefunction tests theregistercommand with various scenarios, including both happy and sad paths. The use of table-driven tests to cover different cases is a good practice for thorough testing.154-185: The
TestCmdCancelFeeSharefunction follows the same pattern asTestCmdRegisterFeeShare, testing thecancelcommand with different inputs. The consistency in test structure across different commands is good for maintainability.187-227: The
TestCmdUpdateFeeSharefunction tests theupdatecommand, ensuring that the command behaves correctly for various inputs. The structure and approach are consistent with the other test functions, which is good for code readability.
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #1728 +/- ##
==========================================
- Coverage 73.89% 73.45% -0.44%
==========================================
Files 192 195 +3
Lines 15142 15451 +309
==========================================
+ Hits 11189 11350 +161
- Misses 3297 3439 +142
- Partials 656 662 +6
|
There was a problem hiding this comment.
Review Status
Actionable comments generated: 2
Configuration used: CodeRabbit UI
Files selected for processing (1)
- x/devgas/v1/client/cli/cli_test.go (1 hunks)
Additional comments: 5
x/devgas/v1/client/cli/cli_test.go (5)
28-38: The
CLITestSuitestruct is well-defined with necessary fields for testing. It includes a keyring for account management, encoding configurations, and contexts for client and base operations, as well as a test account.71-78: The
commonTxArgsfunction provides a clear and concise way to define common transaction arguments. This is a good use of a helper function to avoid repetition in test cases.113-153: The
TestCmdRegisterFeeSharemethod defines test cases for theregistercommand. The test cases are well-structured, and the use of table-driven tests is a good practice. Ensure that the error messages are specific enough to catch the intended errors.Verification successful
The shell scripts intended to verify the specificity and accuracy of the error messages in the test cases did not produce any output. This means that the error messages such as 'decoding bech32 failed', 'invalid contract address', and 'invalid withdraw address' were not found elsewhere in the codebase, or the pattern used for
ast-grepdid not match any code.Given this result, it's not possible to confirm whether the error messages used in the test cases are consistent with the actual error messages produced by the code under test. To proceed, we can attempt a different strategy to verify the error messages by looking for the functions that might produce these errors and checking if the error messages align with the test expectations.
Let's generate a new script to search for functions that could be related to the error messages mentioned in the test cases.
The new context from the shell scripts provides instances where the error messages 'decoding bech32 failed', 'invalid contract address', and 'invalid withdraw address' are used in the codebase. These instances are found in test files and in the actual implementation code, which suggests that the error messages in the
TestCmdRegisterFeeSharemethod are consistent with the error handling in the rest of the codebase.Here are the findings:
- The error message "decoding bech32 failed" is used in various test cases across the codebase, indicating that it is a standard error message for bech32 decoding issues.
- The error message "invalid contract address" is used in the
msg_server.goandmsg.gofiles when there is an error parsing the contract address from a bech32 string, which aligns with the test case expectation.- The error message "invalid withdraw address" is used in the
query.goandmsg.gofiles when there is an error parsing the withdraw address from a bech32 string, which also aligns with the test case expectation.Based on this information, we can conclude that the error messages used in the test cases are specific and accurate, reflecting the actual error messages that would be produced by the code under test.
* 155-186: The `TestCmdCancelFeeShare` method is similar to `TestCmdRegisterFeeShare` and follows the same pattern for defining test cases. The structure and approach are consistent with the previous test method.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the error messages used in the test cases are specific and accurate. rg --type go --files-with-matches 'decoding bech32 failed' | xargs -L1 ast-grep --lang go --pattern '$_("$ERROR_MESSAGE")' rg --type go --files-with-matches 'invalid contract address' | xargs -L1 ast-grep --lang go --pattern '$_("$ERROR_MESSAGE")' rg --type go --files-with-matches 'invalid withdraw address' | xargs -L1 ast-grep --lang go --pattern '$_("$ERROR_MESSAGE")'Length of output: 378
Script:
#!/bin/bash # Search for functions that might produce the error messages mentioned in the test cases. rg --type go 'decoding bech32 failed' -C 3 rg --type go 'invalid contract address' -C 3 rg --type go 'invalid withdraw address' -C 3Length of output: 12962
- 188-228: The
TestCmdUpdateFeeSharemethod also uses table-driven tests for theupdatecommand. The test cases are consistent with the other test methods, and the approach is correct. As with the other test methods, ensure that the error messages are specific and meaningful.
| // Runs once before the entire test suite. | ||
| func (s *CLITestSuite) SetupSuite() { | ||
| s.encCfg = testutilmod.MakeTestEncodingConfig(devgas.AppModuleBasic{}) | ||
| s.keyring = keyring.NewInMemory(s.encCfg.Codec) | ||
| s.baseCtx = sdkclient.Context{}. | ||
| WithKeyring(s.keyring). | ||
| WithTxConfig(s.encCfg.TxConfig). | ||
| WithCodec(s.encCfg.Codec). | ||
| WithClient(sdktestutilcli.MockTendermintRPC{Client: rpcclientmock.Client{}}). | ||
| WithAccountRetriever(sdkclient.MockAccountRetriever{}). | ||
| WithOutput(io.Discard). | ||
| WithChainID("test-chain") | ||
|
|
||
| var outBuf bytes.Buffer | ||
| ctxGen := func() sdkclient.Context { | ||
| bz, _ := s.encCfg.Codec.Marshal(&sdk.TxResponse{}) | ||
| c := sdktestutilcli.NewMockTendermintRPC(abci.ResponseQuery{ | ||
| Value: bz, | ||
| }) | ||
| return s.baseCtx.WithClient(c) | ||
| } | ||
| s.clientCtx = ctxGen().WithOutput(&outBuf) | ||
|
|
||
| testAccs := sdktestutil.CreateKeyringAccounts(s.T(), s.keyring, 1) | ||
| s.testAcc = testAccs[0] | ||
| } |
There was a problem hiding this comment.
The SetupSuite method correctly sets up the encoding configuration, keyring, base context, and client context. It also initializes a test account. However, the error from Marshal is ignored, which could lead to silent failures if marshaling fails.
- bz, _ := s.encCfg.Codec.Marshal(&sdk.TxResponse{})
+ bz, err := s.encCfg.Codec.Marshal(&sdk.TxResponse{})
+ s.Require().NoError(err, "marshaling TxResponse should not fail")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| // Runs once before the entire test suite. | |
| func (s *CLITestSuite) SetupSuite() { | |
| s.encCfg = testutilmod.MakeTestEncodingConfig(devgas.AppModuleBasic{}) | |
| s.keyring = keyring.NewInMemory(s.encCfg.Codec) | |
| s.baseCtx = sdkclient.Context{}. | |
| WithKeyring(s.keyring). | |
| WithTxConfig(s.encCfg.TxConfig). | |
| WithCodec(s.encCfg.Codec). | |
| WithClient(sdktestutilcli.MockTendermintRPC{Client: rpcclientmock.Client{}}). | |
| WithAccountRetriever(sdkclient.MockAccountRetriever{}). | |
| WithOutput(io.Discard). | |
| WithChainID("test-chain") | |
| var outBuf bytes.Buffer | |
| ctxGen := func() sdkclient.Context { | |
| bz, _ := s.encCfg.Codec.Marshal(&sdk.TxResponse{}) | |
| c := sdktestutilcli.NewMockTendermintRPC(abci.ResponseQuery{ | |
| Value: bz, | |
| }) | |
| return s.baseCtx.WithClient(c) | |
| } | |
| s.clientCtx = ctxGen().WithOutput(&outBuf) | |
| testAccs := sdktestutil.CreateKeyringAccounts(s.T(), s.keyring, 1) | |
| s.testAcc = testAccs[0] | |
| } | |
| // Runs once before the entire test suite. | |
| func (s *CLITestSuite) SetupSuite() { | |
| s.encCfg = testutilmod.MakeTestEncodingConfig(devgas.AppModuleBasic{}) | |
| s.keyring = keyring.NewInMemory(s.encCfg.Codec) | |
| s.baseCtx = sdkclient.Context{}. | |
| WithKeyring(s.keyring). | |
| WithTxConfig(s.encCfg.TxConfig). | |
| WithCodec(s.encCfg.Codec). | |
| WithClient(sdktestutilcli.MockTendermintRPC{Client: rpcclientmock.Client{}}). | |
| WithAccountRetriever(sdkclient.MockAccountRetriever{}). | |
| WithOutput(io.Discard). | |
| WithChainID("test-chain") | |
| var outBuf bytes.Buffer | |
| ctxGen := func() sdkclient.Context { | |
| bz, err := s.encCfg.Codec.Marshal(&sdk.TxResponse{}) | |
| s.Require().NoError(err, "marshaling TxResponse should not fail") | |
| c := sdktestutilcli.NewMockTendermintRPC(abci.ResponseQuery{ | |
| Value: bz, | |
| }) | |
| return s.baseCtx.WithClient(c) | |
| } | |
| s.clientCtx = ctxGen().WithOutput(&outBuf) | |
| testAccs := sdktestutil.CreateKeyringAccounts(s.T(), s.keyring, 1) | |
| s.testAcc = testAccs[0] | |
| } |
| type TestCase struct { | ||
| name string | ||
| args []string | ||
| extraArgs []string | ||
| wantErr string | ||
| } | ||
|
|
||
| func (tc TestCase) NewCtx(s *CLITestSuite) sdkclient.Context { | ||
| return s.baseCtx | ||
| } | ||
|
|
||
| func (tc TestCase) Run(s *CLITestSuite) { | ||
| s.Run(tc.name, func() { | ||
| ctx := svrcmd.CreateExecuteContext(context.Background()) | ||
|
|
||
| cmd := cli.NewTxCmd() | ||
| cmd.SetContext(ctx) | ||
| args := append(tc.args, commonTxArgs()...) | ||
| cmd.SetArgs(append(args, tc.extraArgs...)) | ||
|
|
||
| s.Require().NoError(sdkclient.SetCmdClientContextHandler(tc.NewCtx(s), cmd)) | ||
|
|
||
| err := cmd.Execute() | ||
| if tc.wantErr != "" { | ||
| s.Require().Error(err) | ||
| s.ErrorContains(err, tc.wantErr) | ||
| return | ||
| } | ||
| s.Require().NoError(err) | ||
| }) | ||
| } |
There was a problem hiding this comment.
The TestCase struct and its associated methods are well-structured for defining and running test cases. The Run method properly encapsulates the test logic within a named subtest. However, the error message check in line 106 should be refined to ensure that it is not too broad and accurately captures the expected error.
- s.ErrorContains(err, tc.wantErr)
+ s.Require().ErrorContains(err, tc.wantErr, "the error message should contain the expected text")Committable suggestion
❗ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| type TestCase struct { | |
| name string | |
| args []string | |
| extraArgs []string | |
| wantErr string | |
| } | |
| func (tc TestCase) NewCtx(s *CLITestSuite) sdkclient.Context { | |
| return s.baseCtx | |
| } | |
| func (tc TestCase) Run(s *CLITestSuite) { | |
| s.Run(tc.name, func() { | |
| ctx := svrcmd.CreateExecuteContext(context.Background()) | |
| cmd := cli.NewTxCmd() | |
| cmd.SetContext(ctx) | |
| args := append(tc.args, commonTxArgs()...) | |
| cmd.SetArgs(append(args, tc.extraArgs...)) | |
| s.Require().NoError(sdkclient.SetCmdClientContextHandler(tc.NewCtx(s), cmd)) | |
| err := cmd.Execute() | |
| if tc.wantErr != "" { | |
| s.Require().Error(err) | |
| s.ErrorContains(err, tc.wantErr) | |
| return | |
| } | |
| s.Require().NoError(err) | |
| }) | |
| } | |
| type TestCase struct { | |
| name string | |
| args []string | |
| extraArgs []string | |
| wantErr string | |
| } | |
| func (tc TestCase) NewCtx(s *CLITestSuite) sdkclient.Context { | |
| return s.baseCtx | |
| } | |
| func (tc TestCase) Run(s *CLITestSuite) { | |
| s.Run(tc.name, func() { | |
| ctx := svrcmd.CreateExecuteContext(context.Background()) | |
| cmd := cli.NewTxCmd() | |
| cmd.SetContext(ctx) | |
| args := append(tc.args, commonTxArgs()...) | |
| cmd.SetArgs(append(args, tc.extraArgs...)) | |
| s.Require().NoError(sdkclient.SetCmdClientContextHandler(tc.NewCtx(s), cmd)) | |
| err := cmd.Execute() | |
| if tc.wantErr != "" { | |
| s.Require().Error(err) | |
| s.Require().ErrorContains(err, tc.wantErr, "the error message should contain the expected text") | |
| return | |
| } | |
| s.Require().NoError(err) | |
| }) | |
| } |
Purpose / Abstract
Summary by CodeRabbit
devgasmodule, enhancing reliability and functionality.