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

refactor: limit breaking changes for next version #22972

Merged
merged 2 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
57 changes: 0 additions & 57 deletions .github/scripts/check-compat.sh

This file was deleted.

75 changes: 0 additions & 75 deletions .github/workflows/software-compat-v052.yml

This file was deleted.

3 changes: 1 addition & 2 deletions client/grpc_query_test.go
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
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

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
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
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