test: optimize feature asset locks test #6374
Conversation
This results in test time locally going from ~4 minutes to ~3 minutes
src/evo/creditpool.cpp
Outdated
| @@ -164,7 +164,7 @@ CCreditPool CCreditPoolManager::ConstructCreditPool(const CBlockIndex* const blo | |||
| } | |||
|
|
|||
| const CBlockIndex* distant_block_index = block_index; | |||
| for (size_t i = 0; i < CCreditPoolManager::LimitBlocksToTrace; ++i) { | |||
| for (auto i = 0; i < Params().CreditPoolPeriodBlocks(); ++i) { | |||
There was a problem hiding this comment.
better to auto imo
There was a problem hiding this comment.
better to auto imo
I'd disagree with your opinion.
There's so many cases, when auto is useful and increase code readability. Because it makes not only shorter to write but also easier to read. For example, iterators for containers, such as std::vector::const_iterator - If type of container is non-trivial even more useful. Also, if there's any lambda function involved, auto's are so useful to assign lambda or pass somewhere.
But in this particular case, Params().CreditPoolPeriodBlocks(); is not related to type of auto, is only thing is matter is a 0 from the initialization; you should read this code such as auto i = 0. So, this code compiles without warning, but not because you used auto, but because accidentally a type of Params().CreditPoolPeriodBlocks() is matched with `int.
In this particular case, auto makes code more difficult to read: why there's auto?
yes, it compiles. yes, it works as expected. but there's nothing "better"
| @@ -188,6 +189,8 @@ class CChainParams | |||
| int nMinSporkKeys; | |||
| uint16_t nDefaultPlatformP2PPort; | |||
| uint16_t nDefaultPlatformHTTPPort; | |||
| /// The number of blocks the credit pool tracks; 576 (one day) on mainnet, reduced on regtest | |||
There was a problem hiding this comment.
Maybe then apply 100 on all test nets? (Trstnet, devnet, regtest?
There was a problem hiding this comment.
I don't see a point to introduce breaking changes for devnet / testnet over this. Only real benefit is in CI situations imo
PastaPastaPasta
force-pushed
the
optimize-feature-asset-locks-test
branch
from
a153390
to
2d05df0
Compare
Issue being fixed or feature implemented
Minimize number of blocks needed to be mined in asset locks test
Based on CI test goes from about 270s -> 213s
What was done?
Reduce number of blocks needed by reducing hard fork activation points
How Has This Been Tested?
Ran test locally, built
Breaking Changes
Breaking for regtests, nothing else.
Checklist:
Go over all the following points, and put an
xin all the boxes that apply.