Tests: Contract testing for the procedure AddTimeData and related fixes #15052
Conversation
src/test/timedata_tests.cpp
Outdated
| BOOST_AUTO_TEST_CASE(util_UtilBuildAddress) | ||
| { | ||
| CNetAddr cn1 = UtilBuildAddress(0x001, 0x001, 0x001, 0x0D2); // 1.1.1.210 | ||
|
|
There was a problem hiding this comment.
Drop all these empty lines surrounding single code lines. Applies throughout this PR :-)
src/test/timedata_tests.cpp
Outdated
|
|
||
| BOOST_AUTO_TEST_CASE(util_AddTimeDataAlgorithmComputeOffsetWhenNewSampleCountIsGreaterEqualFiveAndUneven) | ||
| { | ||
| int capacity = 20; // can store up to 20 samples |
There was a problem hiding this comment.
Should be unsigned to match what CMedianFilter expects?
Applies for all int capacity in this file.
src/test/timedata_tests.cpp
Outdated
|
|
||
|
|
||
| for (unsigned int sample = 1; sample < 10; sample++) { // precondition: at least 4 samples. (including the init sample : 0) | ||
| CNetAddr addr = UtilBuildAddress(0x001, 0x001, 0x001, sample); |
There was a problem hiding this comment.
Make the implicit conversion from unsigned int to unsigned char explicit here.
Applies for all CNetAddr addr = UtilBuildAddress(…, sample); in this file.
src/wallet/test/wallet_tests.cpp
Outdated
| { | ||
| // The ComputeTimeSmart function within the wallet reads the current time using the GetAdjustedTime function in timedata. | ||
| // | ||
| // This function computes the time based on the system time (which suports mocking) |
There was a problem hiding this comment.
Should be “supports” :-)
src/wallet/test/wallet_tests.cpp
Outdated
| BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 200, 220), 100); | ||
| SetMockTime(20); | ||
| int64_t newBlockTime = GetAdjustedTime() + 10; | ||
| BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, newBlockTime), clockTime); // time has not changed from the preceeding transaction |
There was a problem hiding this comment.
Should be “preceding” :-)
src/wallet/test/wallet_tests.cpp
Outdated
| // New transaction should use clock time if lower than block time. | ||
| BOOST_CHECK_EQUAL(AddTx(m_wallet, 1, 100, 120), 100); | ||
| SetMockTime(10); | ||
| int64_t clockTime = GetAdjustedTime(); // time + time offset (unfortunately, from a statefull data structure) |
There was a problem hiding this comment.
Should be “stateful” :-)
src/test/timedata_tests.cpp
Outdated
| int capacity = 10; | ||
| int64_t offset = 0; | ||
| std::set<CNetAddr> knownSet; | ||
| CMedianFilter<int64_t> offsetFilter(capacity, 0); // max size : 10 , init samplee: 0 |
There was a problem hiding this comment.
Should be “sample”?
src/test/timedata_tests.cpp
Outdated
|
|
||
| BOOST_CHECK(offset != 0); // offset has changed from the initial value | ||
|
|
||
| assert(offsetFilter.size() == 10); // next sample will be the 11th (uneven) and will trigger a new computation of the offset |
There was a problem hiding this comment.
Why assert(…) and not BOOST_CHECK(…)? Applies throughout this PR.
src/test/timedata_tests.cpp
Outdated
| for (int sample = 1; sample < 4; sample++) { // precondition: 4 samples, all outside bounds | ||
| CNetAddr addr = UtilBuildAddress(0x001, 0x001, 0x001, sample); // 1.1.1.[1,2,3] | ||
| AddTimeDataAlgorithm(addr, 2 * DEFAULT_MAX_TIME_ADJUSTMENT, knownSet, offsetFilter, offset); // offsetSample = 2 * DEFAULT_MAX_TIME_ADJUSTMENT (out of bounds) | ||
|
|
There was a problem hiding this comment.
Drop empty line at end of block.
| Needs rebase |
|
Going to close as "Up for Grabs". @dongcarl You might be interested in having a look here. |
|
Believe #16563 removes the need for this. If so, "up for grabs" label should be removed. |
7 participants
Objective: Make the contract for AddTimeData testable.
The procedure AddTimeData is stateful, so any testing beyond trivialities requires some modification in the code:
Additionally:
wallet_tests.cpp contains errors -> Tests for ComputeTimeSmart falsely assumed that time offset is zero