From 0b2fc82a6f22e26d7d060cb0463f18e53b5b6967 Mon Sep 17 00:00:00 2001 From: Dimitrios Siganos Date: Tue, 23 Jan 2024 09:40:05 +0700 Subject: [PATCH] Simplify system::get_available_port() The function get_available_port is too confusing. To simplify it, I am refactoring one test job it has into a separate test function. The test function it has is to speculatively choose a free tcp binding port, for one unit test case, to check tat the node can be configured with a manual port. --- nano/core_test/network.cpp | 3 +- nano/test_common/network.cpp | 25 ++++++++++++++++ nano/test_common/network.hpp | 4 +++ nano/test_common/system.cpp | 57 +++++++++--------------------------- nano/test_common/system.hpp | 7 ++++- 5 files changed, 51 insertions(+), 45 deletions(-) diff --git a/nano/core_test/network.cpp b/nano/core_test/network.cpp index 6804e7bb50..cdb15ee648 100644 --- a/nano/core_test/network.cpp +++ b/nano/core_test/network.cpp @@ -57,7 +57,8 @@ TEST (network, tcp_connection) TEST (network, construction_with_specified_port) { nano::test::system system{}; - auto const port = system.get_available_port (/* do not allow 0 port */ false); + auto const port = nano::test::speculatively_choose_a_free_tcp_bind_port (); + ASSERT_NE (port, 0); auto const node = system.add_node (nano::node_config{ port, system.logging }); EXPECT_EQ (port, node->network.port); EXPECT_EQ (port, node->network.endpoint ().port ()); diff --git a/nano/test_common/network.cpp b/nano/test_common/network.cpp index 3241afca61..bd90f0f5c3 100644 --- a/nano/test_common/network.cpp +++ b/nano/test_common/network.cpp @@ -31,3 +31,28 @@ std::shared_ptr nano::test::add_outer_node (nano::test::system & sys system_a.nodes.push_back (outer_node); return outer_node; } + +// Note: this is not guaranteed to work, it is speculative +uint16_t nano::test::speculatively_choose_a_free_tcp_bind_port () +{ + /* + * This works because the kernel doesn't seem to reuse port numbers until it absolutely has to. + * Subsequent binds to port 0 will allocate a different port number. + */ + boost::asio::io_context io_ctx; + boost::asio::ip::tcp::acceptor acceptor{ io_ctx }; + boost::asio::ip::tcp::tcp::endpoint endpoint{ boost::asio::ip::tcp::v4 (), 0 }; + acceptor.open (endpoint.protocol ()); + + boost::asio::socket_base::reuse_address option{ true }; + acceptor.set_option (option); // set SO_REUSEADDR option + + acceptor.bind (endpoint); + + auto actual_endpoint = acceptor.local_endpoint (); + auto port = actual_endpoint.port (); + + acceptor.close (); + + return port; +} \ No newline at end of file diff --git a/nano/test_common/network.hpp b/nano/test_common/network.hpp index ddfb3619cd..c8835139fe 100644 --- a/nano/test_common/network.hpp +++ b/nano/test_common/network.hpp @@ -18,7 +18,11 @@ namespace test class system; /** Waits until a TCP connection is established and returns the TCP channel on success*/ std::shared_ptr establish_tcp (nano::test::system &, nano::node &, nano::endpoint const &); + /** Adds a node to the system without establishing connections */ std::shared_ptr add_outer_node (nano::test::system & system, nano::node_flags = nano::node_flags ()); + + /** speculatively (it is not guaranteed that the port will remain free) find a free tcp binding port and return it */ + uint16_t speculatively_choose_a_free_tcp_bind_port (); } } diff --git a/nano/test_common/system.cpp b/nano/test_common/system.cpp index b0f2ff1dfc..e70648af6c 100644 --- a/nano/test_common/system.cpp +++ b/nano/test_common/system.cpp @@ -567,56 +567,27 @@ nano::node_config nano::test::system::default_config () return config; } -uint16_t nano::test::system::get_available_port (bool can_be_zero) +uint16_t nano::test::system::get_available_port () { auto base_port_str = std::getenv ("NANO_TEST_BASE_PORT"); - if (base_port_str) - { - // Maximum possible sockets which may feasibly be used in 1 test - constexpr auto max = 200; - static uint16_t current = 0; - // Read the TEST_BASE_PORT environment and override the default base port if it exists - uint16_t base_port = boost::lexical_cast (base_port_str); - - uint16_t const available_port = base_port + current; - ++current; - // Reset port number once we have reached the maximum - if (current == max) - { - current = 0; - } + if (!base_port_str) + return 0; // let the O/S decide - return available_port; - } - else - { - if (!can_be_zero) - { - /* - * This works because the kernel doesn't seem to reuse port numbers until it absolutely has to. - * Subsequent binds to port 0 will allocate a different port number. - */ - boost::asio::ip::tcp::acceptor acceptor{ io_ctx }; - boost::asio::ip::tcp::tcp::endpoint endpoint{ boost::asio::ip::tcp::v4 (), 0 }; - acceptor.open (endpoint.protocol ()); + // Maximum possible sockets which may feasibly be used in 1 test + constexpr auto max = 200; + static uint16_t current = 0; - boost::asio::socket_base::reuse_address option{ true }; - acceptor.set_option (option); // set SO_REUSEADDR option + // Read the TEST_BASE_PORT environment and override the default base port if it exists + uint16_t base_port = boost::lexical_cast (base_port_str); - acceptor.bind (endpoint); + uint16_t const available_port = base_port + current; + ++current; - auto actual_endpoint = acceptor.local_endpoint (); - auto port = actual_endpoint.port (); + // Reset port number once we have reached the maximum + if (current >= max) + current = 0; - acceptor.close (); - - return port; - } - else - { - return 0; - } - } + return available_port; } void nano::test::cleanup_dev_directories_on_exit () diff --git a/nano/test_common/system.hpp b/nano/test_common/system.hpp index e03532ccef..4b197b95ae 100644 --- a/nano/test_common/system.hpp +++ b/nano/test_common/system.hpp @@ -61,7 +61,12 @@ namespace test * Returns default config for node running in test environment */ nano::node_config default_config (); - uint16_t get_available_port (bool can_be_zero = true); + + /* + * Returns port 0 by default, to let the O/S choose a port number. + * If NANO_TEST_BASE_PORT is set then it allocates numbers by itself from that range. + */ + uint16_t get_available_port (); public: boost::asio::io_context io_ctx;