Skip to content

Commit

Permalink
The following misc test improvements: (#612)
Browse files Browse the repository at this point in the history 
* x/gamm: Deduplicate the FundAccounts coins constant
* x/lockup: Fix noise from FundAccount in gas_test
* osmotestutils: Add GetFeeString function
* Replace complex fee cli line with simple osmotestutils call
  • Loading branch information
@ValarDragon
ValarDragon authored Nov 24, 2021
1 parent e9b47f8 commit 17f3de3
Show file tree
Hide file tree
Showing 6 changed files with 53 additions and 45 deletions.
14 changes: 14 additions & 0 deletions osmotestutils/cli_helpers.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
package osmotestutils

import (
"fmt"

"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/testutil/network"
sdk "github.com/cosmos/cosmos-sdk/types"
)

func DefaultFeeString(cfg network.Config) string {
feeCoins := sdk.NewCoins(sdk.NewCoin(cfg.BondDenom, sdk.NewInt(10)))
return fmt.Sprintf("--%s=%s", flags.FlagFees, feeCoins.String())
}
15 changes: 8 additions & 7 deletions x/gamm/client/cli/cli_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
"github.com/osmosis-labs/osmosis/app"
"github.com/osmosis-labs/osmosis/osmotestutils"
"github.com/osmosis-labs/osmosis/x/gamm/client/cli"
gammtestutil "github.com/osmosis-labs/osmosis/x/gamm/client/testutil"
"github.com/osmosis-labs/osmosis/x/gamm/types"
Expand Down Expand Up @@ -78,7 +79,7 @@ func (s *IntegrationTestSuite) TestNewCreatePoolCmd() {
newAddr,
sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 200000000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down Expand Up @@ -324,7 +325,7 @@ func (s *IntegrationTestSuite) TestNewCreatePoolCmd() {
// common args
fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(sdk.DefaultBondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
fmt.Sprintf("--%s=%s", flags.FlagGas, fmt.Sprint(300000)),
}

Expand Down Expand Up @@ -357,7 +358,7 @@ func (s IntegrationTestSuite) TestNewJoinPoolCmd() {
newAddr,
sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down Expand Up @@ -495,7 +496,7 @@ func (s IntegrationTestSuite) TestNewSwapExactAmountOutCmd() {
newAddr,
sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down Expand Up @@ -559,7 +560,7 @@ func (s IntegrationTestSuite) TestNewJoinSwapExternAmountInCmd() {
newAddr,
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down Expand Up @@ -667,7 +668,7 @@ func (s IntegrationTestSuite) TestNewJoinSwapShareAmountOutCmd() {
newAddr,
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down Expand Up @@ -1118,7 +1119,7 @@ func (s IntegrationTestSuite) TestNewSwapExactAmountInCmd() {
newAddr,
sdk.NewCoins(sdk.NewInt64Coin(s.cfg.BondDenom, 20000), sdk.NewInt64Coin("node0token", 20000)), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down
12 changes: 6 additions & 6 deletions x/gamm/keeper/invariants.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ func PoolAccountInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant {
pools, err := keeper.GetPools(ctx)
if err != nil {
return sdk.FormatInvariant(types.ModuleName, poolBalanceInvariantName,
fmt.Sprintf("\tgamm pool retrieval failed")), true
"\tgamm pool retrieval failed"), true
}

for _, pool := range pools {
Expand All @@ -57,7 +57,7 @@ func PoolAccountInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant {
}

return sdk.FormatInvariant(types.ModuleName, poolBalanceInvariantName,
fmt.Sprintf("\tgamm all pool asset coins and account coins match\n")), false
"\tgamm all pool asset coins and account coins match\n"), false
}
}

Expand All @@ -68,7 +68,7 @@ func PoolTotalWeightInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant
pools, err := keeper.GetPools(ctx)
if err != nil {
return sdk.FormatInvariant(types.ModuleName, "pool-total-weight",
fmt.Sprintf("\tgamm pool retrieval failed")), true
"\tgamm pool retrieval failed"), true
}

for _, pool := range pools {
Expand All @@ -84,7 +84,7 @@ func PoolTotalWeightInvariant(keeper Keeper, bk types.BankKeeper) sdk.Invariant
}

return sdk.FormatInvariant(types.ModuleName, "pool-total-weight",
fmt.Sprintf("\tgamm all pool calculated and stored total weight match\n")), false
"\tgamm all pool calculated and stored total weight match\n"), false
}
}

Expand Down Expand Up @@ -130,7 +130,7 @@ func PoolProductConstantInvariant(keeper Keeper) sdk.Invariant {
newpools, err := keeper.GetPools(ctx)
if err != nil {
return sdk.FormatInvariant(types.ModuleName, "pool-product-constant",
fmt.Sprintf("\tgamm pool retrieval failed")), true
"\tgamm pool retrieval failed"), true
}

for _, pool := range newpools {
Expand All @@ -150,6 +150,6 @@ func PoolProductConstantInvariant(keeper Keeper) sdk.Invariant {
}

return sdk.FormatInvariant(types.ModuleName, "pool-product-constant",
fmt.Sprintf("\tgamm all pool product constant preserved\n")), false
"\tgamm all pool product constant preserved\n"), false
}
}
34 changes: 10 additions & 24 deletions x/gamm/keeper/pool_service_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,12 @@ var (
Token: sdk.NewCoin("bar", sdk.NewInt(10000)),
}
defaultPoolAssets []types.PoolAsset = []types.PoolAsset{defaultFooAsset, defaultBarAsset}
defaultAcctFunds sdk.Coins = sdk.NewCoins(
sdk.NewCoin("uosmo", sdk.NewInt(10000000000)),
sdk.NewCoin("foo", sdk.NewInt(10000000)),
sdk.NewCoin("bar", sdk.NewInt(10000000)),
sdk.NewCoin("baz", sdk.NewInt(10000000)),
)
)

