coderabbitai[bot]
commented
1 day ago 📝 Walkthrough
📝 Walkthrough
## Walkthrough The changes in this pull request primarily focus on enhancing context management across various integration tests within the Cosmos SDK. The modifications involve replacing instances of `f.ctx` with `f.app.Context()` to ensure consistent context usage derived from the `integration.App` instance. Additionally, the initialization of the `integrationApp` has been streamlined by removing unnecessary context parameters. These adjustments aim to simplify the test setup process and improve the clarity of context handling throughout the test suite. ## Changes | File Path | Change Summary | |------------------------------------------------|-------------------------------------------------------------------------------------------------------------------------| | `tests/integration/accounts/fixture_test.go` | Replaced `f.ctx` with `f.app.Context()` in methods like `runBundle`, `mint`, and `balance`. Removed `newCtx` parameter in `initFixture`. | | `tests/integration/auth/keeper/accounts_retro_compatibility_test.go` | Updated context handling in `TestAuthToAccountsGRPCCompat` and `TestAccountsBaseAccountRetroCompat` to use `f.app.Context()`. | | `tests/integration/auth/keeper/fixture_test.go` | Removed `ctx` field from `fixture` struct and eliminated `newCtx` from `initFixture`. | | `tests/integration/auth/keeper/migrate_x_accounts_test.go` | Replaced `f.ctx` with `f.app.Context()` in account management operations. | | `tests/integration/auth/keeper/msg_server_test.go` | Updated `testutil.FundAccount` to use `f.app.Context()` instead of `f.ctx`. | | `tests/integration/bank/keeper/deterministic_test.go` | Removed `newCtx` and simplified initialization of `integrationApp`. | | `tests/integration/distribution/keeper/msg_server_test.go` | Removed `newCtx` and adjusted context handling for proposer and voting details. | | `tests/integration/evidence/keeper/infraction_test.go` | Streamlined context initialization by removing `newCtx`. | | `tests/integration/example/example_test.go` | Removed context and multi-store initialization in `Example` and `Example_oneModule` functions. | | `tests/integration/gov/keeper/grpc_query_test.go` | Enhanced test structure for `TestLegacyGRPCQueryTally` with clearer variable initialization. | | `tests/integration/gov/keeper/keeper_test.go` | Removed assertions for setting parameters in `bankKeeper` and `govKeeper`, eliminated `newCtx`. | | `tests/integration/slashing/keeper/keeper_test.go` | Simplified context management by removing `newCtx` and expanded tests for validator behavior. | | `tests/integration/staking/keeper/common_test.go` | Removed context and multi-store initialization, simplifying test setup. | | `tests/integration/staking/keeper/deterministic_test.go` | Altered context creation by removing multi-store context and adjusted bank parameters handling. | | `tests/integration/type_check.go` | Reorganized import statements without functional changes. | | `testutil/integration/helpers.go` | Introduced `CreateMultiStore` function for setting up multiple stores for modules. | | `testutil/integration/router.go` | Removed `moduleManager` field, updated function signatures, and improved context handling in `NewIntegrationApp`. | | `x/bank/keeper/send.go` | Added comment regarding error handling in `GetParams` method without altering functionality. | ## Possibly related PRs - **#21480**: The main PR's changes to context management in integration tests are related to the removal of the `newCtx` parameter in the `x/accounts` module, which aligns with the context handling improvements in the main PR. - **#21632**: The changes in the main PR regarding context management are relevant to the removal of the `AccountKeeper` dependency in the `x/authz` module, as both involve enhancing context usage. - **#21822**: The main PR's focus on improving context management in integration tests connects with the introduction of the `x/validate` module, which also emphasizes structured handling of contexts. - **#22043**: The refactoring of context handling in the main PR is related to the decoupling of runtime components from gRPC, as both aim to streamline the management of application contexts. - **#22131**: The simplification of merged iterators in the main PR aligns with the changes in context management, as both aim to enhance clarity and efficiency in handling data structures. - **#22147**: The updates to the documentation in the main PR regarding context management are relevant to the fixes for inaccessible links, as both improve the overall clarity and usability of the SDK. - **#22211**: The changes in the main PR to improve context handling are related to the updates in the core services documentation, as both aim to enhance clarity and structure. - **#22557**: The main PR's modifications to context management are relevant to the updates in the `NewUncachedContext` method, as both focus on ensuring accurate header information is populated. - **#22566**: The refactoring of context handling in the main PR connects with the clean-up of APIs in the cometbft module, as both aim to improve the overall structure and maintainability of the code. - **#22568**: The main PR's focus on context management is related to the changes in the `NewUncachedContext` method, ensuring that the chainID is only overwritten when necessary. - **#22620**: The adjustments in the main PR to context handling are relevant to the fixes in the `TestSendEnabled` integration test, as both aim to enhance the reliability of the tests. ## Suggested labels `C:x/accounts` ## Suggested reviewers - tac0turtle - sontrinh16 - akhilkumarpilli📜 Recent review details
**Configuration used: .coderabbit.yml** **Review profile: CHILL**📥 Commits
Reviewing files that changed from the base of the PR and between 3ab3fa21656b5c5bc3a376168f0c49fb1a62bfd2 and 6bcb81a0288fdf3a3d205fa8732b5e27546f445c.📒 Files selected for processing (1)
* `tests/integration/staking/keeper/common_test.go` (2 hunks)🚧 Files skipped from review as they are similar to previous changes (1)
* tests/integration/staking/keeper/common_test.goThank 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?
julienrbrt
Description
While debugging we the issue with gRPC state that got found in https://github.com/cosmos/cosmos-sdk/pull/22392 We realized that the double sdk context in the integration framework was super confusing.
This changed it to simply use one. Additionally, as the integration framework call initchainer, which calls init genesis, setting the default params isn't needed.
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
New Features
CreateMultiStorefor setting up multiple stores for modules.Bug Fixes
GetParamsmethod to clarify potential issues with parameter retrieval.Refactor
Appand updated related function signatures to enhance clarity.