Skip to content

Commit

Permalink
[backport#14935] tests: Test for expected return values when calling …
Browse files Browse the repository at this point in the history 
…functions returning a success code

Summary:
c84c2b8c92 tests: Test for expected return values when calling functions returning a success code (practicalswift)

Pull request description:

  Test for expected return values when calling functions returning a success code (instead of discarding the return values).

  **Note to reviewers:** The following commands can be used to verify that the only text fragments added in this PR are `BOOST_CHECK(`, `!` and `)` :

  ```
  $ git diff HEAD~1 | grep -E '^[\-][^\-]' | cut -b2- > before.txt
  $ git diff HEAD~1 | grep -E '^[\+][^\+]' | cut -b2- > after.txt
  $ cat after.txt | sed 's/BOOST_CHECK(//g' | sed 's/));/);/g' | tr -d '!' > after-sed.txt
  $ diff -u before.txt after-sed.txt
  $
  ```
---

Backport of Core [[bitcoin/bitcoin#14935 | PR14935]] and [[bitcoin/bitcoin#14969 | PR14969]]

Test Plan:
  ninja check

Reviewers: #bitcoin_abc, Fabien

Reviewed By: #bitcoin_abc, Fabien

Subscribers: Fabien

Differential Revision: https://reviews.bitcoinabc.org/D6663
  • Loading branch information
@majcosta
MarcoFalke authored and majcosta committed Jun 22, 2020
1 parent 06b3ec6 commit a06ba8d
Show file tree
Hide file tree
Showing 15 changed files with 131 additions and 110 deletions.
72 changes: 39 additions & 33 deletions src/test/addrman_tests.cpp
View file
Expand Up @@ -137,11 +137,11 @@ BOOST_AUTO_TEST_CASE(addrman_ports) {

// Test: Addr with same IP but diff port does not replace existing addr.
CService addr1 = ResolveService("250.1.1.1", 8333);
addrman.Add(CAddress(addr1, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 1U);

CService addr1_port = ResolveService("250.1.1.1", 8334);
addrman.Add(CAddress(addr1_port, NODE_NONE), source);
BOOST_CHECK(!addrman.Add(CAddress(addr1_port, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 1U);
CAddrInfo addr_ret2 = addrman.Select();
BOOST_CHECK_EQUAL(addr_ret2.ToString(), "250.1.1.1:8333");
Expand All @@ -162,7 +162,7 @@ BOOST_AUTO_TEST_CASE(addrman_select) {

// Test: Select from new with 1 addr in new.
CService addr1 = ResolveService("250.1.1.1", 8333);
addrman.Add(CAddress(addr1, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 1U);

bool newOnly = true;
Expand All @@ -185,20 +185,26 @@ BOOST_AUTO_TEST_CASE(addrman_select) {
CService addr3 = ResolveService("250.3.2.2", 9999);
CService addr4 = ResolveService("250.3.3.3", 9999);

addrman.Add(CAddress(addr2, NODE_NONE), ResolveService("250.3.1.1", 8333));
addrman.Add(CAddress(addr3, NODE_NONE), ResolveService("250.3.1.1", 8333));
addrman.Add(CAddress(addr4, NODE_NONE), ResolveService("250.4.1.1", 8333));
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE),
ResolveService("250.3.1.1", 8333)));
BOOST_CHECK(addrman.Add(CAddress(addr3, NODE_NONE),
ResolveService("250.3.1.1", 8333)));
BOOST_CHECK(addrman.Add(CAddress(addr4, NODE_NONE),
ResolveService("250.4.1.1", 8333)));

// Add three addresses to tried table.
CService addr5 = ResolveService("250.4.4.4", 8333);
CService addr6 = ResolveService("250.4.5.5", 7777);
CService addr7 = ResolveService("250.4.6.6", 8333);

addrman.Add(CAddress(addr5, NODE_NONE), ResolveService("250.3.1.1", 8333));
BOOST_CHECK(addrman.Add(CAddress(addr5, NODE_NONE),
ResolveService("250.3.1.1", 8333)));
addrman.Good(CAddress(addr5, NODE_NONE));
addrman.Add(CAddress(addr6, NODE_NONE), ResolveService("250.3.1.1", 8333));
BOOST_CHECK(addrman.Add(CAddress(addr6, NODE_NONE),
ResolveService("250.3.1.1", 8333)));
addrman.Good(CAddress(addr6, NODE_NONE));
addrman.Add(CAddress(addr7, NODE_NONE), ResolveService("250.1.1.3", 8333));
BOOST_CHECK(addrman.Add(CAddress(addr7, NODE_NONE),
ResolveService("250.1.1.3", 8333)));
addrman.Good(CAddress(addr7, NODE_NONE));

// Test: 6 addrs + 1 addr from last test = 7.
Expand All @@ -221,19 +227,19 @@ BOOST_AUTO_TEST_CASE(addrman_new_collisions) {

for (unsigned int i = 1; i < 18; i++) {
CService addr = ResolveService("250.1.1." + std::to_string(i));
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));

// Test: No collision in new table yet.
BOOST_CHECK_EQUAL(addrman.size(), i);
}

// Test: new table collision!
CService addr1 = ResolveService("250.1.1.18");
addrman.Add(CAddress(addr1, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 17U);

CService addr2 = ResolveService("250.1.1.19");
addrman.Add(CAddress(addr2, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 18U);
}

Expand All @@ -246,7 +252,7 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) {

for (unsigned int i = 1; i < 80; i++) {
CService addr = ResolveService("250.1.1." + std::to_string(i));
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(CAddress(addr, NODE_NONE));

// Test: No collision in tried table yet.
Expand All @@ -255,11 +261,11 @@ BOOST_AUTO_TEST_CASE(addrman_tried_collisions) {

// Test: tried table collision!
CService addr1 = ResolveService("250.1.1.80");
addrman.Add(CAddress(addr1, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr1, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 79U);

CService addr2 = ResolveService("250.1.1.81");
addrman.Add(CAddress(addr2, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr2, NODE_NONE), source));
BOOST_CHECK_EQUAL(addrman.size(), 80U);
}

Expand All @@ -275,9 +281,9 @@ BOOST_AUTO_TEST_CASE(addrman_find) {
CNetAddr source1 = ResolveIP("250.1.2.1");
CNetAddr source2 = ResolveIP("250.1.2.2");

addrman.Add(addr1, source1);
addrman.Add(addr2, source2);
addrman.Add(addr3, source1);
BOOST_CHECK(addrman.Add(addr1, source1));
BOOST_CHECK(!addrman.Add(addr2, source2));
BOOST_CHECK(addrman.Add(addr3, source1));

// Test: ensure Find returns an IP matching what we searched on.
CAddrInfo *info1 = addrman.Find(addr1);
Expand Down Expand Up @@ -355,11 +361,11 @@ BOOST_AUTO_TEST_CASE(addrman_getaddr) {
CNetAddr source2 = ResolveIP("250.2.3.3");

// Test: Ensure GetAddr works with new addresses.
addrman.Add(addr1, source1);
addrman.Add(addr2, source2);
addrman.Add(addr3, source1);
addrman.Add(addr4, source2);
addrman.Add(addr5, source1);
BOOST_CHECK(addrman.Add(addr1, source1));
BOOST_CHECK(addrman.Add(addr2, source2));
BOOST_CHECK(addrman.Add(addr3, source1));
BOOST_CHECK(addrman.Add(addr4, source2));
BOOST_CHECK(addrman.Add(addr5, source1));

// GetAddr returns 23% of addresses, 23% of 5 is 1 rounded down.
BOOST_CHECK_EQUAL(addrman.GetAddr().size(), 1U);
Expand Down Expand Up @@ -522,7 +528,7 @@ BOOST_AUTO_TEST_CASE(addrman_selecttriedcollision) {
CNetAddr source = ResolveIP("252.2.2.2");
for (unsigned int i = 1; i < 23; i++) {
CService addr = ResolveService("250.1.1." + std::to_string(i));
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(addr);

// No collisions yet.
Expand All @@ -547,7 +553,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) {
CNetAddr source = ResolveIP("252.2.2.2");
for (unsigned int i = 1; i < 23; i++) {
CService addr = ResolveService("250.1.1." + std::to_string(i));
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(addr);

// No collision yet.
Expand All @@ -557,7 +563,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) {

// Collision between 23 and 19.
CService addr23 = ResolveService("250.1.1.23");
addrman.Add(CAddress(addr23, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr23, NODE_NONE), source));
addrman.Good(addr23);

BOOST_CHECK(addrman.size() == 23);
Expand All @@ -570,7 +576,7 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) {
// Lets create two collisions.
for (unsigned int i = 24; i < 33; i++) {
CService addr = ResolveService("250.1.1." + std::to_string(i));
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(addr);

BOOST_CHECK(addrman.size() == i);
Expand All @@ -579,14 +585,14 @@ BOOST_AUTO_TEST_CASE(addrman_noevict) {

// Cause a collision.
CService addr33 = ResolveService("250.1.1.33");
addrman.Add(CAddress(addr33, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr33, NODE_NONE), source));
addrman.Good(addr33);
BOOST_CHECK(addrman.size() == 33);

BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.27:0");

// Cause a second collision.
addrman.Add(CAddress(addr23, NODE_NONE), source);
BOOST_CHECK(!addrman.Add(CAddress(addr23, NODE_NONE), source));
addrman.Good(addr23);
BOOST_CHECK(addrman.size() == 33);

Expand All @@ -607,7 +613,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) {
CNetAddr source = ResolveIP("252.2.2.2");
for (unsigned int i = 1; i < 23; i++) {
CService addr = ResolveService("250.1.1." + std::to_string(i));
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(addr);

// No collision yet.
Expand All @@ -617,7 +623,7 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) {

// Collision between 23 and 19.
CService addr = ResolveService("250.1.1.23");
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(addr);

BOOST_CHECK(addrman.size() == 23);
Expand All @@ -632,14 +638,14 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks) {
BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");

// If 23 was swapped for 19, then this should cause no collisions.
addrman.Add(CAddress(addr, NODE_NONE), source);
BOOST_CHECK(!addrman.Add(CAddress(addr, NODE_NONE), source));
addrman.Good(addr);

BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "[::]:0");

// If we insert 19 is should collide with 23.
CService addr19 = ResolveService("250.1.1.19");
addrman.Add(CAddress(addr19, NODE_NONE), source);
BOOST_CHECK(!addrman.Add(CAddress(addr19, NODE_NONE), source));
addrman.Good(addr19);

BOOST_CHECK(addrman.SelectTriedCollision().ToString() == "250.1.1.23:0");
Expand Down
4 changes: 3 additions & 1 deletion src/test/checkqueue_tests.cpp
View file
Expand Up @@ -329,7 +329,9 @@ BOOST_AUTO_TEST_CASE(test_CheckQueue_FrozenCleanup) {
// would get called twice).
vChecks[0].should_freeze = true;
control.Add(vChecks);
control.Wait(); // Hangs here
// Hangs here
bool waitResult = control.Wait();
assert(waitResult);
});
{
std::unique_lock<std::mutex> l(FrozenCleanupCheck::m);
Expand Down
14 changes: 7 additions & 7 deletions src/test/coins_tests.cpp
View file
Expand Up @@ -189,7 +189,7 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) {
} else {
removed_an_entry = true;
coin.Clear();
stack.back()->SpendCoin(COutPoint(txid, 0));
BOOST_CHECK(stack.back()->SpendCoin(COutPoint(txid, 0)));
}
}

Expand Down Expand Up @@ -225,14 +225,14 @@ BOOST_AUTO_TEST_CASE(coins_cache_simulation_test) {
if (InsecureRandRange(100) == 0) {
if (stack.size() > 1 && InsecureRandBool() == 0) {
unsigned int flushIndex = InsecureRandRange(stack.size() - 1);
stack[flushIndex]->Flush();
BOOST_CHECK(stack[flushIndex]->Flush());
}
}
if (InsecureRandRange(100) == 0) {
// Every 100 iterations, change the cache stack.
if (stack.size() > 0 && InsecureRandBool() == 0) {
// Remove the top cache
stack.back()->Flush();
BOOST_CHECK(stack.back()->Flush());
delete stack.back();
stack.pop_back();
}
Expand Down Expand Up @@ -431,7 +431,7 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) {
// Disconnect the tx from the current UTXO
// See code in DisconnectBlock
// remove outputs
stack.back()->SpendCoin(utxod->first);
BOOST_CHECK(stack.back()->SpendCoin(utxod->first));

// restore inputs
if (!tx.IsCoinBase()) {
Expand Down Expand Up @@ -478,13 +478,13 @@ BOOST_AUTO_TEST_CASE(updatecoins_simulation_test) {
// Every 100 iterations, flush an intermediate cache
if (stack.size() > 1 && InsecureRandBool() == 0) {
unsigned int flushIndex = InsecureRandRange(stack.size() - 1);
stack[flushIndex]->Flush();
BOOST_CHECK(stack[flushIndex]->Flush());
}
}
if (InsecureRandRange(100) == 0) {
// Every 100 iterations, change the cache stack.
if (stack.size() > 0 && InsecureRandBool() == 0) {
stack.back()->Flush();
BOOST_CHECK(stack.back()->Flush());
delete stack.back();
stack.pop_back();
}
Expand Down Expand Up @@ -630,7 +630,7 @@ void GetCoinMapEntry(const CCoinsMap &map, Amount &value, char &flags) {
void WriteCoinViewEntry(CCoinsView &view, const Amount value, char flags) {
CCoinsMap map;
InsertCoinMapEntry(map, value, flags);
view.BatchWrite(map, BlockHash());
BOOST_CHECK(view.BatchWrite(map, BlockHash()));
}

class SingleEntryCacheTest {
Expand Down
5 changes: 3 additions & 2 deletions src/test/cuckoocache_tests.cpp
View file
Expand Up @@ -233,7 +233,7 @@ template <typename Cache> static void test_cache_erase(size_t megabytes) {
}
/** Erase the first quarter */
for (uint32_t i = 0; i < (n_insert / 4); ++i) {
set.contains(hashes[i], true);
BOOST_CHECK(set.contains(hashes[i], true));
}
/** Insert the second half */
for (uint32_t i = (n_insert / 2); i < n_insert; ++i) {
Expand Down Expand Up @@ -318,7 +318,8 @@ static void test_cache_erase_parallel(size_t megabytes) {
size_t start = ntodo * x;
size_t end = ntodo * (x + 1);
for (uint32_t i = start; i < end; ++i) {
set.contains(hashes[i], true);
bool contains = set.contains(hashes[i], true);
assert(contains);
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion src/test/dbwrapper_tests.cpp
View file
Expand Up @@ -71,7 +71,7 @@ BOOST_AUTO_TEST_CASE(dbwrapper_batch) {
// Remove key3 before it's even been written
batch.Erase(key3);

dbw.WriteBatch(batch);
BOOST_CHECK(dbw.WriteBatch(batch));

BOOST_CHECK(dbw.Read(key, res));
BOOST_CHECK_EQUAL(res.ToString(), in.ToString());
Expand Down

0 comments on commit a06ba8d

Please sign in to comment.