From 08b32db3d924dbd08676fa91b8fc8c31a418ae49 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Wo=CC=81jcik?= <3044353+pwojcikdev@users.noreply.github.com> Date: Tue, 26 Mar 2024 17:20:29 +0100 Subject: [PATCH] Do not directly stop nodes in tests --- nano/core_test/network.cpp | 2 +- nano/core_test/node.cpp | 20 ++++++-------------- nano/core_test/rep_crawler.cpp | 4 ++-- nano/core_test/socket.cpp | 10 ---------- nano/core_test/telemetry.cpp | 4 +--- nano/test_common/system.cpp | 18 ++++++++++++++++++ nano/test_common/system.hpp | 5 ++++- 7 files changed, 32 insertions(+), 31 deletions(-) diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index 8b9a372ef..73bcd7d94 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -569,7 +569,7 @@ TEST (network, ipv6_bind_send_ipv4) TEST (network, endpoint_bad_fd) { nano::test::system system (1); - system.nodes[0]->stop (); + system.stop_node (*system.nodes[0]); auto endpoint (system.nodes[0]->network.endpoint ()); ASSERT_TRUE (endpoint.address ().is_loopback ()); // The endpoint is invalidated asynchronously diff --git a/nano/core_test/node.cpp b/nano/core_test/node.cpp index f9bc35ec5..d396a2cba 100644 --- a/nano/core_test/node.cpp +++ b/nano/core_test/node.cpp @@ -40,8 +40,6 @@ TEST (node, stop) { nano::test::system system (1); ASSERT_NE (system.nodes[0]->wallets.items.end (), system.nodes[0]->wallets.items.begin ()); - system.nodes[0]->stop (); - system.io_ctx->run (); ASSERT_TRUE (true); } @@ -77,9 +75,10 @@ TEST (node, block_store_path_failure) auto path (nano::unique_path ()); nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits::max () }; auto node (std::make_shared (io_ctx, system.get_available_port (), path, pool)); + system.register_node (node); ASSERT_TRUE (node->wallets.items.empty ()); - node->stop (); } + #if defined(__clang__) && defined(__linux__) && CI // Disable test due to instability with clang and actions TEST (node_DeathTest, DISABLED_readonly_block_store_not_exist) @@ -102,16 +101,13 @@ TEST (node_DeathTest, readonly_block_store_not_exist) TEST (node, password_fanout) { nano::test::system system; - auto io_ctx = std::make_shared (); - auto path (nano::unique_path ()); nano::node_config config; config.peering_port = system.get_available_port (); nano::work_pool pool{ nano::dev::network_params.network, std::numeric_limits::max () }; config.password_fanout = 10; - nano::node node (io_ctx, path, config, pool); + auto & node = *system.add_node (config); auto wallet (node.wallets.create (100)); ASSERT_EQ (10, wallet->store.password.values.size ()); - node.stop (); } TEST (node, balance) @@ -284,8 +280,6 @@ TEST (node, auto_bootstrap) ASSERT_TIMELY_EQ (5s, node1->ledger.cache.block_count, 3); // Confirmation for all blocks ASSERT_TIMELY_EQ (5s, node1->ledger.cache.cemented_count, 3); - - node1->stop (); } TEST (node, auto_bootstrap_reverse) @@ -329,8 +323,6 @@ TEST (node, auto_bootstrap_age) ASSERT_TIMELY (10s, node0->stats.count (nano::stat::type::bootstrap, nano::stat::detail::initiate_legacy_age, nano::stat::dir::out) >= 3); // More attempts with frontiers age ASSERT_GE (node0->stats.count (nano::stat::type::bootstrap, nano::stat::detail::initiate_legacy_age, nano::stat::dir::out), node0->stats.count (nano::stat::type::bootstrap, nano::stat::detail::initiate, nano::stat::dir::out)); - - node1->stop (); } TEST (node, merge_peers) @@ -2843,7 +2835,7 @@ TEST (node, peers) ASSERT_TRUE (store.peer.exists (store.tx_begin_read (), endpoint_key)); // Stop the peer node and check that it is removed from the store - node1->stop (); + system.stop_node (*node1); // TODO: In `tcp_channels::store_all` we skip store operation when there are no peers present, // so the best we can do here is check if network is empty @@ -2873,7 +2865,7 @@ TEST (node, peer_cache_restart) auto list (node2->network.list (2)); ASSERT_EQ (node1->network.endpoint (), list[0]->get_endpoint ()); ASSERT_EQ (1, node2->network.size ()); - node2->stop (); + system.stop_node (*node2); } // Restart node { @@ -2896,7 +2888,7 @@ TEST (node, peer_cache_restart) auto list (node3->network.list (2)); ASSERT_EQ (node1->network.endpoint (), list[0]->get_endpoint ()); ASSERT_EQ (1, node3->network.size ()); - node3->stop (); + system.stop_node (*node3); } } diff --git a/nano/core_test/rep_crawler.cpp b/nano/core_test/rep_crawler.cpp index 0dc098ada..1addde26c 100644 --- a/nano/core_test/rep_crawler.cpp +++ b/nano/core_test/rep_crawler.cpp @@ -220,7 +220,7 @@ TEST (rep_crawler, rep_remove) ASSERT_TIMELY_EQ (10s, searching_node.rep_crawler.representative_count (), 2); // When Rep2 is stopped, it should not be found as principal representative anymore - node_rep2->stop (); + system.stop_node (*node_rep2); ASSERT_TIMELY_EQ (10s, searching_node.rep_crawler.representative_count (), 1); // Now only genesisRep should be found: @@ -239,7 +239,7 @@ TEST (rep_crawler, rep_connection_close) // Add working representative (node 2) system.wallet (1)->insert_adhoc (nano::dev::genesis_key.prv); ASSERT_TIMELY_EQ (10s, node1.rep_crawler.representative_count (), 1); - node2.stop (); + system.stop_node (node2); // Remove representative with closed channel ASSERT_TIMELY_EQ (10s, node1.rep_crawler.representative_count (), 0); } diff --git a/nano/core_test/socket.cpp b/nano/core_test/socket.cpp index bbbf9d88a..680e8ef25 100644 --- a/nano/core_test/socket.cpp +++ b/nano/core_test/socket.cpp @@ -103,8 +103,6 @@ TEST (socket, max_connections) ASSERT_TIMELY_EQ (5s, get_tcp_accept_successes (), 5); ASSERT_TIMELY_EQ (5s, connection_attempts, 8); // connections initiated by the client ASSERT_TIMELY_EQ (5s, server_sockets.size (), 5); // connections accepted by the server - - node->stop (); } TEST (socket, max_connections_per_ip) @@ -162,8 +160,6 @@ TEST (socket, max_connections_per_ip) ASSERT_TIMELY_EQ (5s, get_tcp_accept_successes (), max_ip_connections); ASSERT_TIMELY_EQ (5s, get_tcp_max_per_ip (), 1); ASSERT_TIMELY_EQ (5s, connection_attempts, max_ip_connections + 1); - - node->stop (); } TEST (socket, limited_subnet_address) @@ -283,8 +279,6 @@ TEST (socket, max_connections_per_subnetwork) ASSERT_TIMELY_EQ (5s, get_tcp_accept_successes (), max_subnetwork_connections); ASSERT_TIMELY_EQ (5s, get_tcp_max_per_subnetwork (), 1); ASSERT_TIMELY_EQ (5s, connection_attempts, max_subnetwork_connections + 1); - - node->stop (); } TEST (socket, disabled_max_peers_per_ip) @@ -344,8 +338,6 @@ TEST (socket, disabled_max_peers_per_ip) ASSERT_TIMELY_EQ (5s, get_tcp_accept_successes (), max_ip_connections + 1); ASSERT_TIMELY_EQ (5s, get_tcp_max_per_ip (), 0); ASSERT_TIMELY_EQ (5s, connection_attempts, max_ip_connections + 1); - - node->stop (); } TEST (socket, disconnection_of_silent_connections) @@ -399,8 +391,6 @@ TEST (socket, disconnection_of_silent_connections) ASSERT_EQ (0, get_tcp_io_timeout_drops ()); // Asserts the silent checker worked. ASSERT_EQ (1, get_tcp_silent_connection_drops ()); - - node->stop (); } TEST (socket, drop_policy) diff --git a/nano/core_test/telemetry.cpp b/nano/core_test/telemetry.cpp index e607a3603..fe6e5db72 100644 --- a/nano/core_test/telemetry.cpp +++ b/nano/core_test/telemetry.cpp @@ -334,16 +334,14 @@ TEST (telemetry, disconnected) nano::node_flags node_flags; auto node_client = system.add_node (node_flags); auto node_server = system.add_node (node_flags); - nano::test::wait_peer_connections (system); - auto channel = node_client->network.find_node_id (node_server->get_node_id ()); ASSERT_NE (nullptr, channel); // Ensure telemetry is available before disconnecting ASSERT_TIMELY (5s, node_client->telemetry.get_telemetry (channel->get_endpoint ())); - node_server->stop (); + system.stop_node (*node_server); ASSERT_TRUE (channel); // Ensure telemetry from disconnected peer is removed diff --git a/nano/test_common/system.cpp b/nano/test_common/system.cpp index 46c72dd10..3005d4c5e 100644 --- a/nano/test_common/system.cpp +++ b/nano/test_common/system.cpp @@ -211,6 +211,24 @@ std::shared_ptr nano::test::system::make_disconnected_node (std::opt return node; } +void nano::test::system::register_node (std::shared_ptr const & node) +{ + debug_assert (std::find (nodes.begin (), nodes.end (), node) == nodes.end ()); + nodes.push_back (node); +} + +void nano::test::system::stop_node (nano::node & node) +{ + auto stopped = std::async (std::launch::async, [&node] () { + node.stop (); + }); + auto ec = poll_until_true (5s, [&] () { + auto status = stopped.wait_for (0s); + return status == std::future_status::ready; + }); + debug_assert (!ec); +} + void nano::test::system::ledger_initialization_set (std::vector const & reps, nano::amount const & reserve) { nano::block_hash previous = nano::dev::genesis->hash (); diff --git a/nano/test_common/system.hpp b/nano/test_common/system.hpp index dffa0a47d..e0c9f127c 100644 --- a/nano/test_common/system.hpp +++ b/nano/test_common/system.hpp @@ -25,6 +25,8 @@ namespace test system (uint16_t, nano::transport::transport_type = nano::transport::transport_type::tcp, nano::node_flags = nano::node_flags ()); ~system (); + void stop (); + void ledger_initialization_set (std::vector const & reps, nano::amount const & reserve = 0); void generate_activity (nano::node &, std::vector &); void generate_mass_activity (uint32_t, nano::node &); @@ -50,7 +52,6 @@ namespace test std::error_code poll (std::chrono::nanoseconds const & sleep_time = std::chrono::milliseconds (50)); std::error_code poll_until_true (std::chrono::nanoseconds deadline, std::function); void delay_ms (std::chrono::milliseconds const & delay); - void stop (); void deadline_set (std::chrono::duration const & delta); /* * Convenience function to get a reference to a node at given index. Does bound checking. @@ -61,6 +62,8 @@ namespace test // Make an independent node that uses system resources but is not part of the system node list and does not automatically connect to other nodes std::shared_ptr make_disconnected_node (std::optional opt_node_config = std::nullopt, nano::node_flags = nano::node_flags ()); + void register_node (std::shared_ptr const &); + void stop_node (nano::node &); /* * Returns default config for node running in test environment