clean up tests and prove this works, also some slight perf improvements

Author: josibake

src/test/btcsignals_tests.cpp

@@ -159,11 +159,8 @@ FIXTURE_TEST_CASE(thread_safety, BasicTestingSetup)
         for (int i = 0; i < 1000; i++) {
             sig0();
         }
-        // Because these calls are purposely happening on both threads at the
-        // same time, these must be asserts rather than CHECKs to prevent
-        // a race inside of CHECK itself (writing to the log).
-        assert(!sig0.empty());
-        assert(conn0.connected());
+        CHECK(!sig0.empty());
+        CHECK(conn0.connected());
     });
 
     std::thread extra_increment_injector([&conn0, &sig0, &val] {

src/test/checkqueue_tests.cpp

@@ -328,7 +328,7 @@ FIXTURE_TEST_CASE(test_CheckQueue_FrozenCleanup, CheckQueueTest)
         std::vector<FrozenCleanupCheck> vChecks(1);
         control.Add(std::move(vChecks));
         auto result = control.Complete(); // Hangs here
-        assert(!result);
+        CHECK(!result);
     });
     {
         std::unique_lock<std::mutex> l(FrozenCleanupCheck::m);

src/test/cuckoocache_tests.cpp

@@ -230,7 +230,10 @@ void test_cache_erase_parallel(size_t bytes)
             size_t end = ntodo*(x+1);
             for (uint32_t i = start; i < end; ++i) {
                 bool contains = set.contains(hashes[i], true);
-                assert(contains);
+                if (!contains) {
+                    CHECK(contains);
+                    break;
+                }
             }
         });
 

src/test/scheduler_tests.cpp

@@ -151,12 +151,12 @@ TEST_CASE(singlethreadedscheduler_ordered)
     for (int i = 0; i < 100; ++i) {
         queue1.insert([i, &counter1]() {
             bool expectation = i == counter1++;
-            assert(expectation);
+            CHECK(expectation);
         });
 
         queue2.insert([i, &counter2]() {
             bool expectation = i == counter2++;
-            assert(expectation);
+            CHECK(expectation);
         });
     }
 

src/test/sock_tests.cpp

@@ -10,7 +10,7 @@
 #include <util/threadinterrupt.h>
 
 #include <test/util/framework.hpp>
-#include <cassert>
+#include <stdexcept>
 #include <thread>
 
 using namespace std::chrono_literals;
@@ -163,15 +163,9 @@ FIXTURE_TEST_CASE(recv_until_terminator_limit, BasicTestingSetup)
 
     std::thread receiver([&socks, &timeout, &interrupt]() {
         constexpr size_t max_data{10};
-        bool threw_as_expected{false};
-        // CHECK_EXCEPTION() writes to some variables shared with the main thread which
-        // creates a data race. So mimic it manually.
-        try {
-            (void)socks.receiver.RecvUntilTerminator('\n', timeout, interrupt, max_data);
-        } catch (const std::runtime_error& e) {
-            threw_as_expected = HasReason("too many bytes without a terminator")(e);
-        }
-        assert(threw_as_expected);
+        CHECK_EXCEPTION((void)socks.receiver.RecvUntilTerminator('\n', timeout, interrupt, max_data),
+                        std::runtime_error,
+                        HasReason("too many bytes without a terminator"));
     });
 
     REQUIRE_NOTHROW(socks.sender.SendComplete("1234567", timeout, interrupt));

src/test/validation_block_tests.cpp

@@ -206,7 +206,7 @@ FIXTURE_TEST_CASE(processnewblock_signals_ordering, MinerTestingSetup)
             for (const auto& block : blocks) {
                 if (block->vtx.size() == 1) {
                     bool processed = ProcessNewBlock(*Assert(m_node.chainman), block, true, true, &ignored);
-                    assert(processed);
+                    CHECK(processed);
                 }
             }
         });
@@ -322,12 +322,16 @@ FIXTURE_TEST_CASE(mempool_locks_reorg, MinerTestingSetup)
                 // be atomic. So the caller assumes that the returned mempool
                 // is consistent. That is, it has all txs that were there
                 // before the reorg.
-                assert(m_node.mempool->size() == txs.size());
+                const auto mempool_size{m_node.mempool->size()};
+                if (mempool_size != txs.size()) {
+                    CHECK(mempool_size == txs.size());
+                    break;
+                }
                 continue;
             }
             LOCK(cs_main);
             // We are done with the reorg, so the tip must have changed
-            assert(tip_init != m_node.chainman->ActiveChain().Tip()->GetBlockHash());
+            CHECK(tip_init != m_node.chainman->ActiveChain().Tip()->GetBlockHash());
         }};
 
         // Submit the reorg in this thread to invalidate and remove the txs from the tx pool