func (suite *KeeperTestSuite) TestCreatePool() {
Expand Down Expand Up @@ -196,12 +202,7 @@ func (suite *KeeperTestSuite) TestCreatePool() {

// Mint some assets to the accounts.
for _, acc := range []sdk.AccAddress{acc1, acc2, acc3} {
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, sdk.NewCoins(
sdk.NewCoin("uosmo", sdk.NewInt(10000000000)),
sdk.NewCoin("foo", sdk.NewInt(10000000)),
sdk.NewCoin("bar", sdk.NewInt(10000000)),
sdk.NewCoin("baz", sdk.NewInt(10000000)),
))
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, defaultAcctFunds)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -280,12 +281,7 @@ func (suite *KeeperTestSuite) TestJoinPool() {

// Mint some assets to the accounts.
for _, acc := range []sdk.AccAddress{acc1, acc2, acc3} {
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, sdk.NewCoins(
sdk.NewCoin("uosmo", sdk.NewInt(10000000000)),
sdk.NewCoin("foo", sdk.NewInt(10000000)),
sdk.NewCoin("bar", sdk.NewInt(10000000)),
sdk.NewCoin("baz", sdk.NewInt(10000000)),
))
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, defaultAcctFunds)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -377,12 +373,7 @@ func (suite *KeeperTestSuite) TestExitPool() {

// Mint some assets to the accounts.
for _, acc := range []sdk.AccAddress{acc1, acc2, acc3} {
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, sdk.NewCoins(
sdk.NewCoin("uosmo", sdk.NewInt(10000000000)),
sdk.NewCoin("foo", sdk.NewInt(10000000)),
sdk.NewCoin("bar", sdk.NewInt(10000000)),
sdk.NewCoin("baz", sdk.NewInt(10000000)),
))
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, defaultAcctFunds)
if err != nil {
panic(err)
}
Expand Down Expand Up @@ -415,12 +406,7 @@ func (suite *KeeperTestSuite) TestActivePool() {

// Mint some assets to the accounts.
for _, acc := range []sdk.AccAddress{acc1, acc2, acc3} {
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, sdk.NewCoins(
sdk.NewCoin("uosmo", sdk.NewInt(10000000000)),
sdk.NewCoin("foo", sdk.NewInt(10000000)),
sdk.NewCoin("bar", sdk.NewInt(10000000)),
sdk.NewCoin("baz", sdk.NewInt(10000000)),
))
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, acc, defaultAcctFunds)
if err != nil {
panic(err)
}
Expand Down
5 changes: 3 additions & 2 deletions x/lockup/client/cli/cli_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
banktestutil "github.com/cosmos/cosmos-sdk/x/bank/client/testutil"
"github.com/osmosis-labs/osmosis/app"
"github.com/osmosis-labs/osmosis/osmotestutils"
"github.com/osmosis-labs/osmosis/x/lockup/client/cli"
lockuptestutil "github.com/osmosis-labs/osmosis/x/lockup/client/testutil"
"github.com/osmosis-labs/osmosis/x/lockup/types"
Expand Down Expand Up @@ -152,7 +153,7 @@ func (s *IntegrationTestSuite) TestBeginUnlockingCmd() {
newAddr,
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down Expand Up @@ -220,7 +221,7 @@ func (s *IntegrationTestSuite) TestNewBeginUnlockPeriodLockCmd() {
newAddr,
sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(20000))), fmt.Sprintf("--%s=true", flags.FlagSkipConfirmation),
fmt.Sprintf("--%s=%s", flags.FlagBroadcastMode, flags.BroadcastBlock),
fmt.Sprintf("--%s=%s", flags.FlagFees, sdk.NewCoins(sdk.NewCoin(s.cfg.BondDenom, sdk.NewInt(10))).String()),
osmotestutils.DefaultFeeString(s.cfg),
)
s.Require().NoError(err)

