Skip to content

refactor: limit breaking changes for next version #22972

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.

Sign up for GitHub

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

Merged
julienrbrt merged 2 commits into from
Dec 18, 2024
Merged

refactor: limit breaking changes for next version #22972

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
ef14a84
refactor: limit breaking changes for next version
julienrbrt Dec 18, 2024
73a4af2
remove CI
julienrbrt Dec 18, 2024
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
3 changes: 1 addition & 2 deletions client/grpc_query_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/testutil"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil/integration"
"github.com/cosmos/cosmos-sdk/testutil/testdata"
"github.com/cosmos/cosmos-sdk/testutil/x/counter"
counterkeeper "github.com/cosmos/cosmos-sdk/testutil/x/counter/keeper"
Expand All @@ -38,7 +37,7 @@ func (s *IntegrationTestSuite) SetupSuite() {

logger := log.NewNopLogger()
keys := storetypes.NewKVStoreKeys(countertypes.StoreKey)
cms := integration.CreateMultiStore(keys, logger)
cms := moduletestutil.CreateMultiStore(keys, logger)
s.ctx = sdk.NewContext(cms, true, logger)
cfg := moduletestutil.MakeTestEncodingConfig(testutil.CodecOptions{}, counter.AppModule{})
s.cdc = cfg.Codec
Expand Down
3 changes: 1 addition & 2 deletions tests/integration/distribution/migration_v4_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ import (
"github.com/cosmos/cosmos-sdk/crypto/keys/secp256k1"
"github.com/cosmos/cosmos-sdk/runtime"
"github.com/cosmos/cosmos-sdk/testutil"
"github.com/cosmos/cosmos-sdk/testutil/integration"
sdk "github.com/cosmos/cosmos-sdk/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
"github.com/cosmos/cosmos-sdk/x/auth"
Expand Down Expand Up @@ -75,7 +74,7 @@ func TestFundsMigration(t *testing.T) {
authtypes.StoreKey, banktypes.StoreKey, disttypes.StoreKey,
)
logger := log.NewTestLogger(t)
cms := integration.CreateMultiStore(keys, logger)
cms := moduletestutil.CreateMultiStore(keys, logger)
Copy link
Contributor

@coderabbitai coderabbitai bot Dec 18, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

The CreateMultiStore change is correct but part of a larger migration

The verification shows that the change in tests/integration/distribution/migration_v4_test.go is correct, as CreateMultiStore is already aliased in testutil/integration/helpers.go to point to moduletestutil.CreateMultiStore. However, this change is part of a larger migration effort since:

  • The files still importing the old package are using other integration utilities like NewIntegrationApp, WithAutomaticFinalizeBlock, and WithAutomaticCommit
  • The old import path is still referenced in multiple test files
  • The testutil/integration/helpers.go file shows CreateMultiStore is marked as deprecated with instructions to use the new path directly
🔗 Analysis chain

LGTM: Successful migration of CreateMultiStore usage.

The change aligns with the PR objectives to preserve helper functions while removing the testutil/integration module.

Let's verify that all usages of CreateMultiStore have been migrated consistently:

Migration of CreateMultiStore requires additional changes

The verification revealed that there are still several files importing the old testutil/integration package, and the migration is incomplete. Key findings:

  • Files still using the old import:
    • tests/integration/server/grpc_test.go
    • tests/integration/example/example_test.go
    • tests/integration/bank/keeper/deterministic_test.go
    • tests/integration/auth/keeper/msg_server_test.go
    • tests/integration/auth/keeper/fixture_test.go

Let's verify if these files also use CreateMultiStore and need similar migration:

Let's check what other functions these files are using from the integration package to better understand the migration scope:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for any remaining references to integration.CreateMultiStore
# and verify consistent usage of moduletestutil.CreateMultiStore

# Check for any remaining references to the old import
rg -l "github.com/cosmos/cosmos-sdk/testutil/integration"

# Check for any remaining references to integration.CreateMultiStore
rg "integration\.CreateMultiStore"

# Verify consistent usage of the new import
rg -l "github.com/cosmos/cosmos-sdk/types/module/testutil"

# Verify consistent usage of moduletestutil.CreateMultiStore
rg "moduletestutil\.CreateMultiStore"

Length of output: 4520

Script:

#!/bin/bash
# Check the content of files still using the old import to see if they use CreateMultiStore
for file in tests/integration/server/grpc_test.go tests/integration/example/example_test.go tests/integration/bank/keeper/deterministic_test.go tests/integration/auth/keeper/msg_server_test.go tests/integration/auth/keeper/fixture_test.go; do
    echo "=== $file ==="
    rg "CreateMultiStore" "$file" -C 2
done

Length of output: 2145

Script:

#!/bin/bash
# Check what other integration package functions these files are using
for file in tests/integration/server/grpc_test.go tests/integration/example/example_test.go tests/integration/bank/keeper/deterministic_test.go tests/integration/auth/keeper/msg_server_test.go tests/integration/auth/keeper/fixture_test.go; do
    echo "=== $file ==="
    # Get a few lines around any line that uses the integration package
    rg -A 2 -B 2 "integration\." "$file"
done

# Also check the helpers file to understand the integration package usage
echo "=== testutil/integration/helpers.go ==="
cat testutil/integration/helpers.go

Length of output: 4404

encCfg := moduletestutil.MakeTestEncodingConfig(codectestutil.CodecOptions{}, auth.AppModule{}, bank.AppModule{}, distribution.AppModule{})
ctx := sdk.NewContext(cms, true, logger)
addressCodec := addresscodec.NewBech32Codec(sdk.Bech32MainPrefix)
Expand Down
19 changes: 3 additions & 16 deletions testutil/integration/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,22 +1,9 @@
package integration

import (
coretesting "cosmossdk.io/core/testing"
"cosmossdk.io/log"
"cosmossdk.io/store"
"cosmossdk.io/store/metrics"
storetypes "cosmossdk.io/store/types"
moduletestutil "github.com/cosmos/cosmos-sdk/types/module/testutil"
)

// CreateMultiStore is a helper for setting up multiple stores for provided modules.
func CreateMultiStore(keys map[string]*storetypes.KVStoreKey, logger log.Logger) storetypes.CommitMultiStore {
db := coretesting.NewMemDB()
cms := store.NewCommitMultiStore(db, logger, metrics.NewNoOpMetrics())

for key := range keys {
cms.MountStoreWithDB(keys[key], storetypes.StoreTypeIAVL, db)
}

_ = cms.LoadLatestVersion()
return cms
}
// Deprecated: use github.com/cosmos/cosmos-sdk/types/module/testutil.CreateMultiStore instead.
var CreateMultiStore = moduletestutil.CreateMultiStore
19 changes: 19 additions & 0 deletions types/module/testutil/codec.go → types/module/testutil/helpers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,12 @@
package testutil

import (
coretesting "cosmossdk.io/core/testing"
"cosmossdk.io/log"
"cosmossdk.io/store"
"cosmossdk.io/store/metrics"
storetypes "cosmossdk.io/store/types"

"github.com/cosmos/cosmos-sdk/client"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/testutil"
Expand Down Expand Up @@ -78,3 +84,16 @@ type TestTxBuilder struct {
func (b *TestTxBuilder) SetExtensionOptions(extOpts ...*types.Any) {
b.ExtOptions = extOpts
}

// CreateMultiStore is a helper for setting up multiple stores for provided modules.
func CreateMultiStore(keys map[string]*storetypes.KVStoreKey, logger log.Logger) storetypes.CommitMultiStore {
db := coretesting.NewMemDB()
cms := store.NewCommitMultiStore(db, logger, metrics.NewNoOpMetrics())

for key := range keys {
cms.MountStoreWithDB(keys[key], storetypes.StoreTypeIAVL, db)
}

_ = cms.LoadLatestVersion()
return cms
}
Loading