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

style: address forced type assertions in tests #5116

Merged
merged 11 commits into from
May 10, 2023
4 changes: 2 additions & 2 deletions app/upgrades/v15/upgrade_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -178,7 +178,7 @@ func (suite *UpgradeTestSuite) TestRegisterOsmoIonMetadata() {
bankKeeper := suite.App.BankKeeper

// meta data should not be found pre-registration of meta data
uosmoMetadata, found := suite.App.BankKeeper.GetDenomMetaData(ctx, "uosmo")
_, found := suite.App.BankKeeper.GetDenomMetaData(ctx, "uosmo")
suite.Require().False(found)

uionMetadata, found := suite.App.BankKeeper.GetDenomMetaData(ctx, "uion")
Expand All @@ -187,7 +187,7 @@ func (suite *UpgradeTestSuite) TestRegisterOsmoIonMetadata() {
// system under test.
v15.RegisterOsmoIonMetadata(ctx, *bankKeeper)

uosmoMetadata, found = suite.App.BankKeeper.GetDenomMetaData(ctx, "uosmo")
uosmoMetadata, found := suite.App.BankKeeper.GetDenomMetaData(ctx, "uosmo")
suite.Require().True(found)

uionMetadata, found = suite.App.BankKeeper.GetDenomMetaData(ctx, "uion")
Expand Down
47 changes: 36 additions & 11 deletions tests/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,11 @@ func (s *IntegrationTestSuite) TestConcentratedLiquidity() {
s.Require().NoError(err)

var (
denom0 string = "uion"
denom1 string = "uosmo"
tickSpacing uint64 = 100
swapFee = "0.001" // 0.1%
swapFeeDec sdk.Dec = sdk.MustNewDecFromStr("0.001")
denom0 = "uion"
denom1 = "uosmo"
tickSpacing uint64 = 100
swapFee = "0.001" // 0.1%
swapFeeDec = sdk.MustNewDecFromStr("0.001")
)

// Get the permisionless pool creation parameter.
Expand Down Expand Up @@ -1056,12 +1056,34 @@ func (s *IntegrationTestSuite) TestIBCWasmHooks() {
var response map[string]interface{}
s.Require().Eventually(func() bool {
response, err = nodeA.QueryWasmSmartObject(contractAddr, fmt.Sprintf(`{"get_total_funds": {"addr": "%s"}}`, senderBech32))
totalFunds := response["total_funds"].([]interface{})[0]
amount := totalFunds.(map[string]interface{})["amount"].(string)
denom := totalFunds.(map[string]interface{})["denom"].(string)
Copy link
Contributor

Choose a reason for hiding this comment

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

I find the old versions much more readable here (since it's just a test). I'm not strongly against the type assertion checks and get the point in chain code, but in tests I tend to favor making the test shorted since robustness doesn't matter.

For json in particular, we may want to use https://github.com/tidwall/gjson

if err != nil {
return false
}

totalFundsIface, ok := response["total_funds"].([]interface{})
if !ok || len(totalFundsIface) == 0 {
return false
}

totalFunds, ok := totalFundsIface[0].(map[string]interface{})
if !ok {
return false
}

amount, ok := totalFunds["amount"].(string)
if !ok {
return false
}

denom, ok := totalFunds["denom"].(string)
if !ok {
return false
}

// check if denom contains "uosmo"
return err == nil && amount == strconv.FormatInt(transferAmount, 10) && strings.Contains(denom, "ibc")
return amount == strconv.FormatInt(transferAmount, 10) && strings.Contains(denom, "ibc")
},

15*time.Second,
10*time.Millisecond,
)
Expand Down Expand Up @@ -1108,8 +1130,11 @@ func (s *IntegrationTestSuite) TestPacketForwarding() {
if err != nil {
return false
}
count := response["count"].(float64)
return err == nil && count == 0
countValue, ok := response["count"].(float64)
if !ok {
return false
}
return err == nil && countValue == 0
faddat marked this conversation as resolved.
Show resolved Hide resolved
},
15*time.Second,
10*time.Millisecond,
Expand Down
24 changes: 19 additions & 5 deletions tests/ibc-hooks/ibc_middleware_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1049,11 +1049,25 @@ func (suite *HooksTestSuite) TestCrosschainSwaps() {
var responseJson map[string]interface{}
err = json.Unmarshal(res, &responseJson)
suite.Require().NoError(err)
suite.Require().Len(responseJson["sent_amount"].(string), 3) // Not using exact amount in case calculations change
suite.Require().Equal(responseJson["denom"].(string), "token1")
suite.Require().Equal(responseJson["channel_id"].(string), "channel-0")
suite.Require().Equal(responseJson["receiver"].(string), suite.chainB.SenderAccount.GetAddress().String())
suite.Require().Equal(responseJson["packet_sequence"].(float64), 1.0)
sentAmount, ok := responseJson["sent_amount"].(string)
suite.Require().True(ok)
suite.Require().Len(sentAmount, 3) // Not using exact amount in case calculations change

denom, ok := responseJson["denom"].(string)
suite.Require().True(ok)
suite.Require().Equal(denom, "token1")

channelID, ok := responseJson["channel_id"].(string)
suite.Require().True(ok)
suite.Require().Equal(channelID, "channel-0")

receiver, ok := responseJson["receiver"].(string)
suite.Require().True(ok)
suite.Require().Equal(receiver, suite.chainB.SenderAccount.GetAddress().String())

packetSequence, ok := responseJson["packet_sequence"].(float64)
suite.Require().True(ok)
suite.Require().Equal(packetSequence, 1.0)

balanceSender2 := osmosisApp.BankKeeper.GetBalance(suite.chainA.GetContext(), owner, "token0")
suite.Require().Equal(int64(1000), balanceSender.Amount.Sub(balanceSender2.Amount).Int64())
Expand Down
22 changes: 13 additions & 9 deletions x/concentrated-liquidity/lp_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1137,7 +1137,10 @@ func (s *KeeperTestSuite) TestSendCoinsBetweenPoolAndUser() {
// store pool interface
poolI, err := s.App.ConcentratedLiquidityKeeper.GetPoolById(s.Ctx, 1)
s.Require().NoError(err)
concentratedPool := poolI.(types.ConcentratedPoolExtension)
concentratedPool, ok := poolI.(types.ConcentratedPoolExtension)
if !ok {
s.FailNow("poolI is not a ConcentratedPoolExtension")
}

// fund pool address and user address
s.FundAcc(poolI.GetAddress(), sdk.NewCoins(sdk.NewCoin("eth", sdk.NewInt(10000000000000)), sdk.NewCoin("usdc", sdk.NewInt(1000000000000))))
Expand Down Expand Up @@ -1363,21 +1366,22 @@ func (s *KeeperTestSuite) TestUpdatePosition() {
// validate if pool liquidity has been updated properly
poolI, err := s.App.ConcentratedLiquidityKeeper.GetPoolById(s.Ctx, tc.poolId)
s.Require().NoError(err)
concentratedPool := poolI.(types.ConcentratedPoolExtension)
concentratedPool, ok := poolI.(types.ConcentratedPoolExtension)
if !ok {
s.FailNow("poolI is not a ConcentratedPoolExtension")
}
s.Require().Equal(tc.expectedPoolLiquidity, concentratedPool.GetLiquidity())
}
})
}
}

func (s *KeeperTestSuite) TestInitializeInitialPositionForPool() {
var (
sqrt = func(x int64) sdk.Dec {
sqrt, err := sdk.NewDec(x).ApproxSqrt()
s.Require().NoError(err)
return sqrt
}
)
sqrt := func(x int64) sdk.Dec {
sqrt, err := sdk.NewDec(x).ApproxSqrt()
s.Require().NoError(err)
return sqrt
}

type sendTest struct {
amount0Desired sdk.Int
Expand Down
10 changes: 8 additions & 2 deletions x/concentrated-liquidity/pool_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,10 @@ import (
func (s *KeeperTestSuite) TestInitializePool() {
// Create a valid PoolI from a valid ConcentratedPoolExtension
validConcentratedPool := s.PrepareConcentratedPool()
validPoolI := validConcentratedPool.(poolmanagertypes.PoolI)
validPoolI, ok := validConcentratedPool.(poolmanagertypes.PoolI)
if !ok {
s.FailNow("validConcentratedPool is not a PoolI")
}
faddat marked this conversation as resolved.
Show resolved Hide resolved

// Create a concentrated liquidity pool with unauthorized tick spacing
invalidTickSpacing := uint64(25)
Expand Down Expand Up @@ -209,7 +212,10 @@ func (s *KeeperTestSuite) TestPoolIToConcentratedPool() {

// Create default CL pool
concentratedPool := s.PrepareConcentratedPool()
poolI := concentratedPool.(poolmanagertypes.PoolI)
poolI, ok := concentratedPool.(poolmanagertypes.PoolI)
if !ok {
s.Fail("failed to cast pool to PoolI")
faddat marked this conversation as resolved.
Show resolved Hide resolved
}

// Ensure no error occurs when converting to ConcentratedPool
_, err := cl.ConvertPoolInterfaceToConcentrated(poolI)
Expand Down
29 changes: 23 additions & 6 deletions x/gamm/keeper/swap_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -210,20 +210,26 @@ func (suite *KeeperTestSuite) TestCalcOutAmtGivenIn() {
suite.SetupTest()
keeper := suite.App.GAMMKeeper
ctx := suite.Ctx

var pool poolmanagertypes.PoolI
if test.param.poolType == "balancer" {
poolId := suite.PrepareBalancerPool()
poolExt, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, poolId)
suite.NoError(err)
pool = poolExt.(poolmanagertypes.PoolI)
poolI, ok := poolExt.(poolmanagertypes.PoolI)
faddat marked this conversation as resolved.
Show resolved Hide resolved
if !ok {
suite.FailNow("failed to cast pool to poolI")
}
pool = poolI
} else if test.param.poolType == "stableswap" {
poolId := suite.PrepareBasicStableswapPool()
poolExt, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, poolId)
suite.NoError(err)
pool = poolExt.(poolmanagertypes.PoolI)
poolI, ok := poolExt.(poolmanagertypes.PoolI)
if !ok {
suite.FailNow("failed to cast pool to poolI")
}
pool = poolI
}

swapFee := pool.GetSwapFee(suite.Ctx)

_, err := keeper.CalcOutAmtGivenIn(ctx, pool, test.param.tokenIn, test.param.tokenOutDenom, swapFee)
Expand Down Expand Up @@ -283,12 +289,23 @@ func (suite *KeeperTestSuite) TestCalcInAmtGivenOut() {
poolId := suite.PrepareBalancerPool()
poolExt, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, poolId)
suite.NoError(err)
pool = poolExt.(poolmanagertypes.PoolI)
poolI, ok := poolExt.(poolmanagertypes.PoolI)
if !ok {
suite.FailNow("failed to cast pool to poolI")
}

pool = poolI

} else if test.param.poolType == "stableswap" {
poolId := suite.PrepareBasicStableswapPool()
poolExt, err := suite.App.GAMMKeeper.GetPool(suite.Ctx, poolId)
suite.NoError(err)
pool = poolExt.(poolmanagertypes.PoolI)
poolI, ok := poolExt.(poolmanagertypes.PoolI)
if !ok {
suite.FailNow("failed to cast pool to poolI")
}

pool = poolI
}

swapFee := pool.GetSwapFee(suite.Ctx)
Expand Down
11 changes: 7 additions & 4 deletions x/ibc-rate-limit/types/params_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,18 +59,21 @@ func TestValidateParams(t *testing.T) {

for name, tc := range testCases {
t.Run(name, func(t *testing.T) {
addr, ok := tc.addr.(string)
require.True(t, ok, "unexpected type of address")

params := Params{
ContractAddress: tc.addr.(string),
ContractAddress: addr,
}

err := params.Validate()

// Assertions.
if !tc.expected {
require.Error(t, err)
return
} else {
require.NoError(t, err)
}

require.NoError(t, err)
})
}
}
2 changes: 1 addition & 1 deletion x/incentives/keeper/suite_test.go
Original file line number Diff line number Diff line change
@@ -1,8 +1,8 @@
package keeper_test

import (
"crypto/rand"
faddat marked this conversation as resolved.
Show resolved Hide resolved
"fmt"
"math/rand"
"time"

"github.com/osmosis-labs/osmosis/v15/x/incentives/types"
Expand Down
4 changes: 3 additions & 1 deletion x/mint/keeper/keeper_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -236,7 +236,9 @@ func (suite *KeeperTestSuite) TestDistributeMintedCoin() {
suite.Require().NoError(err)

// validate that AfterDistributeMintedCoin hook was called once.
suite.Require().Equal(1, mintKeeper.GetMintHooksUnsafe().(*mintHooksMock).hookCallCount)
hooks, ok := mintKeeper.GetMintHooksUnsafe().(*mintHooksMock)
suite.Require().True(ok, "unexpected type of mint hooks")
suite.Require().Equal(1, hooks.hookCallCount)

// validate distributions to fee collector.
feeCollectorBalanceAmount := bankKeeper.GetBalance(ctx, accountKeeper.GetModuleAddress(authtypes.FeeCollectorName), sdk.DefaultBondDenom).Amount.ToDec()
Expand Down
8 changes: 7 additions & 1 deletion x/poolmanager/client/cli/cli_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -530,7 +530,13 @@ func (s *IntegrationTestSuite) TestNewCreatePoolCmd() {
err = clientCtx.Codec.UnmarshalJSON(out.Bytes(), tc.respType)
s.Require().NoError(err, out.String())

txResp := tc.respType.(*sdk.TxResponse)
var txResp *sdk.TxResponse
switch resp := tc.respType.(type) {
case *sdk.TxResponse:
txResp = resp
default:
s.T().Fatalf("unexpected response type: %T", tc.respType)
}
s.Require().Equal(tc.expectedCode, txResp.Code, out.String())
}
})
Expand Down
4 changes: 2 additions & 2 deletions x/protorev/keeper/protorev_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -102,9 +102,9 @@ func (suite *KeeperTestSuite) TestGetPoolForDenomPair() {

// Should be able to delete all pools for a base denom
suite.App.ProtoRevKeeper.DeleteAllPoolsForBaseDenom(suite.Ctx, "Atom")
pool, err = suite.App.ProtoRevKeeper.GetPoolForDenomPair(suite.Ctx, "Atom", types.OsmosisDenomination)
_, err = suite.App.ProtoRevKeeper.GetPoolForDenomPair(suite.Ctx, "Atom", types.OsmosisDenomination)
suite.Require().Error(err)
pool, err = suite.App.ProtoRevKeeper.GetPoolForDenomPair(suite.Ctx, "Atom", "weth")
_, err = suite.App.ProtoRevKeeper.GetPoolForDenomPair(suite.Ctx, "Atom", "weth")
suite.Require().Error(err)

// Other denoms should still exist
Expand Down
4 changes: 3 additions & 1 deletion x/protorev/types/gov_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,9 @@ func (suite *GovTestSuite) TestEnableProposal() {

for _, tc := range testCases {
proposal := types.NewSetProtoRevEnabledProposal("title", "description", tc.enabled)
suite.Require().Equal(tc.enabled, proposal.(*types.SetProtoRevEnabledProposal).Enabled)
setProtoRevEnabledProposal, ok := proposal.(*types.SetProtoRevEnabledProposal)
suite.Require().True(ok, "proposal is not a SetProtoRevEnabledProposal")
suite.Require().Equal(tc.enabled, setProtoRevEnabledProposal.Enabled)
}
}

Expand Down
16 changes: 8 additions & 8 deletions x/twap/api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ var (
sdk.ZeroDec(), // TODO: choose correct
)

spotPriceError = errors.New("twap: error in pool spot price occurred between start and end time, twap result may be faulty")
errSpotPrice = errors.New("twap: error in pool spot price occurred between start and end time, twap result may be faulty")
)

func (s *TestSuite) TestGetBeginBlockAccumulatorRecord() {
Expand Down Expand Up @@ -311,23 +311,23 @@ func (s *TestSuite) TestGetArithmeticTwap() {
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(baseTime, tPlusOneMin, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectError: spotPriceError,
expectError: errSpotPrice,
expectSpErr: baseTime,
},
"spot price error in record at record time (start time > record time)": {
recordsToSet: []types.TwapRecord{withLastErrTime(baseRecord, baseTime)},
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(tPlusOne, tPlusOneMin, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectError: spotPriceError,
expectError: errSpotPrice,
expectSpErr: baseTime,
},
"spot price error in record after record time": {
recordsToSet: []types.TwapRecord{withLastErrTime(baseRecord, tPlusOne)},
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(baseTime, tPlusOneMin, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectError: spotPriceError,
expectError: errSpotPrice,
expectSpErr: baseTime,
},
// should error, since start time may have been used to interpolate this value
Expand All @@ -336,15 +336,15 @@ func (s *TestSuite) TestGetArithmeticTwap() {
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(baseTime, tPlusOne, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectError: spotPriceError,
expectError: errSpotPrice,
expectSpErr: baseTime,
},
"spot price error exactly at end time": {
recordsToSet: []types.TwapRecord{withLastErrTime(baseRecord, tPlusOne)},
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(baseTime, tPlusOne, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectError: spotPriceError,
expectError: errSpotPrice,
expectSpErr: baseTime,
},
// should not happen, but if it did would error
Expand All @@ -353,7 +353,7 @@ func (s *TestSuite) TestGetArithmeticTwap() {
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(baseTime, tPlusOne, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectError: spotPriceError,
expectError: errSpotPrice,
expectSpErr: baseTime,
},
}
Expand Down Expand Up @@ -754,7 +754,7 @@ func (s *TestSuite) TestGetArithmeticTwapToNow() {
ctxTime: tPlusOneMin,
input: makeSimpleTwapInput(tPlusOne, tPlusOneMin, baseQuoteBA),
expTwap: sdk.NewDec(10),
expectedError: spotPriceError,
expectedError: errSpotPrice,
},
}
for name, test := range tests {
Expand Down