Expand Down
18 changes: 12 additions & 6 deletions x/lockup/keeper/gas_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"fmt"
"time"

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

Expand All @@ -13,8 +14,13 @@ var (
)

func (suite *KeeperTestSuite) measureLockGas(addr sdk.AccAddress, coins sdk.Coins, dur time.Duration) uint64 {
// fundAccount outside of gas measurement
err := simapp.FundAccount(suite.app.BankKeeper, suite.ctx, addr, coins)
suite.Require().NoError(err)
// start measuring gas
alreadySpent := suite.ctx.GasMeter().GasConsumed()
suite.LockTokens(addr, coins, dur)
_, err = suite.app.LockupKeeper.LockTokens(suite.ctx, addr, coins, dur)
suite.Require().NoError(err)
newSpent := suite.ctx.GasMeter().GasConsumed()
spentNow := newSpent - alreadySpent
return spentNow
Expand Down Expand Up @@ -50,15 +56,15 @@ func (suite *KeeperTestSuite) TestRepeatedLockTokensGas() {
totalNumLocks := 10000

firstLockGasAmount := suite.measureLockGas(defaultAddr, defaultCoins, time.Second)
suite.Assert().Equal(93703, int(firstLockGasAmount))
suite.Assert().Equal(70305, int(firstLockGasAmount))

for i := 1; i < startAveragingAt; i++ {
suite.LockTokens(defaultAddr, defaultCoins, time.Second)
}
avgGas, maxGas := suite.measureAvgAndMaxLockGas(totalNumLocks-startAveragingAt, defaultAddr, coinsFn, durFn)
fmt.Printf("test deets: total locks created %d, begin average at %d\n", totalNumLocks, startAveragingAt)
suite.Assert().Equal(75618, int(avgGas), "average gas / lock")
suite.Assert().Equal(75708, int(maxGas), "max gas / lock")
suite.Assert().Equal(59527, int(avgGas), "average gas / lock")
suite.Assert().Equal(59617, int(maxGas), "max gas / lock")
}

func (suite *KeeperTestSuite) TestRepeatedLockTokensDistinctDurationGas() {
Expand All @@ -70,6 +76,6 @@ func (suite *KeeperTestSuite) TestRepeatedLockTokensDistinctDurationGas() {

avgGas, maxGas := suite.measureAvgAndMaxLockGas(totalNumLocks, defaultAddr, coinsFn, durFn)
fmt.Printf("test deets: total locks created %d\n", totalNumLocks)
suite.Assert().EqualValues(122316, int(avgGas), "average gas / lock")
suite.Assert().EqualValues(242903, int(maxGas), "max gas / lock")
suite.Assert().EqualValues(106225, int(avgGas), "average gas / lock")
suite.Assert().EqualValues(226812, int(maxGas), "max gas / lock")
}

0 comments on commit 17f3de3

Please sign in to comment.