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.

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

Jump to bottom

refactor(tests): improve x/reports tests using gomock #1184

Merged
merged 2 commits into from
Jun 26, 2023
Merged

refactor(tests): improve x/reports tests using gomock #1184

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a89646c
chore(tests): improve x/reports tests using gomock
dadamu Jun 26, 2023
59bc4f3
chore: improve after subspace deleted hook by change deletion order
dadamu Jun 26, 2023
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
56 changes: 19 additions & 37 deletions x/reports/keeper/common_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,14 +3,8 @@ package keeper_test
import (
"testing"

authkeeper "github.com/cosmos/cosmos-sdk/x/auth/keeper"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

profileskeeper "github.com/desmos-labs/desmos/v5/x/profiles/keeper"
profilestypes "github.com/desmos-labs/desmos/v5/x/profiles/types"

postskeeper "github.com/desmos-labs/desmos/v5/x/posts/keeper"
poststypes "github.com/desmos-labs/desmos/v5/x/posts/types"
"github.com/golang/mock/gomock"

db "github.com/cometbft/cometbft-db"
"github.com/cometbft/cometbft/libs/log"
Expand All @@ -19,43 +13,32 @@ import (
"github.com/cosmos/cosmos-sdk/store"
storetypes "github.com/cosmos/cosmos-sdk/store/types"
sdk "github.com/cosmos/cosmos-sdk/types"
paramstypes "github.com/cosmos/cosmos-sdk/x/params/types"
"github.com/stretchr/testify/suite"

"github.com/desmos-labs/desmos/v5/app"
relationshipskeeper "github.com/desmos-labs/desmos/v5/x/relationships/keeper"
relationshipstypes "github.com/desmos-labs/desmos/v5/x/relationships/types"
"github.com/desmos-labs/desmos/v5/x/reports/keeper"
"github.com/desmos-labs/desmos/v5/x/reports/testutil"
"github.com/desmos-labs/desmos/v5/x/reports/types"
subspaceskeeper "github.com/desmos-labs/desmos/v5/x/subspaces/keeper"
subspacestypes "github.com/desmos-labs/desmos/v5/x/subspaces/types"
)

type KeeperTestSuite struct {
suite.Suite

cdc codec.Codec
legacyAminoCdc *codec.LegacyAmino
ctx sdk.Context
cdc codec.Codec
ctx sdk.Context

storeKey storetypes.StoreKey
k keeper.Keeper

ak profileskeeper.Keeper
sk subspaceskeeper.Keeper
rk relationshipskeeper.Keeper
pk postskeeper.Keeper
ak *testutil.MockProfilesKeeper
sk *testutil.MockSubspacesKeeper
rk *testutil.MockRelationshipsKeeper
pk *testutil.MockPostsKeeper
}

func (suite *KeeperTestSuite) SetupTest() {
// Define store keys
keys := sdk.NewMemoryStoreKeys(
paramstypes.StoreKey, authtypes.StoreKey,
profilestypes.StoreKey, relationshipstypes.StoreKey,
subspacestypes.StoreKey, poststypes.StoreKey,
types.StoreKey,
)
tKeys := sdk.NewTransientStoreKeys(paramstypes.TStoreKey)
keys := sdk.NewMemoryStoreKeys(types.StoreKey)
suite.storeKey = keys[types.StoreKey]

// Create an in-memory db
Expand All @@ -64,23 +47,22 @@ func (suite *KeeperTestSuite) SetupTest() {
for _, key := range keys {
ms.MountStoreWithDB(key, storetypes.StoreTypeIAVL, memDB)
}
for _, tKey := range tKeys {
ms.MountStoreWithDB(tKey, storetypes.StoreTypeTransient, memDB)
}

if err := ms.LoadLatestVersion(); err != nil {
panic(err)
}

suite.ctx = sdk.NewContext(ms, tmproto.Header{ChainID: "test-chain"}, false, log.NewNopLogger())
suite.cdc, suite.legacyAminoCdc = app.MakeCodecs()
suite.cdc, _ = app.MakeCodecs()

// Mocks initializations
ctrl := gomock.NewController(suite.T())
defer ctrl.Finish()

// Define keeper
suite.sk = subspaceskeeper.NewKeeper(suite.cdc, keys[subspacestypes.StoreKey], nil, nil, authtypes.NewModuleAddress("gov").String())
suite.rk = relationshipskeeper.NewKeeper(suite.cdc, keys[relationshipstypes.StoreKey], suite.sk)
authKeeper := authkeeper.NewAccountKeeper(suite.cdc, keys[authtypes.StoreKey], authtypes.ProtoBaseAccount, app.GetMaccPerms(), "cosmos", authtypes.NewModuleAddress("gov").String())
suite.ak = profileskeeper.NewKeeper(suite.cdc, suite.legacyAminoCdc, keys[profilestypes.StoreKey], authKeeper, suite.rk, nil, nil, nil, authtypes.NewModuleAddress("gov").String())
suite.pk = postskeeper.NewKeeper(suite.cdc, keys[poststypes.StoreKey], suite.ak, suite.sk, suite.rk, authtypes.NewModuleAddress("gov").String())
// Setup keepers
suite.sk = testutil.NewMockSubspacesKeeper(ctrl)
suite.rk = testutil.NewMockRelationshipsKeeper(ctrl)
suite.ak = testutil.NewMockProfilesKeeper(ctrl)
suite.pk = testutil.NewMockPostsKeeper(ctrl)
suite.k = keeper.NewKeeper(
suite.cdc,
suite.storeKey,
Expand Down
18 changes: 9 additions & 9 deletions x/reports/keeper/external_hooks.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,6 @@ func (h Hooks) AfterSubspaceSaved(ctx sdk.Context, subspaceID uint64) {

// AfterSubspaceDeleted implements subspacestypes.Hooks
func (h Hooks) AfterSubspaceDeleted(ctx sdk.Context, subspaceID uint64) {
// Delete the reason id key
h.k.DeleteNextReasonID(ctx, subspaceID)

// Delete all the reasons related to this subspace
h.k.IterateSubspaceReasons(ctx, subspaceID, func(reason types.Reason) (stop bool) {
h.k.DeleteReason(ctx, reason.SubspaceID, reason.ID)
return false
})

// Delete the report id key
h.k.DeleteNextReportID(ctx, subspaceID)

Expand All @@ -53,6 +44,15 @@ func (h Hooks) AfterSubspaceDeleted(ctx sdk.Context, subspaceID uint64) {
h.k.DeleteReport(ctx, report.SubspaceID, report.ID)
return false
})

// Delete the reason id key
h.k.DeleteNextReasonID(ctx, subspaceID)

// Delete all the reasons related to this subspace
h.k.IterateSubspaceReasons(ctx, subspaceID, func(reason types.Reason) (stop bool) {
h.k.DeleteReason(ctx, reason.SubspaceID, reason.ID)
return false
})
Comment on lines +47 to +55
Copy link
Contributor

Choose a reason for hiding this comment

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

Why was this code moved here instead of keeping it above?

Copy link
Contributor Author

@dadamu dadamu Jun 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

DeleteReason also deletes reports containing reasons here, but it causes O(N) since it iterates all the reports when removing a reason, as a result, it makes AfterSubspaceDeleted has O(N^2) time complexity.

Moving this code above can have better performance with O(2N) time complexity since reports containing reasons are removed first.

}

// AfterSubspaceGroupSaved implements subspacestypes.Hooks
Expand Down
77 changes: 10 additions & 67 deletions x/reports/keeper/external_hooks_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,33 +3,21 @@ package keeper_test
import (
"time"

poststypes "github.com/desmos-labs/desmos/v5/x/posts/types"

sdk "github.com/cosmos/cosmos-sdk/types"

"github.com/desmos-labs/desmos/v5/x/reports/types"
subspacestypes "github.com/desmos-labs/desmos/v5/x/subspaces/types"
)

func (suite *KeeperTestSuite) TestKeeper_AfterSubspaceSaved() {
testCases := []struct {
name string
store func(ctx sdk.Context)
subspace subspacestypes.Subspace
check func(ctx sdk.Context)
name string
store func(ctx sdk.Context)
subspaceID uint64
check func(ctx sdk.Context)
}{
{
name: "saving a subspaces adds the correct keys",
subspace: subspacestypes.NewSubspace(
1,
"Test subspace",
"This is a test subspace",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
time.Date(2020, 1, 2, 12, 00, 00, 000, time.UTC),
nil,
),
name: "saving a subspaces adds the correct keys",
subspaceID: 1,
check: func(ctx sdk.Context) {
storedReasonID, err := suite.k.GetNextReasonID(ctx, 1)
suite.Require().NoError(err)
Expand All @@ -43,29 +31,10 @@ func (suite *KeeperTestSuite) TestKeeper_AfterSubspaceSaved() {
{
name: "reason and report ids are not overwritten",
store: func(ctx sdk.Context) {
suite.sk.SaveSubspace(ctx, subspacestypes.NewSubspace(
1,
"Test subspace",
"This is a test subspace",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
time.Date(2020, 1, 2, 12, 00, 00, 000, time.UTC),
nil,
))
suite.k.SetNextReportID(ctx, 1, 2)
suite.k.SetNextReasonID(ctx, 1, 2)
},
subspace: subspacestypes.NewSubspace(
1,
"Test subspace",
"This is a test subspace",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
"cosmos1a0cj0j6ujn2xap8p40y6648d0w2npytw3xvenm",
time.Date(2020, 1, 2, 12, 00, 00, 000, time.UTC),
nil,
),
subspaceID: 1,
check: func(ctx sdk.Context) {
storedReasonID, err := suite.k.GetNextReasonID(ctx, 1)
suite.Require().NoError(err)
Expand All @@ -78,9 +47,6 @@ func (suite *KeeperTestSuite) TestKeeper_AfterSubspaceSaved() {
},
}

// Set the hooks
suite.sk.SetHooks(suite.k.Hooks())

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
Expand All @@ -89,7 +55,7 @@ func (suite *KeeperTestSuite) TestKeeper_AfterSubspaceSaved() {
tc.store(ctx)
}

suite.sk.SaveSubspace(ctx, tc.subspace)
suite.k.Hooks().AfterSubspaceSaved(ctx, tc.subspaceID)
if tc.check != nil {
tc.check(ctx)
}
Expand Down Expand Up @@ -141,9 +107,6 @@ func (suite *KeeperTestSuite) TestKeeper_AfterSubspaceDeleted() {
},
}

// Set the hooks
suite.sk.SetHooks(suite.k.Hooks())

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
Expand All @@ -152,7 +115,7 @@ func (suite *KeeperTestSuite) TestKeeper_AfterSubspaceDeleted() {
tc.store(ctx)
}

suite.sk.DeleteSubspace(ctx, tc.subspaceID)
suite.k.Hooks().AfterSubspaceDeleted(ctx, tc.subspaceID)
if tc.check != nil {
tc.check(ctx)
}
Expand All @@ -171,23 +134,6 @@ func (suite *KeeperTestSuite) TestKeeper_AfterPostDeleted() {
{
name: "deleting a post removes all the associated reports",
store: func(ctx sdk.Context) {
suite.pk.SavePost(ctx, poststypes.NewPost(
1,
0,
1,
"External ID",
"This is a text",
"cosmos13t6y2nnugtshwuy0zkrq287a95lyy8vzleaxmd",
1,
nil,
nil,
nil,
poststypes.REPLY_SETTING_EVERYONE,
time.Date(2020, 1, 1, 12, 00, 00, 000, time.UTC),
nil,
"cosmos13t6y2nnugtshwuy0zkrq287a95lyy8vzleaxmd",
))

suite.k.SaveReport(ctx, types.NewReport(
1,
1,
Expand All @@ -209,9 +155,6 @@ func (suite *KeeperTestSuite) TestKeeper_AfterPostDeleted() {
},
}

// Set the hooks
suite.pk.SetHooks(suite.k.Hooks())

for _, tc := range testCases {
tc := tc
suite.Run(tc.name, func() {
Expand All @@ -220,7 +163,7 @@ func (suite *KeeperTestSuite) TestKeeper_AfterPostDeleted() {
tc.store(ctx)
}

suite.pk.DeletePost(ctx, tc.subspaceID, tc.postID)
suite.k.Hooks().AfterPostDeleted(ctx, tc.subspaceID, tc.postID)
if tc.check != nil {
tc.check(ctx)
}
Expand Down
Loading
Loading