From fdfbce014435ed20ddc20858eaf8f52819e0f77f Mon Sep 17 00:00:00 2001 From: stackman27 Date: Sun, 3 Jul 2022 17:44:46 -0700 Subject: [PATCH 1/3] test: helper function for assertions conditional on panics --- osmoutils/cli_helpers.go | 15 +++++++++++++++ x/gamm/pool-models/balancer/pool_suite_test.go | 3 ++- x/gamm/pool-models/balancer/pool_test.go | 2 +- x/gamm/pool-models/balancer/util_test.go | 10 ---------- x/mint/keeper/genesis_test.go | 11 +++-------- x/mint/keeper/keeper_test.go | 2 +- 6 files changed, 22 insertions(+), 21 deletions(-) diff --git a/osmoutils/cli_helpers.go b/osmoutils/cli_helpers.go index 88b2a25a0dd..b343effa694 100644 --- a/osmoutils/cli_helpers.go +++ b/osmoutils/cli_helpers.go @@ -4,10 +4,12 @@ import ( "fmt" "strconv" "strings" + "testing" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/testutil/network" sdk "github.com/cosmos/cosmos-sdk/types" + "github.com/stretchr/testify/require" ) func DefaultFeeString(cfg network.Config) string { @@ -47,3 +49,16 @@ func ParseSdkIntFromString(s string, separator string) ([]sdk.Int, error) { } return parsedInts, nil } + +// ConditionalPanic checks if expectPanic is true, asserts that sut (system under test) +// panics. If expectPanic is false, asserts that sut does not panic. +// returns true if sut panics and false it it doesnot +func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) bool { + if expectPanic { + require.Panics(t, sut) + return true + } + + require.NotPanics(t, sut) + return false +} diff --git a/x/gamm/pool-models/balancer/pool_suite_test.go b/x/gamm/pool-models/balancer/pool_suite_test.go index afd071d0acb..851aa185f0e 100644 --- a/x/gamm/pool-models/balancer/pool_suite_test.go +++ b/x/gamm/pool-models/balancer/pool_suite_test.go @@ -13,6 +13,7 @@ import ( "github.com/osmosis-labs/osmosis/v7/app/apptesting" v10 "github.com/osmosis-labs/osmosis/v7/app/upgrades/v10" + "github.com/osmosis-labs/osmosis/v7/osmoutils" "github.com/osmosis-labs/osmosis/v7/x/gamm/pool-models/balancer" "github.com/osmosis-labs/osmosis/v7/x/gamm/types" ) @@ -720,7 +721,7 @@ func (suite *KeeperTestSuite) TestCalcJoinPoolShares() { require.True(t, ok) assertPoolStateNotModified(t, balancerPool, func() { - assertPanic(t, tc.expectPanic, sut) + osmoutils.ConditionalPanic(t, tc.expectPanic, sut) }) }) } diff --git a/x/gamm/pool-models/balancer/pool_test.go b/x/gamm/pool-models/balancer/pool_test.go index d4b3133672f..f02a6666d60 100644 --- a/x/gamm/pool-models/balancer/pool_test.go +++ b/x/gamm/pool-models/balancer/pool_test.go @@ -202,7 +202,7 @@ func TestCalcSingleAssetJoin(t *testing.T) { } assertPoolStateNotModified(t, balancerPool, func() { - assertPanic(t, tc.expectPanic, sut) + osmoutils.ConditionalPanic(t, tc.expectPanic, sut) }) }) } diff --git a/x/gamm/pool-models/balancer/util_test.go b/x/gamm/pool-models/balancer/util_test.go index 8a6f7f0afb2..c5e8405b696 100644 --- a/x/gamm/pool-models/balancer/util_test.go +++ b/x/gamm/pool-models/balancer/util_test.go @@ -76,13 +76,3 @@ func assertPoolStateNotModified(t *testing.T, pool *balancer.Pool, sut func()) { require.Equal(t, oldLiquidity, newLiquidity) require.Equal(t, oldShares, newShares) } - -// assertPanic if expectPanic is true, asserts that sut (system under test) -// panics. If expectPanic is false, asserts that sut does not panic. -func assertPanic(t *testing.T, expectPanic bool, sut func()) { - if expectPanic { - require.Panics(t, sut) - } else { - require.NotPanics(t, sut) - } -} diff --git a/x/mint/keeper/genesis_test.go b/x/mint/keeper/genesis_test.go index 7aa8ddcf01f..65865480ef9 100644 --- a/x/mint/keeper/genesis_test.go +++ b/x/mint/keeper/genesis_test.go @@ -7,6 +7,7 @@ import ( authtypes "github.com/cosmos/cosmos-sdk/x/auth/types" "github.com/stretchr/testify/suite" + "github.com/osmosis-labs/osmosis/v7/osmoutils" "github.com/osmosis-labs/osmosis/v7/x/mint/keeper" "github.com/osmosis-labs/osmosis/v7/x/mint/types" ) @@ -111,17 +112,11 @@ func (suite *KeeperTestSuite) TestMintInitGenesis() { originalVestingCoins := bankKeeper.GetBalance(ctx, developerAccount, tc.mintDenom) // Test. - if tc.expectPanic { - suite.Panics(func() { - mintKeeper.InitGenesis(ctx, tc.mintGenesis) - }) + + if osmoutils.ConditionalPanic(suite.T(), tc.expectPanic || false, func() { mintKeeper.InitGenesis(ctx, tc.mintGenesis) }) { return } - suite.NotPanics(func() { - mintKeeper.InitGenesis(ctx, tc.mintGenesis) - }) - // Assertions. // Module account was created. diff --git a/x/mint/keeper/keeper_test.go b/x/mint/keeper/keeper_test.go index 802cfd98268..a7c65186a10 100644 --- a/x/mint/keeper/keeper_test.go +++ b/x/mint/keeper/keeper_test.go @@ -268,7 +268,7 @@ func (suite *KeeperTestSuite) TestSetInitialSupplyOffsetDuringMigration() { isDeveloperModuleAccountCreated: true, }, "dev vesting module account does not exist": { - blockHeight: 1, + blockHeight: 1, expectedError: keeper.ErrDevVestingModuleAccountNotCreated, }, } From 367f9334487bd099f4a835ebddb83284e812b903 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Mon, 4 Jul 2022 22:35:35 -0700 Subject: [PATCH 2/3] added devs comments --- osmoutils/cli_helpers.go | 5 ++--- x/mint/keeper/genesis_test.go | 4 ++-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/osmoutils/cli_helpers.go b/osmoutils/cli_helpers.go index b343effa694..1e9e29f8789 100644 --- a/osmoutils/cli_helpers.go +++ b/osmoutils/cli_helpers.go @@ -53,12 +53,11 @@ func ParseSdkIntFromString(s string, separator string) ([]sdk.Int, error) { // ConditionalPanic checks if expectPanic is true, asserts that sut (system under test) // panics. If expectPanic is false, asserts that sut does not panic. // returns true if sut panics and false it it doesnot -func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) bool { +func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) { if expectPanic { require.Panics(t, sut) - return true + return } require.NotPanics(t, sut) - return false } diff --git a/x/mint/keeper/genesis_test.go b/x/mint/keeper/genesis_test.go index 65865480ef9..72a29ba8b9c 100644 --- a/x/mint/keeper/genesis_test.go +++ b/x/mint/keeper/genesis_test.go @@ -112,8 +112,8 @@ func (suite *KeeperTestSuite) TestMintInitGenesis() { originalVestingCoins := bankKeeper.GetBalance(ctx, developerAccount, tc.mintDenom) // Test. - - if osmoutils.ConditionalPanic(suite.T(), tc.expectPanic || false, func() { mintKeeper.InitGenesis(ctx, tc.mintGenesis) }) { + osmoutils.ConditionalPanic(suite.T(), tc.expectPanic, func() { mintKeeper.InitGenesis(ctx, tc.mintGenesis) }) + if tc.expectPanic { return } From 00e2b76ceb570ca817919e94f3d9ccf585504fd1 Mon Sep 17 00:00:00 2001 From: stackman27 Date: Tue, 5 Jul 2022 11:06:52 -0700 Subject: [PATCH 3/3] matts comment --- osmoutils/cli_helpers.go | 14 -------------- osmoutils/test_helpers.go | 18 ++++++++++++++++++ 2 files changed, 18 insertions(+), 14 deletions(-) create mode 100644 osmoutils/test_helpers.go diff --git a/osmoutils/cli_helpers.go b/osmoutils/cli_helpers.go index 1e9e29f8789..88b2a25a0dd 100644 --- a/osmoutils/cli_helpers.go +++ b/osmoutils/cli_helpers.go @@ -4,12 +4,10 @@ import ( "fmt" "strconv" "strings" - "testing" "github.com/cosmos/cosmos-sdk/client/flags" "github.com/cosmos/cosmos-sdk/testutil/network" sdk "github.com/cosmos/cosmos-sdk/types" - "github.com/stretchr/testify/require" ) func DefaultFeeString(cfg network.Config) string { @@ -49,15 +47,3 @@ func ParseSdkIntFromString(s string, separator string) ([]sdk.Int, error) { } return parsedInts, nil } - -// ConditionalPanic checks if expectPanic is true, asserts that sut (system under test) -// panics. If expectPanic is false, asserts that sut does not panic. -// returns true if sut panics and false it it doesnot -func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) { - if expectPanic { - require.Panics(t, sut) - return - } - - require.NotPanics(t, sut) -} diff --git a/osmoutils/test_helpers.go b/osmoutils/test_helpers.go new file mode 100644 index 00000000000..a331ce00c3a --- /dev/null +++ b/osmoutils/test_helpers.go @@ -0,0 +1,18 @@ +package osmoutils + +import ( + "testing" + + "github.com/stretchr/testify/require" +) + +// ConditionalPanic checks if expectPanic is true, asserts that sut (system under test) +// panics. If expectPanic is false, asserts that sut does not panic. +// returns true if sut panics and false it it does not +func ConditionalPanic(t *testing.T, expectPanic bool, sut func()) { + if expectPanic { + require.Panics(t, sut) + return + } + require.NotPanics(t, sut) +}