diff options
author | merge-script <fanquake@gmail.com> | 2024-07-15 17:21:55 +0100 |
---|---|---|
committer | merge-script <fanquake@gmail.com> | 2024-07-15 17:21:55 +0100 |
commit | ff827a8f46e504712285c145e13e72e1b19eb169 (patch) | |
tree | 6422a69ba7e162c7dce004cc3584d51ee9cd2eff /src/test | |
parent | 262260ce1e919613ba60194a5861b0b0a51dfe01 (diff) | |
parent | fa690c8e532672f7ab53be6f7a0bb3070858745e (diff) | |
download | bitcoin-ff827a8f46e504712285c145e13e72e1b19eb169.tar.xz |
Merge bitcoin/bitcoin#30407: test: [refactor] Pass TestOpts
fa690c8e532672f7ab53be6f7a0bb3070858745e test: [refactor] Pass TestOpts (MarcoFalke)
Pull request description:
Currently optional test context setup settings are passed by adding a new optional argument to the constructors. For example `extra_args`. This is problematic, because:
* Adding more optional settings in the future requires touching all affected constructors, increasing their verbosity.
* Setting only a later option requires setting the earlier ones.
* Clang-tidy named args passed to `std::make_unique` are not checked.
Fix all issues by adding a new struct `TestOpts`, which holds all options. Notes:
* The chain type is not an option in the struct for now, because the default values vary.
* The struct holds all possible test options globally. Not all fields may be used by all constructors. Albeit harmless, it is up to the test author to not set a field that is unused.
ACKs for top commit:
kevkevinpal:
utACK [fa690c8](https://github.com/bitcoin/bitcoin/pull/30407/commits/fa690c8e532672f7ab53be6f7a0bb3070858745e)
dergoegge:
utACK fa690c8e532672f7ab53be6f7a0bb3070858745e
TheCharlatan:
Nice, ACK fa690c8e532672f7ab53be6f7a0bb3070858745e
Tree-SHA512: 8db8efa5dff854a73757d3f454f8f902e41bb4358f5f9bae29dbb3e251e20ee93489605de51d0822ba31d97835cd15526a29c075278dd6a8bbde26134feb4f49
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/checkqueue_tests.cpp | 2 | ||||
-rw-r--r-- | src/test/fuzz/process_message.cpp | 2 | ||||
-rw-r--r-- | src/test/fuzz/process_messages.cpp | 2 | ||||
-rw-r--r-- | src/test/fuzz/rpc.cpp | 2 | ||||
-rw-r--r-- | src/test/fuzz/utxo_total_supply.cpp | 2 | ||||
-rw-r--r-- | src/test/i2p_tests.cpp | 4 | ||||
-rw-r--r-- | src/test/net_peer_connection_tests.cpp | 2 | ||||
-rw-r--r-- | src/test/txvalidationcache_tests.cpp | 2 | ||||
-rw-r--r-- | src/test/util/setup_common.cpp | 26 | ||||
-rw-r--r-- | src/test/util/setup_common.h | 26 | ||||
-rw-r--r-- | src/test/validation_chainstatemanager_tests.cpp | 7 |
11 files changed, 38 insertions, 39 deletions
diff --git a/src/test/checkqueue_tests.cpp b/src/test/checkqueue_tests.cpp index 023a5e8e70..7810d91a77 100644 --- 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 index d10d9dafe8..6373eac1c3 100644 --- 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 index 38acd432fa..62f38967a3 100644 --- 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 index 4e52c1c091..9122617e46 100644 --- 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 index 48ed266abe..b0f1a1251a 100644 --- 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 index 0512c6134f..bb9ca88019 100644 --- 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 index 5f38ce112c..2dde6daee5 100644 --- 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 index e3b215ad83..af36a95693 100644 --- a/src/test/txvalidationcache_tests.cpp +++ b/src/test/txvalidationcache_tests.cpp @@ -17,7 +17,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 index 5633757b97..60f9261e7e 100644 --- a/src/test/util/setup_common.cpp +++ b/src/test/util/setup_common.cpp @@ -108,7 +108,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; @@ -125,7 +125,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()); } @@ -211,8 +211,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(); @@ -295,13 +295,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); @@ -330,11 +328,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 index dbd66e3585..e8b630af94 100644 --- a/src/test/util/setup_common.h +++ b/src/test/util/setup_common.h @@ -48,6 +48,12 @@ 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. */ @@ -55,7 +61,7 @@ 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 index 1641c4cd22..f93e3cdfb1 100644 --- 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, + }, } { } |