test: remove some deps from testutil/network #20843
Conversation
WalkthroughThe changes involve refactoring test configurations and logic in the Cosmos SDK project. This includes modifying test setup configurations, refactoring test functions to standalone functions with updated imports and error handling, and restructuring dependencies to better isolate modules. Additionally, several Go module files were updated to remove or consolidate dependencies, reflecting a broader effort to decouple modules and reduce interdependencies. Changes
Possibly Related 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? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (5)
- client/grpc/cmtservice/status_test.go (2 hunks)
- client/rpc/rpc_test.go (2 hunks)
- simapp/app_test.go (3 hunks)
- tests/integration/server/grpc/server_test.go (2 hunks)
- testutil/network/network.go (3 hunks)
Files skipped from review due to trivial changes (1)
- testutil/network/network.go
Additional context used
Path-based instructions (4)
client/grpc/cmtservice/status_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"client/rpc/rpc_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"tests/integration/server/grpc/server_test.go (3)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"simapp/app_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
golangci-lint
client/grpc/cmtservice/status_test.go
7-7: File is not
gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order(gci)
9-9: File is not
gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order(gci)
Additional comments not posted (4)
client/grpc/cmtservice/status_test.go (1)
18-18: Verify the usage ofdepinject.Configs().Ensure that
depinject.Configs()is correctly configured and used in theDefaultConfigWithAppConfigfunction.Verification successful
The usage of
depinject.Configs()is correctly configured and utilized within theDefaultConfigWithAppConfigfunction.
- The
appConfigparameter is appropriately passed todepinject.Configs()for dependency injection.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage and configuration of `depinject.Configs()` in the `DefaultConfigWithAppConfig` function. # Test: Search for the function definition and usage of `depinject.Configs()`. Expect: Correct configuration and usage. rg --type go 'depinject.Configs'Length of output: 5742
Script:
#!/bin/bash # Description: Locate the definition of the `DefaultConfigWithAppConfig` function and inspect its implementation. # Find the file containing the `DefaultConfigWithAppConfig` function definition. rg --type go 'func DefaultConfigWithAppConfig' -l | xargs -I {} sh -c 'echo "File: {}"; rg --type go "func DefaultConfigWithAppConfig" {}'Length of output: 254
Script:
#!/bin/bash # Description: Extract the implementation of the `DefaultConfigWithAppConfig` function from the `testutil/network/network.go` file. # Extract the function implementation. rg --type go -A 30 'func DefaultConfigWithAppConfig' testutil/network/network.goLength of output: 890
client/rpc/rpc_test.go (1)
20-33: LGTM! Verify the changes in the test functions.The changes improve the test structure by using standalone functions and
requireassertions for better error handling and readability.However, ensure that the refactored test functions are correctly integrated and executed.
tests/integration/server/grpc/server_test.go (1)
51-59: LGTM! Verify the new imports and their usage.The new imports are added to set up the test configuration using various modules. Ensure that the imports are correctly used in the test setup.
However, verify the usage of the new imports in the test configuration.
Verification successful
Verified: The new imports are correctly used in the test configuration.
The imports for
configurator.AccountsModule,configurator.AuthModule,configurator.BankModule,configurator.GenutilModule,configurator.StakingModule,configurator.ConsensusModule, andconfigurator.TxModuleare correctly utilized in the test setup.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new imports in the test configuration. # Test: Search for the usage of new imports. Expect: Correct usage in the test setup. rg --type go 'configurator.NewAppConfig'Length of output: 2298
Script:
#!/bin/bash # Description: Verify the imports of various modules in the tests/integration/server/grpc/server_test.go file. # Test: Search for the import statements of the modules used in configurator.NewAppConfig. rg --type go 'configurator\.(AccountsModule|AuthModule|BankModule|GenutilModule|StakingModule|ConsensusModule|TxModule)' tests/integration/server/grpc/server_test.goLength of output: 383
simapp/app_test.go (1)
321-323: LGTM! Verify the removal of unnecessary dependencies.The import of
github.com/cosmos/cosmos-sdk/testutil/networkis removed and calls tonetwork.MinimumAppConfig()are replaced withAppConfig(). Ensure that the removal does not affect the test functionality.However, verify the replacement of
network.MinimumAppConfig()withAppConfig().Verification successful
Verification successful
The replacement of
network.MinimumAppConfig()withAppConfig()has been verified successfully. No occurrences ofnetwork.MinimumAppConfig()were found in the codebase.
network.MinimumAppConfig()has been fully removed and replaced byAppConfig().Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the replacement of `network.MinimumAppConfig()` with `AppConfig()`. # Test: Search for the usage of `AppConfig()`. Expect: Correct replacement in the test functions. rg --type go 'AppConfig'Length of output: 9170
Script:
#!/bin/bash # Description: Verify the removal of `network.MinimumAppConfig()` from the codebase. # Test: Search for the usage of `network.MinimumAppConfig()`. Expect: No occurrences in the codebase. rg --type go 'network.MinimumAppConfig'Length of output: 39
| @@ -4,6 +4,7 @@ import ( | |||
| "fmt" | |||
| "testing" | |||
|
|
|||
| "cosmossdk.io/depinject" | |||
There was a problem hiding this comment.
Sort the import statements.
The import statements should be sorted according to the gci tool with the specified options.
-import (
+import (
+ "fmt"
+ "testing"
+
+ "cosmossdk.io/depinject"
+ "github.com/stretchr/testify/require"
+
+ "github.com/cosmos/cosmos-sdk/server"
+ clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
+ "github.com/cosmos/cosmos-sdk/testutil/network"
+)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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "cosmossdk.io/depinject" | |
| import ( | |
| "fmt" | |
| "testing" | |
| "cosmossdk.io/depinject" | |
| "github.com/stretchr/testify/require" | |
| "github.com/cosmos/cosmos-sdk/server" | |
| clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" | |
| "github.com/cosmos/cosmos-sdk/testutil/network" | |
| ) |
Tools
golangci-lint
7-7: File is not
gci-ed with --skip-generated -s standard -s default -s prefix(cosmossdk.io) -s prefix(github.com/cosmos/cosmos-sdk) --custom-order(gci)
| @@ -6,59 +6,34 @@ import ( | |||
| "testing" | |||
|
|
|||
| abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" | |||
| "github.com/stretchr/testify/suite" | |||
| "github.com/stretchr/testify/require" | |||
There was a problem hiding this comment.
Sort the import statements.
The import statements should be sorted according to the gci tool with the specified options.
-import (
+import (
+ "context"
+ "strconv"
+ "testing"
+
+ abci "github.com/cometbft/cometbft/api/cometbft/abci/v1"
+ "github.com/stretchr/testify/require"
+ "google.golang.org/grpc"
+ "google.golang.org/grpc/metadata"
+
+ "github.com/cosmos/cosmos-sdk/client"
+ clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli"
+ "github.com/cosmos/cosmos-sdk/testutil/testdata"
+ "github.com/cosmos/cosmos-sdk/types/address"
+ grpctypes "github.com/cosmos/cosmos-sdk/types/grpc"
+)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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| "github.com/stretchr/testify/require" | |
| import ( | |
| "context" | |
| "strconv" | |
| "testing" | |
| abci "github.com/cometbft/cometbft/api/cometbft/abci/v1" | |
| "github.com/stretchr/testify/require" | |
| "google.golang.org/grpc" | |
| "google.golang.org/grpc/metadata" | |
| "github.com/cosmos/cosmos-sdk/client" | |
| clitestutil "github.com/cosmos/cosmos-sdk/testutil/cli" | |
| "github.com/cosmos/cosmos-sdk/testutil/testdata" | |
| "github.com/cosmos/cosmos-sdk/types/address" | |
| grpctypes "github.com/cosmos/cosmos-sdk/types/grpc" | |
| ) |
| _ "cosmossdk.io/x/accounts" | ||
| _ "cosmossdk.io/x/auth" | ||
| authclient "cosmossdk.io/x/auth/client" | ||
| _ "cosmossdk.io/x/auth/tx/config" | ||
| _ "cosmossdk.io/x/bank" | ||
| banktypes "cosmossdk.io/x/bank/types" | ||
| _ "cosmossdk.io/x/consensus" | ||
| _ "cosmossdk.io/x/staking" |
There was a problem hiding this comment.
Sort the import statements.
The import statements should be sorted according to the gci tool with the specified options.
-import (
+import (
+ "context"
+ "fmt"
+ "testing"
+ "time"
+
+ "github.com/jhump/protoreflect/grpcreflect"
+ "github.com/stretchr/testify/require"
+ "github.com/stretchr/testify/suite"
+ "google.golang.org/grpc"
+ "google.golang.org/grpc/metadata"
+
+ _ "cosmossdk.io/x/accounts"
+ _ "cosmossdk.io/x/auth"
+ authclient "cosmossdk.io/x/auth/client"
+ _ "cosmossdk.io/x/auth/tx/config"
+ _ "cosmossdk.io/x/bank"
+ banktypes "cosmossdk.io/x/bank/types"
+ _ "cosmossdk.io/x/consensus"
+ _ "cosmossdk.io/x/staking"
+ stakingtypes "cosmossdk.io/x/staking/types"
+
+ "github.com/cosmos/cosmos-sdk/client"
+ reflectionv1 "github.com/cosmos/cosmos-sdk/client/grpc/reflection"
+ clienttx "github.com/cosmos/cosmos-sdk/client/tx"
+ "github.com/cosmos/cosmos-sdk/codec"
+ reflectionv2 "github.com/cosmos/cosmos-sdk/server/grpc/reflection/v2alpha1"
+ "github.com/cosmos/cosmos-sdk/testutil/configurator"
+ "github.com/cosmos/cosmos-sdk/testutil/network"
+ "github.com/cosmos/cosmos-sdk/testutil/testdata"
+ sdk "github.com/cosmos/cosmos-sdk/types"
+ grpctypes "github.com/cosmos/cosmos-sdk/types/grpc"
+ txtypes "github.com/cosmos/cosmos-sdk/types/tx"
+ "github.com/cosmos/cosmos-sdk/types/tx/signing"
+)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. Thoroughly test & benchmark the code to ensure it meets the requirements.
| _ "cosmossdk.io/x/accounts" | |
| _ "cosmossdk.io/x/auth" | |
| authclient "cosmossdk.io/x/auth/client" | |
| _ "cosmossdk.io/x/auth/tx/config" | |
| _ "cosmossdk.io/x/bank" | |
| banktypes "cosmossdk.io/x/bank/types" | |
| _ "cosmossdk.io/x/consensus" | |
| _ "cosmossdk.io/x/staking" | |
| import ( | |
| "context" | |
| "fmt" | |
| "testing" | |
| "time" | |
| "github.com/jhump/protoreflect/grpcreflect" | |
| "github.com/stretchr/testify/require" | |
| "github.com/stretchr/testify/suite" | |
| "google.golang.org/grpc" | |
| "google.golang.org/grpc/metadata" | |
| _ "cosmossdk.io/x/accounts" | |
| _ "cosmossdk.io/x/auth" | |
| authclient "cosmossdk.io/x/auth/client" | |
| _ "cosmossdk.io/x/auth/tx/config" | |
| _ "cosmossdk.io/x/bank" | |
| banktypes "cosmossdk.io/x/bank/types" | |
| _ "cosmossdk.io/x/consensus" | |
| _ "cosmossdk.io/x/staking" | |
| stakingtypes "cosmossdk.io/x/staking/types" | |
| "github.com/cosmos/cosmos-sdk/client" | |
| reflectionv1 "github.com/cosmos/cosmos-sdk/client/grpc/reflection" | |
| clienttx "github.com/cosmos/cosmos-sdk/client/tx" | |
| "github.com/cosmos/cosmos-sdk/codec" | |
| reflectionv2 "github.com/cosmos/cosmos-sdk/server/grpc/reflection/v2alpha1" | |
| "github.com/cosmos/cosmos-sdk/testutil/configurator" | |
| "github.com/cosmos/cosmos-sdk/testutil/network" | |
| "github.com/cosmos/cosmos-sdk/testutil/testdata" | |
| sdk "github.com/cosmos/cosmos-sdk/types" | |
| grpctypes "github.com/cosmos/cosmos-sdk/types/grpc" | |
| txtypes "github.com/cosmos/cosmos-sdk/types/tx" | |
| "github.com/cosmos/cosmos-sdk/types/tx/signing" | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (20)
client/v2/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumserver/v2/cometbft/go.sumis excluded by!**/*.sumx/auth/go.sumis excluded by!**/*.sumx/authz/go.sumis excluded by!**/*.sumx/bank/go.sumis excluded by!**/*.sumx/circuit/go.sumis excluded by!**/*.sumx/consensus/go.sumis excluded by!**/*.sumx/distribution/go.sumis excluded by!**/*.sumx/epochs/go.sumis excluded by!**/*.sumx/evidence/go.sumis excluded by!**/*.sumx/feegrant/go.sumis excluded by!**/*.sumx/gov/go.sumis excluded by!**/*.sumx/mint/go.sumis excluded by!**/*.sumx/nft/go.sumis excluded by!**/*.sumx/params/go.sumis excluded by!**/*.sumx/protocolpool/go.sumis excluded by!**/*.sumx/slashing/go.sumis excluded by!**/*.sumx/staking/go.sumis excluded by!**/*.sumx/upgrade/go.sumis excluded by!**/*.sum
Files selected for processing (8)
- client/grpc/cmtservice/status_test.go (2 hunks)
- go.mod (2 hunks)
- x/auth/go.mod (1 hunks)
- x/authz/go.mod (2 hunks)
- x/bank/go.mod (2 hunks)
- x/feegrant/go.mod (2 hunks)
- x/gov/go.mod (2 hunks)
- x/staking/go.mod (2 hunks)
Files skipped from review due to trivial changes (7)
- go.mod
- x/auth/go.mod
- x/authz/go.mod
- x/bank/go.mod
- x/feegrant/go.mod
- x/gov/go.mod
- x/staking/go.mod
Files skipped from review as they are similar to previous changes (1)
- client/grpc/cmtservice/status_test.go
github-actions
bot
added
C:x/staking
C:x/gov
C:x/slashing
C:x/distribution
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (2)
- codec/depinject_test.go (1 hunks)
- simapp/app_test.go (3 hunks)
Additional context used
Path-based instructions (2)
codec/depinject_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"simapp/app_test.go (2)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.
Pattern
**/*_test.go: "Assess the unit test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (9)
codec/depinject_test.go (3)
20-22: LGTM!The
StringToBytesfunction correctly converts a string to a byte slice.
24-26: LGTM!The
BytesToStringfunction correctly converts a byte slice to a string.
30-73: LGTM!The
TestProvideAddressCodecfunction correctly tests the address codec injection using dependency injection.simapp/app_test.go (6)
Line range hint
1-1: LGTM!The
TestSimAppExportAndBlockedAddrsfunction correctly tests the export and blocked addresses functionality of the SimApp.
Line range hint
1-1: LGTM!The
TestRunMigrationsfunction correctly tests the migration functionality of the SimApp.
Line range hint
1-1: LGTM!The
TestInitGenesisOnMigrationfunction correctly tests the initialization of genesis on migration for the SimApp.
Line range hint
1-1: LGTM!The
TestUpgradeStateOnGenesisfunction correctly tests the upgrade state on genesis for the SimApp.
Line range hint
1-1: LGTM!The
TestMergedRegistryfunction correctly tests the merged registry functionality of the SimApp.
Line range hint
1-1: LGTM!The
TestProtoAnnotationsfunction correctly tests the proto annotations functionality of the SimApp.
Description
Closes: #XXXX
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
Refactor
requireassertions for error handling and validations.Chores
go.modfiles across various modules by removing unnecessary dependencies and consolidatingrequirestatements for better readability and maintenance.