[test] Add CCoinsViewCache Access/Modify/Write tests #9308

pull ryanofsky wants to merge 1 commits into bitcoin:master from ryanofsky:pr/coins-test changing 1 files +364 −0

ryanofsky commented at 3:36 PM on December 9, 2016: member

Add more comprehensive unit tests for CCoinsViewCache. Right now it is hard to refactor caching code or fix bugs in the caching logic because you have to try to mentally enumerate all the different states the cache might be in to make sure a change doesn't cause unintended consequences. The new tests explicitly enumerate relevant cache states, documenting and verifying the behavior in each state, so it will be safer and easier to make changes to the caching code in the future.

[test] Add CCoinsViewCache Access/Modify/Write tests

Add more comprehensive unit tests for CCoinsViewCache. Right now it is hard to
refactor caching code or fix bugs in the caching logic because you have to try
to mentally enumerate all the different states the cache might be in to make
sure a change doesn't cause unintended consequences. The new tests explicitly
enumerate relevant cache states, documenting and verifying the behavior in each
state, so it will be safer and easier to make changes to the caching code in
the future.

07df40babb

ryanofsky force-pushed on Dec 9, 2016
fanquake added the label Tests on Dec 10, 2016
laanwj commented at 9:29 AM on December 14, 2016: member

utACK https://github.com/bitcoin/bitcoin/pull/9308/commits/03ffa4cfff29d8273859edb030bc0eb823b26e7b, thanks for adding tests for this important system
MarcoFalke commented at 11:09 AM on December 14, 2016: member

Concept ACK 03ffa4cfff29d8273859edb030bc0eb823b26e7b

in src/test/coins_tests.cpp:None in 03ffa4cfff outdated

 622 | +    CheckModifyCoins(VALUE1, VALUE2, VALUE3, VALUE3, DIRTY|FRESH, DIRTY|FRESH);
 623 | +}
 624 | +
 625 | +void CheckModifyNewCoins(CAmount cache_value, CAmount modify_value, CAmount expected_value, char cache_flags, char expected_flags, bool coinbase)
 626 | +{
 627 | +    SingleEntryCacheTest test(ABSENT, cache_value, cache_flags);

morcos commented at 6:06 PM on December 19, 2016:

Maybe loop over possibilities for base values

ryanofsky commented at 8:09 PM on December 19, 2016:

Added.

in src/test/coins_tests.cpp:None in 03ffa4cfff outdated

 693 | +    GetCoinsMapEntry(test.cache.map(), result_value, result_flags);
 694 | +    BOOST_CHECK_EQUAL(result_value, expected_value);
 695 | +    BOOST_CHECK_EQUAL(result_flags, expected_flags);
 696 | +}
 697 | +
 698 | +BOOST_AUTO_TEST_CASE(ccoins_write)

morcos commented at 7:36 PM on December 19, 2016:

Maybe try a loop of these tests with Child Flags not DIRTY as well

ryanofsky commented at 8:09 PM on December 19, 2016:

Added.

morcos commented at 7:36 PM on December 19, 2016: member

ACK (left a couple of comments to slightly increase test coverage)
ryanofsky force-pushed on Dec 19, 2016
laanwj merged this on Dec 21, 2016
laanwj closed this on Dec 21, 2016
laanwj referenced this in commit 0698639a38 on Dec 21, 2016
paveljanik commented at 10:43 AM on December 27, 2016: contributor
@ryanofsky What about this warning?
```
test/coins_tests.cpp:432:19: warning: unused variable 'DIRTY_FLAGS' [-Wunused-const-variable]
```
Do you plan to use it or can it be removed?
ryanofsky referenced this in commit ef4a78a691 on Dec 27, 2016
ryanofsky commented at 12:42 PM on December 27, 2016: member

I do have some debug code using it, which I might make a PR from later, but its better to remove for now since it's causing this warning. Created to #9435 to remove.
ryanofsky referenced this in commit 35356b40ef on Dec 27, 2016
jonasschnelli referenced this in commit 675f31ee3f on Jan 2, 2017
ryanofsky referenced this in commit 0fb539b011 on Jan 2, 2017
codablock referenced this in commit 8afa568853 on Sep 27, 2017
codablock referenced this in commit 63f35282ec on Oct 12, 2017
codablock referenced this in commit dc2741ca4b on Oct 23, 2017
codablock referenced this in commit 4b4d222933 on Oct 23, 2017
UdjinM6 referenced this in commit 8cd56f195b on Nov 8, 2017
lateminer referenced this in commit 2f54476d17 on Jan 4, 2019
random-zebra referenced this in commit e9e61fdbda on Aug 9, 2020
random-zebra referenced this in commit 69b0a66147 on Aug 9, 2020
random-zebra referenced this in commit 9e03e67a5b on Aug 9, 2020
random-zebra referenced this in commit 4bd47d64ad on Aug 10, 2020
random-zebra referenced this in commit 46997cb7a8 on Aug 11, 2020
random-zebra referenced this in commit cdbb1a6162 on Aug 12, 2020
random-zebra referenced this in commit 7181a9a4b0 on Aug 12, 2020
random-zebra referenced this in commit af793b7bb9 on Aug 18, 2020
DrahtBot locked this on Sep 8, 2021

Contributors

Labels

Linked (view graph)

#9435 Removed unused variable in test, fixing warning.