test: [refactor] Pass TestOpts

MarcoFalke · MarcoFalke · commit fa690c8e5326 · 2024-07-08T16:11:15.000+02:00
diff --git a/src/bench/logging.cpp b/src/bench/logging.cpp
@@ -20,7 +20,7 @@ static void Logging(benchmark::Bench& bench, const std::vector<const char*>& ext
 
     TestingSetup test_setup{
         ChainType::REGTEST,
-        extra_args,
+        {.extra_args = extra_args},
     };
 
     bench.run([&] { log(); });
diff --git a/src/bench/mempool_stress.cpp b/src/bench/mempool_stress.cpp
@@ -106,7 +106,7 @@ static void ComplexMemPool(benchmark::Bench& bench)
 static void MempoolCheck(benchmark::Bench& bench)
 {
     FastRandomContext det_rand{true};
-    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {"-checkmempool=1"});
+    auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {.extra_args = {"-checkmempool=1"}});
     CTxMemPool& pool = *testing_setup.get()->m_node.mempool;
     LOCK2(cs_main, pool.cs);
     testing_setup->PopulateMempool(det_rand, 400, true);
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp
@@ -28,7 +28,7 @@
 struct NoLockLoggingTestingSetup : public TestingSetup {
     NoLockLoggingTestingSetup()
 #ifdef DEBUG_LOCKCONTENTION
-        : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-debugexclude=lock"}} {}
+        : TestingSetup{ChainType::MAIN, {.extra_args = { "-debugexclude=lock" } }} {}
 #else
         : TestingSetup{ChainType::MAIN} {}
 #endif
diff --git a/src/test/fuzz/process_message.cpp b/src/test/fuzz/process_message.cpp
@@ -42,7 +42,7 @@ void initialize_process_message()
 
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
             /*chain_type=*/ChainType::REGTEST,
-            /*extra_args=*/{"-txreconciliation"});
+            {.extra_args = {"-txreconciliation"}});
     g_setup = testing_setup.get();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
         MineBlock(g_setup->m_node, CScript() << OP_TRUE);
