test: improve handwritten test doubles

Author: johnny9

test/mocks/callcounter.h

@@ -0,0 +1,28 @@
+// Copyright (c) 2026 The Bitcoin Core developers
+// Distributed under the MIT software license, see the accompanying
+// file COPYING or http://www.opensource.org/licenses/mit-license.php.
+
+#ifndef BITCOIN_QML_TEST_MOCKS_CALLCOUNTER_H
+#define BITCOIN_QML_TEST_MOCKS_CALLCOUNTER_H
+
+#include <atomic>
+#include <string_view>
+
+class CallCounter
+{
+public:
+    explicit CallCounter(std::string_view name) : m_name{name} {}
+
+    CallCounter(const CallCounter&) = delete;
+    CallCounter& operator=(const CallCounter&) = delete;
+
+    int operator++() { return m_calls.fetch_add(1) + 1; }
+    int load() const { return m_calls.load(); }
+    std::string_view Name() const { return m_name; }
+
+private:
+    std::atomic<int> m_calls{0};
+    const std::string_view m_name;
+};
+
+#endif // BITCOIN_QML_TEST_MOCKS_CALLCOUNTER_H

test/mocks/mocknode.h

@@ -7,13 +7,13 @@
 
 #include <interfaces/wallet.h>
 #include <outputtype.h>
+#include <test/mocks/callcounter.h>
 #include <wallet/coincontrol.h>
 #include <wallet/types.h>
 #include <wallet/wallet.h>
 
 #include <QtTest/qtestcase.h>
 
-#include <atomic>
 #include <functional>
 #include <source_location>
 #include <sstream>
@@ -88,54 +88,63 @@ class StubWallet : public interfaces::Wallet
     std::unique_ptr<interfaces::Handler> handleCanGetAddressesChanged(CanGetAddressesChangedFn) override { return {}; }
 };
 
