misc test improvements #612
Conversation
* 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
Codecov Report
@@ Coverage Diff @@
## main #612 +/- ##
=======================================
Coverage 18.43% 18.43%
=======================================
Files 172 172
Lines 24203 24203
=======================================
Hits 4462 4462
Misses 18977 18977
Partials 764 764
Continue to review full report at Codecov.
|
| @@ -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), | |||
There was a problem hiding this comment.
Do we also want to move broadcast mode flag to osmotestutils and replace them with osmotestutils call
?
There was a problem hiding this comment.
thats a good idea, though I wonder if theres a better way we can just get more of these common flags to all be handled, to better mitigate the boilerplate. (E.g. Just appending all the common flags to the args array)
We should probs open an issue for tracking how to improve this
| // 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 |
There was a problem hiding this comment.
Good catch on this!
One further thing we can do in the future is set a range and simply test if the avgGas and maxGas is within the range, or something similar to remove hard coding gas, considering that there could be further instances where we would have to manually change the hard-coded gas whenever there is a logic change within the method.
There was a problem hiding this comment.
hrmm, thats a good idea. (Basically lower the sensitivity to small noise that we care about)
Depends on our general attitude towards gas, where does tracking small changes matter. I tend to agree with you, at this stage, having small aberrations is totally fine, as long as we can bound the max, and that max and average don't get too far apart.
This PR contains the following misc test improvements:
Description
This osmotestutils should accrue more improvements to testing here. The latency of trying to get improvements into the SDK, then back onto our own repo is far too high.
For contributor use:
docs/) or specification (x/<module>/spec/)Unreleasedsection inCHANGELOG.mdFiles changedin the Github PR explorer