merge bitcoin#23477: tidy up unit tests

src/addrman.h

@@ -64,6 +64,7 @@ public:
  */
 class AddrMan
 {
+protected:
     const std::unique_ptr<AddrManImpl> m_impl;
 
 public:
@@ -149,9 +150,6 @@ public:
     AddrInfo GetAddressInfo(const CService& addr);
 
     const std::vector<bool>& GetAsmap() const;
-
-    friend class AddrManTest;
-    friend class AddrManDeterministic;
 };
 
 #endif // BITCOIN_ADDRMAN_H

src/test/addrman_tests.cpp

@@ -20,81 +20,20 @@
 
 using namespace std::literals;
 
-class AddrManSerializationMock : public AddrMan
-{
-public:
-    virtual void Serialize(CDataStream& s) const = 0;
-
-    AddrManSerializationMock()
-        : AddrMan(/* asmap */ std::vector<bool>(), /* deterministic */ true, /* consistency_check_ratio */ 100)
-    {}
-};
-
-class AddrManUncorrupted : public AddrManSerializationMock
-{
-public:
-    void Serialize(CDataStream& s) const override
-    {
-        AddrMan::Serialize(s);
-    }
-};
-
-class AddrManCorrupted : public AddrManSerializationMock
-{
-public:
-    void Serialize(CDataStream& s) const override
-    {
-        // Produces corrupt output that claims addrman has 20 addrs when it only has one addr.
-        unsigned char nVersion = 1;
-        s << nVersion;
-        s << ((unsigned char)32);
-        s << uint256::ONE;
-        s << 10; // nNew
-        s << 10; // nTried
-
-        int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
-        s << nUBuckets;
-
-        CService serv;
-        BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false));
-        CAddress addr = CAddress(serv, NODE_NONE);
-        CNetAddr resolved;
-        BOOST_CHECK(LookupHost("252.2.2.2", resolved, false));
-        AddrInfo info = AddrInfo(addr, resolved);
-        s << info;
-    }
-};
-
-static CDataStream AddrmanToStream(const AddrManSerializationMock& _addrman)
-{
-    CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION);
-    ssPeersIn << Params().MessageStart();
-    ssPeersIn << _addrman;
-    std::string str = ssPeersIn.str();
-    std::vector<unsigned char> vchData(str.begin(), str.end());
-    return CDataStream(vchData, SER_DISK, CLIENT_VERSION);
-}
-
 class AddrManTest : public AddrMan
 {
-private:
-    bool deterministic;
-
 public:
-    explicit AddrManTest(bool makeDeterministic = true,
+    explicit AddrManTest(std::vector<bool> asmap = std::vector<bool>())
-                         std::vector<bool> asmap = std::vector<bool>())
+        : AddrMan(asmap, /*deterministic=*/true, /* consistency_check_ratio */ 100)
-        : AddrMan(asmap, makeDeterministic, /* consistency_check_ratio */ 100)
+    {}
-    {
-        deterministic = makeDeterministic;
-    }
 
-    AddrInfo* Find(const CService& addr, int* pnId = nullptr)
+    AddrInfo* Find(const CService& addr)
     {
         LOCK(m_impl->cs);
-        return m_impl->Find(addr, pnId);
+        return m_impl->Find(addr);
     }
 
-    AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId = nullptr)
+    AddrInfo* Create(const CAddress& addr, const CNetAddr& addrSource, int* pnId)
     {
         LOCK(m_impl->cs);
         return m_impl->Create(addr, addrSource, pnId);
@@ -758,8 +697,8 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
 {
     std::vector<bool> asmap1 = FromBytes(raw_tests::asmap, sizeof(raw_tests::asmap) * 8);
 
-    auto addrman_asmap1 = std::make_unique<AddrManTest>(true, asmap1);
+    auto addrman_asmap1 = std::make_unique<AddrManTest>(asmap1);
-    auto addrman_asmap1_dup = std::make_unique<AddrManTest>(true, asmap1);
+    auto addrman_asmap1_dup = std::make_unique<AddrManTest>(asmap1);
     auto addrman_noasmap = std::make_unique<AddrManTest>();
     CDataStream stream(SER_NETWORK, PROTOCOL_VERSION);
 
@@ -790,7 +729,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     BOOST_CHECK(bucketAndEntry_asmap1.second != bucketAndEntry_noasmap.second);
 
     // deserializing non-asmaped peers.dat to asmaped addrman
-    addrman_asmap1 = std::make_unique<AddrManTest>(true, asmap1);
+    addrman_asmap1 = std::make_unique<AddrManTest>(asmap1);
     addrman_noasmap = std::make_unique<AddrManTest>();
     addrman_noasmap->Add({addr}, default_source);
     stream << *addrman_noasmap;
@@ -802,7 +741,7 @@ BOOST_AUTO_TEST_CASE(addrman_serialization)
     BOOST_CHECK(bucketAndEntry_asmap1_deser.second == bucketAndEntry_asmap1_dup.second);
 
     // used to map to different buckets, now maps to the same bucket.
-    addrman_asmap1 = std::make_unique<AddrManTest>(true, asmap1);
+    addrman_asmap1 = std::make_unique<AddrManTest>(asmap1);
     addrman_noasmap = std::make_unique<AddrManTest>();
     CAddress addr1 = CAddress(ResolveService("250.1.1.1"), NODE_NONE);
     CAddress addr2 = CAddress(ResolveService("250.2.1.1"), NODE_NONE);
@@ -1002,9 +941,20 @@ BOOST_AUTO_TEST_CASE(addrman_evictionworks)
     BOOST_CHECK(addrman.SelectTriedCollision().first.ToString() == "[::]:0");
 }
 
+static CDataStream AddrmanToStream(const AddrMan& addrman)
+{
+    CDataStream ssPeersIn(SER_DISK, CLIENT_VERSION);
+    ssPeersIn << Params().MessageStart();
+    ssPeersIn << addrman;
+    std::string str = ssPeersIn.str();
+    std::vector<unsigned char> vchData(str.begin(), str.end());
+    return CDataStream(vchData, SER_DISK, CLIENT_VERSION);
+}
+
 BOOST_AUTO_TEST_CASE(load_addrman)
 {
-    AddrManUncorrupted addrmanUncorrupted;
+    AddrMan addrman{/*asmap=*/ std::vector<bool>(), /*deterministic=*/ true,
+                    /*consistency_check_ratio=*/ 100};
 
     CService addr1, addr2, addr3;
     BOOST_CHECK(Lookup("250.7.1.1", addr1, 8333, false));
@@ -1017,11 +967,11 @@ BOOST_AUTO_TEST_CASE(load_addrman)
     CService source;
     BOOST_CHECK(Lookup("252.5.1.1", source, 8333, false));
     std::vector<CAddress> addresses{CAddress(addr1, NODE_NONE), CAddress(addr2, NODE_NONE), CAddress(addr3, NODE_NONE)};
-    BOOST_CHECK(addrmanUncorrupted.Add(addresses, source));
+    BOOST_CHECK(addrman.Add(addresses, source));
-    BOOST_CHECK(addrmanUncorrupted.size() == 3);
+    BOOST_CHECK(addrman.size() == 3);
 
     // Test that the de-serialization does not throw an exception.
-    CDataStream ssPeers1 = AddrmanToStream(addrmanUncorrupted);
+    CDataStream ssPeers1 = AddrmanToStream(addrman);
     bool exceptionThrown = false;
     AddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
 
@@ -1038,7 +988,7 @@ BOOST_AUTO_TEST_CASE(load_addrman)
     BOOST_CHECK(exceptionThrown == false);
 
     // Test that ReadFromStream creates an addrman with the correct number of addrs.
-    CDataStream ssPeers2 = AddrmanToStream(addrmanUncorrupted);
+    CDataStream ssPeers2 = AddrmanToStream(addrman);
 
     AddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
     BOOST_CHECK(addrman2.size() == 0);
@@ -1046,13 +996,39 @@ BOOST_AUTO_TEST_CASE(load_addrman)
     BOOST_CHECK(addrman2.size() == 3);
 }
 
+// Produce a corrupt peers.dat that claims 20 addrs when it only has one addr.
+static CDataStream MakeCorruptPeersDat()
+{
+    CDataStream s(SER_DISK, CLIENT_VERSION);
+    s << ::Params().MessageStart();
+
+    unsigned char nVersion = 1;
+    s << nVersion;
+    s << ((unsigned char)32);
+    s << uint256::ONE;
+    s << 10; // nNew
+    s << 10; // nTried
+
+    int nUBuckets = ADDRMAN_NEW_BUCKET_COUNT ^ (1 << 30);
+    s << nUBuckets;
+
+    CService serv;
+    BOOST_CHECK(Lookup("252.1.1.1", serv, 7777, false));
+    CAddress addr = CAddress(serv, NODE_NONE);
+    CNetAddr resolved;
+    BOOST_CHECK(LookupHost("252.2.2.2", resolved, false));
+    AddrInfo info = AddrInfo(addr, resolved);
+    s << info;
+
+    std::string str = s.str();
+    std::vector<unsigned char> vchData(str.begin(), str.end());
+    return CDataStream(vchData, SER_DISK, CLIENT_VERSION);
+}
 
 BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
 {
-    AddrManCorrupted addrmanCorrupted;
+    // Test that the de-serialization of corrupted peers.dat throws an exception.
-
+    CDataStream ssPeers1 = MakeCorruptPeersDat();
-    // Test that the de-serialization of corrupted addrman throws an exception.
-    CDataStream ssPeers1 = AddrmanToStream(addrmanCorrupted);
     bool exceptionThrown = false;
     AddrMan addrman1(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
     BOOST_CHECK(addrman1.size() == 0);
@@ -1068,7 +1044,7 @@ BOOST_AUTO_TEST_CASE(load_addrman_corrupted)
     BOOST_CHECK(exceptionThrown);
 
     // Test that ReadFromStream fails if peers.dat is corrupt
-    CDataStream ssPeers2 = AddrmanToStream(addrmanCorrupted);
+    CDataStream ssPeers2 = MakeCorruptPeersDat();
 
     AddrMan addrman2(/* asmap */ std::vector<bool>(), /* deterministic */ false, /* consistency_check_ratio */ 100);
     BOOST_CHECK(addrman2.size() == 0);