+/**
+ * Configurable wallet test double with permissive defaults.
+ *
+ * Unconfigured methods return their StubWallet values. Tests register explicit
+ * expectations for calls that are part of the behavior under test.
+ */
 class MockWallet : public StubWallet
 {
 public:
     class Verification
     {
     public:
-        Verification(MockWallet& wallet, std::source_location where)
-            : m_wallet{&wallet}, m_where{where}
+        explicit Verification(MockWallet& wallet)
+            : m_wallet{&wallet}
         {
         }
 
         Verification(const Verification&) = delete;
         Verification& operator=(const Verification&) = delete;
         Verification(Verification&& other) noexcept
-            : m_wallet{std::exchange(other.m_wallet, nullptr)}, m_where{other.m_where}
+            : m_wallet{std::exchange(other.m_wallet, nullptr)}
         {
         }
 
         ~Verification()
         {
             if (!m_wallet) return;
-            const std::string failure{m_wallet->CallExpectationFailure()};
-            if (!failure.empty()) {
-                QTest::qFail(failure.c_str(), m_where.file_name(), static_cast<int>(m_where.line()));
+            for (const auto& failure : m_wallet->VerificationFailures()) {
+                QTest::qFail(failure.message.c_str(), failure.where.file_name(), static_cast<int>(failure.where.line()));
             }
         }
 
     private:
         MockWallet* m_wallet;
-        std::source_location m_where;
     };
 
-    Verification VerifyOnExit(std::source_location where = std::source_location::current())
+    [[nodiscard]] Verification VerifyOnExit()
     {
-        return Verification{*this, where};
+        return Verification{*this};
     }
 
-    void ExpectCallCount(const std::atomic<int>& calls, int expected, const char* name)
+    void ExpectExactly(const CallCounter& counter, int expected, std::source_location where = std::source_location::current())
     {
-        m_call_expectations.push_back(CallExpectation{&calls, calls.load(), expected, expected, name});
+        m_call_expectations.push_back(CallExpectation{&counter, counter.load(), expected, expected, where});
     }
 
-    void ExpectAtLeast(const std::atomic<int>& calls, int minimum, const char* name)
+    void ExpectNoCalls(const CallCounter& counter, std::source_location where = std::source_location::current())
     {
-        m_call_expectations.push_back(CallExpectation{&calls, calls.load(), minimum, std::nullopt, name});
+        ExpectExactly(counter, 0, where);
     }
 
-    std::function<util::Result<CTransactionRef>(const std::vector<wallet::CRecipient>&, const wallet::CCoinControl&, bool, int&, CAmount&)> createTransactionHandler;
+    void ExpectAtLeast(const CallCounter& counter, int minimum, std::source_location where = std::source_location::current())
+    {
+        m_call_expectations.push_back(CallExpectation{&counter, counter.load(), minimum, std::nullopt, where});
+    }
+
+    std::function<util::Result<CTransactionRef>(const std::vector<wallet::CRecipient>&, const wallet::CCoinControl&, bool, int&, CAmount&)> create_transaction_fn;
     std::function<CTxDestination(OutputType, const std::string&)> get_new_destination_fn;
     std::function<std::set<interfaces::WalletTx>()> get_wallet_txs_fn;
     std::function<CAmount()> get_balance_fn;
@@ -149,22 +158,26 @@ class MockWallet : public StubWallet
     std::function<bool(CMutableTransaction&)> sign_bump_transaction_fn;
     std::function<bool(const Txid&, CMutableTransaction&&, std::vector<bilingual_str>&, Txid&)> commit_bump_transaction_fn;
 
-    std::atomic<int> get_new_destination_calls{0};
-    std::atomic<int> get_wallet_txs_calls{0};
-    std::atomic<int> get_balance_calls{0};
-    std::atomic<int> get_available_balance_calls{0};
-    std::atomic<int> get_required_fee_calls{0};
-    std::atomic<int> list_coins_calls{0};
-    std::atomic<int> get_default_address_type_calls{0};
-    std::atomic<int> handle_transaction_changed_calls{0};
-    std::atomic<int> transaction_can_be_bumped_calls{0};
-    std::atomic<int> create_bump_transaction_calls{0};
-    std::atomic<int> sign_bump_transaction_calls{0};
-    std::atomic<int> commit_bump_transaction_calls{0};
+    struct Calls
+    {
+        CallCounter getNewDestination{"getNewDestination"};
+        CallCounter createTransaction{"createTransaction"};
+        CallCounter getWalletTxs{"getWalletTxs"};
+        CallCounter getBalance{"getBalance"};
+        CallCounter getAvailableBalance{"getAvailableBalance"};
+        CallCounter getRequiredFee{"getRequiredFee"};
+        CallCounter listCoins{"listCoins"};
+        CallCounter getDefaultAddressType{"getDefaultAddressType"};
+        CallCounter handleTransactionChanged{"handleTransactionChanged"};
+        CallCounter transactionCanBeBumped{"transactionCanBeBumped"};
+        CallCounter createBumpTransaction{"createBumpTransaction"};
+        CallCounter signBumpTransaction{"signBumpTransaction"};
+        CallCounter commitBumpTransaction{"commitBumpTransaction"};
+    } calls;
 
     util::Result<CTxDestination> getNewDestination(const OutputType type, const std::string& label) override
     {
-        ++get_new_destination_calls;
+        ++calls.getNewDestination;
         if (get_new_destination_fn) return get_new_destination_fn(type, label);
         return util::Error{Untranslated("no get_new_destination_fn installed")};
     }
@@ -174,10 +187,11 @@ class MockWallet : public StubWallet
         bool sign,
         std::optional<unsigned int>) override
     {
-        if (createTransactionHandler) {
+        ++calls.createTransaction;
+        if (create_transaction_fn) {
             int change_pos{-1};
             CAmount fee{0};
-            auto result = createTransactionHandler(recipients, coin_control, sign, change_pos, fee);
+            auto result = create_transaction_fn(recipients, coin_control, sign, change_pos, fee);
             if (!result) {
                 return util::Error{util::ErrorString(result)};
             }
@@ -187,94 +201,100 @@ class MockWallet : public StubWallet
                 change_pos >= 0 ? std::optional<unsigned int>{static_cast<unsigned int>(change_pos)} : std::nullopt,
                 FeeCalculation{}};
         }
-        return util::Error{Untranslated("no createTransactionHandler installed")};
+        return util::Error{Untranslated("no create_transaction_fn installed")};
     }
 
     std::set<interfaces::WalletTx> getWalletTxs() override
     {
-        ++get_wallet_txs_calls;
+        ++calls.getWalletTxs;
         return get_wallet_txs_fn ? get_wallet_txs_fn() : std::set<interfaces::WalletTx>{};
     }
 
     CAmount getBalance() override
     {
-        ++get_balance_calls;
+        ++calls.getBalance;
         return get_balance_fn ? get_balance_fn() : 0;
     }
 
     CAmount getAvailableBalance(const wallet::CCoinControl& coin_control) override
     {
-        ++get_available_balance_calls;
+        ++calls.getAvailableBalance;
         return get_available_balance_fn ? get_available_balance_fn(coin_control) : 0;
     }
 
     CAmount getRequiredFee(unsigned int tx_bytes) override
     {
-        ++get_required_fee_calls;
+        ++calls.getRequiredFee;
         return get_required_fee_fn ? get_required_fee_fn(tx_bytes) : 0;
     }
 
     CoinsList listCoins() override
     {
-        ++list_coins_calls;
+        ++calls.listCoins;
         return list_coins_fn ? list_coins_fn() : CoinsList{};
     }
 
     OutputType getDefaultAddressType() override
     {
-        ++get_default_address_type_calls;
+        ++calls.getDefaultAddressType;
         return get_default_address_type_fn ? get_default_address_type_fn() : OutputType::BECH32;
     }
 
     std::unique_ptr<interfaces::Handler> handleTransactionChanged(TransactionChangedFn fn) override
     {
-        ++handle_transaction_changed_calls;
+        ++calls.handleTransactionChanged;
         return handle_transaction_changed_fn ? handle_transaction_changed_fn(std::move(fn)) : nullptr;
     }
 
     bool transactionCanBeBumped(const Txid& txid) override
     {
-        ++transaction_can_be_bumped_calls;
+        ++calls.transactionCanBeBumped;
         return transaction_can_be_bumped_fn ? transaction_can_be_bumped_fn(txid) : false;
     }
 
     bool createBumpTransaction(const Txid& txid, const wallet::CCoinControl& coin_control, std::vector<bilingual_str>& errors,
         CAmount& old_fee, CAmount& new_fee, CMutableTransaction& mtx) override
     {
-        ++create_bump_transaction_calls;
+        ++calls.createBumpTransaction;
         return create_bump_transaction_fn ? create_bump_transaction_fn(txid, coin_control, errors, old_fee, new_fee, mtx) : false;
     }
 
     bool signBumpTransaction(CMutableTransaction& mtx) override
     {
-        ++sign_bump_transaction_calls;
+        ++calls.signBumpTransaction;
         return sign_bump_transaction_fn ? sign_bump_transaction_fn(mtx) : false;
     }
 
     bool commitBumpTransaction(const Txid& txid, CMutableTransaction&& mtx, std::vector<bilingual_str>& errors, Txid& bumped_txid) override
     {
-        ++commit_bump_transaction_calls;
+        ++calls.commitBumpTransaction;
         return commit_bump_transaction_fn ? commit_bump_transaction_fn(txid, std::move(mtx), errors, bumped_txid) : false;
     }
 
 private:
     struct CallExpectation
     {
-        const std::atomic<int>* calls;
+        const CallCounter* counter;
         int baseline;
         int minimum;
         std::optional<int> maximum;
-        std::string name;
+        std::source_location where;
+    };
+
+    struct VerificationFailure
+    {
+        std::string message;
+        std::source_location where;
     };
 
-    std::string CallExpectationFailure() const
+    std::vector<VerificationFailure> VerificationFailures() const
     {
-        std::ostringstream message;
+        std::vector<VerificationFailure> failures;
         for (const CallExpectation& expectation : m_call_expectations) {
-            const int actual{expectation.calls->load() - expectation.baseline};
+            const int actual{expectation.counter->load() - expectation.baseline};
             if (actual < expectation.minimum || (expectation.maximum && actual > *expectation.maximum)) {
-                if (message.tellp() > 0) message << '\n';
-                message << "Expected " << expectation.name << " calls to be ";
+                std::ostringstream message;
+                message << "Expected " << expectation.counter->Name() << " calls to be ";
                 if (expectation.maximum && expectation.minimum == *expectation.maximum) {
                     message << expectation.minimum;
                 } else if (expectation.maximum) {
@@ -283,9 +303,10 @@ class MockWallet : public StubWallet
                     message << "at least " << expectation.minimum;
                 }
                 message << ", actual " << actual;
+                failures.push_back({message.str(), expectation.where});
             }
         }
-        return message.str();
+        return failures;
     }
 
     std::vector<CallExpectation> m_call_expectations;

test/mocks/mockwallet.h

@@ -130,6 +130,7 @@ class StubNode : public interfaces::Node
 
 protected:
     virtual void UnhandledCall(const char*) {}
+    interfaces::WalletLoader& FallbackWalletLoader() { return m_wallet_loader; }
 
 private:
     StubWalletLoader m_wallet_loader;

test/mocks/stubnode.h

@@ -64,7 +64,7 @@ void BanListModelTests::refreshPopulatesRolesAndRows()
 
     model.refresh();
 
-    QCOMPARE(node.get_banned_calls.load(), 1);
+    QCOMPARE(node.calls.getBanned.load(), 1);
     QCOMPARE(model.count(), 2);
     QCOMPARE(model.rowCount(), 2);
     QCOMPARE(model.rowCount(model.index(0, 0)), 0);
@@ -112,8 +112,8 @@ void BanListModelTests::unbanAtTargetsSelectedSubnet()
         return true;
     };
     QVERIFY(model.unbanAt(0));