diff --git a/src/test/fuzz/process_messages.cpp b/src/test/fuzz/process_messages.cpp
@@ -32,7 +32,7 @@ void initialize_process_messages()
 {
     static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(
             /*chain_type=*/ChainType::REGTEST,
-            /*extra_args=*/{"-txreconciliation"});
+            {.extra_args = {"-txreconciliation"}});
     g_setup = testing_setup.get();
     for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {
         MineBlock(g_setup->m_node, CScript() << OP_TRUE);
diff --git a/src/test/fuzz/rpc.cpp b/src/test/fuzz/rpc.cpp
@@ -41,7 +41,7 @@ using util::ToString;
 
 namespace {
 struct RPCFuzzTestingSetup : public TestingSetup {
-    RPCFuzzTestingSetup(const ChainType chain_type, const std::vector<const char*>& extra_args) : TestingSetup{chain_type, extra_args}
+    RPCFuzzTestingSetup(const ChainType chain_type, TestOpts opts) : TestingSetup{chain_type, opts}
     {
     }
 
diff --git a/src/test/fuzz/utxo_total_supply.cpp b/src/test/fuzz/utxo_total_supply.cpp
@@ -23,7 +23,7 @@ FUZZ_TARGET(utxo_total_supply)
     ChainTestingSetup test_setup{
         ChainType::REGTEST,
         {
-            "-testactivationheight=bip34@2",
+            .extra_args = {"-testactivationheight=bip34@2"},
         },
     };
     // Create chainstate
diff --git a/src/test/i2p_tests.cpp b/src/test/i2p_tests.cpp
@@ -23,8 +23,8 @@ class EnvTestingSetup : public BasicTestingSetup
 {
 public:
     explicit EnvTestingSetup(const ChainType chainType = ChainType::MAIN,
-                             const std::vector<const char*>& extra_args = {})
-        : BasicTestingSetup{chainType, extra_args},
+                             TestOpts opts = {})
+        : BasicTestingSetup{chainType, opts},
           m_prev_log_level{LogInstance().LogLevel()},
           m_create_sock_orig{CreateSock}
     {
diff --git a/src/test/net_peer_connection_tests.cpp b/src/test/net_peer_connection_tests.cpp
@@ -31,7 +31,7 @@
 
 struct LogIPsTestingSetup : public TestingSetup {
     LogIPsTestingSetup()
-        : TestingSetup{ChainType::MAIN, /*extra_args=*/{"-logips"}} {}
+        : TestingSetup{ChainType::MAIN, {.extra_args = {"-logips"}}} {}
 };
 
 BOOST_FIXTURE_TEST_SUITE(net_peer_connection_tests, LogIPsTestingSetup)
diff --git a/src/test/txvalidationcache_tests.cpp b/src/test/txvalidationcache_tests.cpp
@@ -16,7 +16,7 @@
 
 struct Dersig100Setup : public TestChain100Setup {
     Dersig100Setup()
-        : TestChain100Setup{ChainType::REGTEST, {"-testactivationheight=dersig@102"}} {}
+        : TestChain100Setup{ChainType::REGTEST, {.extra_args = {"-testactivationheight=dersig@102"}}} {}
 };
 
 bool CheckInputScripts(const CTransaction& tx, TxValidationState& state,
diff --git a/src/test/util/setup_common.cpp b/src/test/util/setup_common.cpp
@@ -112,7 +112,7 @@ static void ExitFailure(std::string_view str_err)
     exit(EXIT_FAILURE);
 }
 
-BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args)
+BasicTestingSetup::BasicTestingSetup(const ChainType chainType, TestOpts opts)
     : m_args{}
 {
     m_node.shutdown = &m_interrupt;
@@ -129,7 +129,7 @@ BasicTestingSetup::BasicTestingSetup(const ChainType chainType, const std::vecto
             "-debugexclude=libevent",
             "-debugexclude=leveldb",
         },
-        extra_args);
+        opts.extra_args);
     if (G_TEST_COMMAND_LINE_ARGUMENTS) {
         arguments = Cat(arguments, G_TEST_COMMAND_LINE_ARGUMENTS());
     }
@@ -220,8 +220,8 @@ BasicTestingSetup::~BasicTestingSetup()
     gArgs.ClearArgs();
 }
 
-ChainTestingSetup::ChainTestingSetup(const ChainType chainType, const std::vector<const char*>& extra_args)
-    : BasicTestingSetup(chainType, extra_args)
+ChainTestingSetup::ChainTestingSetup(const ChainType chainType, TestOpts opts)
+    : BasicTestingSetup(chainType, opts)
 {
     const CChainParams& chainparams = Params();
 
@@ -304,13 +304,11 @@ void ChainTestingSetup::LoadVerifyActivateChainstate()
 
 TestingSetup::TestingSetup(
     const ChainType chainType,
-    const std::vector<const char*>& extra_args,
-    const bool coins_db_in_memory,
-    const bool block_tree_db_in_memory)
-    : ChainTestingSetup(chainType, extra_args)
+    TestOpts opts)
+    : ChainTestingSetup(chainType, opts)
 {
-    m_coins_db_in_memory = coins_db_in_memory;
-    m_block_tree_db_in_memory = block_tree_db_in_memory;
+    m_coins_db_in_memory = opts.coins_db_in_memory;
+    m_block_tree_db_in_memory = opts.block_tree_db_in_memory;
     // Ideally we'd move all the RPC tests to the functional testing framework
     // instead of unit tests, but for now we need these here.
     RegisterAllCoreRPCCommands(tableRPC);
@@ -339,11 +337,9 @@ TestingSetup::TestingSetup(
 }
 
 TestChain100Setup::TestChain100Setup(
-        const ChainType chain_type,
-        const std::vector<const char*>& extra_args,
-        const bool coins_db_in_memory,
-        const bool block_tree_db_in_memory)
-    : TestingSetup{ChainType::REGTEST, extra_args, coins_db_in_memory, block_tree_db_in_memory}
+    const ChainType chain_type,
+    TestOpts opts)
+    : TestingSetup{ChainType::REGTEST, opts}
 {
     SetMockTime(1598887952);
     constexpr std::array<unsigned char, 32> vchKey = {
diff --git a/src/test/util/setup_common.h b/src/test/util/setup_common.h
@@ -48,14 +48,20 @@ std::ostream& operator<<(typename std::enable_if<std::is_enum<T>::value, std::os
 
 static constexpr CAmount CENT{1000000};
 
+struct TestOpts {
+    std::vector<const char*> extra_args{};
+    bool coins_db_in_memory{true};
+    bool block_tree_db_in_memory{true};
+};
+
 /** Basic testing setup.
  * This just configures logging, data dir and chain parameters.
  */
 struct BasicTestingSetup {
     util::SignalInterrupt m_interrupt;
     node::NodeContext m_node; // keep as first member to be destructed last
 
-    explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {});
+    explicit BasicTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});
     ~BasicTestingSetup();
 
     fs::path m_path_root;
@@ -73,7 +79,7 @@ struct ChainTestingSetup : public BasicTestingSetup {
     bool m_coins_db_in_memory{true};
     bool m_block_tree_db_in_memory{true};
 
-    explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, const std::vector<const char*>& extra_args = {});
+    explicit ChainTestingSetup(const ChainType chainType = ChainType::MAIN, TestOpts = {});
     ~ChainTestingSetup();
 
     // Supplies a chainstate, if one is needed
@@ -85,9 +91,7 @@ struct ChainTestingSetup : public BasicTestingSetup {
 struct TestingSetup : public ChainTestingSetup {
     explicit TestingSetup(
         const ChainType chainType = ChainType::MAIN,
-        const std::vector<const char*>& extra_args = {},
-        const bool coins_db_in_memory = true,
-        const bool block_tree_db_in_memory = true);
+        TestOpts = {});
 };
 
 /** Identical to TestingSetup, but chain set to regtest */
@@ -106,9 +110,7 @@ class CScript;
 struct TestChain100Setup : public TestingSetup {
     TestChain100Setup(
         const ChainType chain_type = ChainType::REGTEST,
-        const std::vector<const char*>& extra_args = {},
-        const bool coins_db_in_memory = true,
-        const bool block_tree_db_in_memory = true);
+        TestOpts = {});
 
     /**
      * Create a new block with just given transactions, coinbase paying to
@@ -220,16 +222,16 @@ struct TestChain100Setup : public TestingSetup {
  * be used in "hot loops", for example fuzzing or benchmarking.
  */
 template <class T = const BasicTestingSetup>
-std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, const std::vector<const char*>& extra_args = {})
+std::unique_ptr<T> MakeNoLogFileContext(const ChainType chain_type = ChainType::REGTEST, TestOpts opts = {})
 {
-    const std::vector<const char*> arguments = Cat(
+    opts.extra_args = Cat(
         {
             "-nodebuglogfile",
             "-nodebug",
         },
-        extra_args);
+        opts.extra_args);
 
-    return std::make_unique<T>(chain_type, arguments);
+    return std::make_unique<T>(chain_type, opts);
 }
 
 CBlock getBlock13b8a();
diff --git a/src/test/validation_chainstatemanager_tests.cpp b/src/test/validation_chainstatemanager_tests.cpp
@@ -167,9 +167,10 @@ struct SnapshotTestSetup : TestChain100Setup {
     // destructive filesystem operations.
     SnapshotTestSetup() : TestChain100Setup{
                               {},
-                              {},
-                              /*coins_db_in_memory=*/false,
-                              /*block_tree_db_in_memory=*/false,
+                              {
+                                  .coins_db_in_memory = false,
+                                  .block_tree_db_in_memory = false,
+                              },
                           }
     {
     }

Original file line number	Diff line number	Diff line change
`@@ -106,7 +106,7 @@ static void ComplexMemPool(benchmark::Bench& bench)`
`106`	`106`	`static void MempoolCheck(benchmark::Bench& bench)`
`107`	`107`	`{`
`108`	`108`	`FastRandomContext det_rand{true};`
`109`		`- auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {"-checkmempool=1"});`
	`109`	`+ auto testing_setup = MakeNoLogFileContext<TestChain100Setup>(ChainType::REGTEST, {.extra_args = {"-checkmempool=1"}});`
`110`	`110`	`CTxMemPool& pool = *testing_setup.get()->m_node.mempool;`
`111`	`111`	`LOCK2(cs_main, pool.cs);`
`112`	`112`	`testing_setup->PopulateMempool(det_rand, 400, true);`
Original file line number	Diff line number	Diff line change
`@@ -32,7 +32,7 @@ void initialize_process_messages()`
`32`	`32`	`{`
`33`	`33`	`static const auto testing_setup = MakeNoLogFileContext<const TestingSetup>(`
`34`	`34`	`/chain_type=/ChainType::REGTEST,`
`35`		`- /extra_args=/{"-txreconciliation"});`
	`35`	`+ {.extra_args = {"-txreconciliation"}});`
`36`	`36`	`g_setup = testing_setup.get();`
`37`	`37`	`for (int i = 0; i < 2 * COINBASE_MATURITY; i++) {`
`38`	`38`	`MineBlock(g_setup->m_node, CScript() << OP_TRUE);`
Original file line number	Diff line number	Diff line change
`@@ -41,7 +41,7 @@ using util::ToString;`
`41`	`41`
`42`	`42`	`namespace {`
`43`	`43`	`struct RPCFuzzTestingSetup : public TestingSetup {`
`44`		`- RPCFuzzTestingSetup(const ChainType chain_type, const std::vector<const char*>& extra_args) : TestingSetup{chain_type, extra_args}`
	`44`	`+ RPCFuzzTestingSetup(const ChainType chain_type, TestOpts opts) : TestingSetup{chain_type, opts}`
`45`	`45`	`{`
`46`	`46`	`}`
`47`	`47`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ FUZZ_TARGET(utxo_total_supply)`
`23`	`23`	`ChainTestingSetup test_setup{`
`24`	`24`	`ChainType::REGTEST,`
`25`	`25`	`{`
`26`		`- "-testactivationheight=bip34@2",`
	`26`	`+ .extra_args = {"-testactivationheight=bip34@2"},`
`27`	`27`	`},`
`28`	`28`	`};`
`29`	`29`	`// Create chainstate`
Original file line number	Diff line number	Diff line change
`@@ -23,8 +23,8 @@ class EnvTestingSetup : public BasicTestingSetup`
`23`	`23`	`{`
`24`	`24`	`public:`
`25`	`25`	`explicit EnvTestingSetup(const ChainType chainType = ChainType::MAIN,`
`26`		`- const std::vector<const char*>& extra_args = {})`
`27`		`- : BasicTestingSetup{chainType, extra_args},`
	`26`	`+ TestOpts opts = {})`
	`27`	`+ : BasicTestingSetup{chainType, opts},`
`28`	`28`	`m_prev_log_level{LogInstance().LogLevel()},`
`29`	`29`	`m_create_sock_orig{CreateSock}`
`30`	`30`	`{`