-    QCOMPARE(node.get_banned_calls.load(), 1);
-    QCOMPARE(node.unban_calls.load(), 1);
+    QCOMPARE(node.calls.getBanned.load(), 1);
+    QCOMPARE(node.calls.unban.load(), 1);
     QVERIFY(received_expected_subnet);
 }
 
@@ -133,8 +133,8 @@ void BanListModelTests::unbanAtReturnsFalseWhenNodeRejectsSubnet()
 
     node.unban_fn = [](const CSubNet&) { return false; };
     QVERIFY(!model.unbanAt(0));
-    QCOMPARE(node.get_banned_calls.load(), 1);
-    QCOMPARE(node.unban_calls.load(), 1);
+    QCOMPARE(node.calls.getBanned.load(), 1);
+    QCOMPARE(node.calls.unban.load(), 1);
 }
 
 void BanListModelTests::unbanAtIgnoresInvalidRows()
@@ -153,8 +153,8 @@ void BanListModelTests::unbanAtIgnoresInvalidRows()
 
     QVERIFY(!model.unbanAt(-1));
     QVERIFY(!model.unbanAt(42));
-    QCOMPARE(node.get_banned_calls.load(), 1);
-    QCOMPARE(node.unban_calls.load(), 0);
+    QCOMPARE(node.calls.getBanned.load(), 1);
+    QCOMPARE(node.calls.unban.load(), 0);
 }
 
 #ifdef BITCOINQML_NO_TEST_MAIN