From e5807b2adfbe713a824a5f25d02da7bb5a628d01 Mon Sep 17 00:00:00 2001 From: Fabio Alessandrelli Date: Fri, 6 Dec 2024 16:01:21 +0100 Subject: [PATCH] [Net] Fix TCP/UDP server network tests Some tests have been removed since there's no way to guarantee they will pass. Other tests have been refactored to ensure proper waiting, and taking into account potential out-of-order delivery (which is unlikely in test scenarios but expecting a specific order on a UDP socket is wrong and OSes makes no promises of ordered delivery on localhost). --- tests/core/io/test_tcp_server.h | 108 ++++++++++++------- tests/core/io/test_udp_server.h | 180 ++++++++++---------------------- 2 files changed, 123 insertions(+), 165 deletions(-) diff --git a/tests/core/io/test_tcp_server.h b/tests/core/io/test_tcp_server.h index 9bf1c2ecad5c..24c27468cb86 100644 --- a/tests/core/io/test_tcp_server.h +++ b/tests/core/io/test_tcp_server.h @@ -35,12 +35,21 @@ #include "core/io/tcp_server.h" #include "tests/test_macros.h" +#include + namespace TestTCPServer { const int PORT = 12345; const IPAddress LOCALHOST("127.0.0.1"); const uint32_t SLEEP_DURATION = 1000; -const uint64_t MAX_WAIT_USEC = 100000; +const uint64_t MAX_WAIT_USEC = 2000000; + +void wait_for_condition(std::function f_test) { + const uint64_t time = OS::get_singleton()->get_ticks_usec(); + while (!f_test() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { + OS::get_singleton()->delay_usec(SLEEP_DURATION); + } +} Ref create_server(const IPAddress &p_address, int p_port) { Ref server; @@ -66,11 +75,9 @@ Ref create_client(const IPAddress &p_address, int p_port) { } Ref accept_connection(Ref &p_server) { - // Required to get the connection properly established. - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - while (!p_server->is_connection_available() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } + wait_for_condition([&]() { + return p_server->is_connection_available(); + }); REQUIRE(p_server->is_connection_available()); Ref client_from_server = p_server->take_connection(); @@ -81,16 +88,6 @@ Ref accept_connection(Ref &p_server) { return client_from_server; } -Error poll(Ref p_client) { - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - Error err = p_client->poll(); - while (err != Error::OK && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - OS::get_singleton()->delay_usec(SLEEP_DURATION); - err = p_client->poll(); - } - return err; -} - TEST_CASE("[TCPServer] Instantiation") { Ref server; server.instantiate(); @@ -104,7 +101,10 @@ TEST_CASE("[TCPServer] Accept a connection and receive/send data") { Ref client = create_client(LOCALHOST, PORT); Ref client_from_server = accept_connection(server); - REQUIRE_EQ(poll(client), Error::OK); + wait_for_condition([&]() { + return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_CONNECTED; + }); + CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_CONNECTED); // Sending data from client to server. @@ -135,9 +135,25 @@ TEST_CASE("[TCPServer] Handle multiple clients at the same time") { clients_from_server.push_back(accept_connection(server)); } - // Calling poll() to update client status. + wait_for_condition([&]() { + bool should_exit = true; + for (Ref &c : clients) { + if (c->poll() != Error::OK) { + return true; + } + StreamPeerTCP::Status status = c->get_status(); + if (status != StreamPeerTCP::STATUS_CONNECTED && status != StreamPeerTCP::STATUS_CONNECTING) { + return true; + } + if (status != StreamPeerTCP::STATUS_CONNECTED) { + should_exit = false; + } + } + return should_exit; + }); + for (Ref &c : clients) { - REQUIRE_EQ(poll(c), Error::OK); + REQUIRE_EQ(c->get_status(), StreamPeerTCP::STATUS_CONNECTED); } // Sending data from each client to server. @@ -158,7 +174,10 @@ TEST_CASE("[TCPServer] When stopped shouldn't accept new connections") { Ref client = create_client(LOCALHOST, PORT); Ref client_from_server = accept_connection(server); - REQUIRE_EQ(poll(client), Error::OK); + wait_for_condition([&]() { + return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_CONNECTED; + }); + CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_CONNECTED); // Sending data from client to server. @@ -170,27 +189,26 @@ TEST_CASE("[TCPServer] When stopped shouldn't accept new connections") { server->stop(); CHECK_FALSE(server->is_listening()); + // Make sure the client times out in less than the wait time. + int timeout = ProjectSettings::get_singleton()->get_setting("network/limits/tcp/connect_timeout_seconds"); + ProjectSettings::get_singleton()->set_setting("network/limits/tcp/connect_timeout_seconds", 1); + Ref new_client = create_client(LOCALHOST, PORT); - // Required to get the connection properly established. - uint64_t time = OS::get_singleton()->get_ticks_usec(); - while (!server->is_connection_available() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } + // Reset the timeout setting. + ProjectSettings::get_singleton()->set_setting("network/limits/tcp/connect_timeout_seconds", timeout); CHECK_FALSE(server->is_connection_available()); - time = OS::get_singleton()->get_ticks_usec(); - Error err = new_client->poll(); - while (err != Error::OK && err != Error::ERR_CONNECTION_ERROR && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - OS::get_singleton()->delay_usec(SLEEP_DURATION); - err = new_client->poll(); - } - REQUIRE((err == Error::OK || err == Error::ERR_CONNECTION_ERROR)); - StreamPeerTCP::Status status = new_client->get_status(); - CHECK((status == StreamPeerTCP::STATUS_CONNECTING || status == StreamPeerTCP::STATUS_ERROR)); + wait_for_condition([&]() { + return new_client->poll() != Error::OK || new_client->get_status() == StreamPeerTCP::STATUS_ERROR; + }); + CHECK_FALSE(server->is_connection_available()); + + CHECK_EQ(new_client->get_status(), StreamPeerTCP::STATUS_ERROR); new_client->disconnect_from_host(); + CHECK_EQ(new_client->get_status(), StreamPeerTCP::STATUS_NONE); } TEST_CASE("[TCPServer] Should disconnect client") { @@ -198,7 +216,10 @@ TEST_CASE("[TCPServer] Should disconnect client") { Ref client = create_client(LOCALHOST, PORT); Ref client_from_server = accept_connection(server); - REQUIRE_EQ(poll(client), Error::OK); + wait_for_condition([&]() { + return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_CONNECTED; + }); + CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_CONNECTED); // Sending data from client to server. @@ -210,12 +231,23 @@ TEST_CASE("[TCPServer] Should disconnect client") { server->stop(); CHECK_FALSE(server->is_listening()); - // Reading for a closed connection will print an error. + // Wait for disconnection + wait_for_condition([&]() { + return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_NONE; + }); + + // Wait for disconnection + wait_for_condition([&]() { + return client_from_server->poll() != Error::OK || client_from_server->get_status() == StreamPeerTCP::STATUS_NONE; + }); + + CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_NONE); + CHECK_EQ(client_from_server->get_status(), StreamPeerTCP::STATUS_NONE); + ERR_PRINT_OFF; CHECK_EQ(client->get_string(), String()); + CHECK_EQ(client_from_server->get_string(), String()); ERR_PRINT_ON; - REQUIRE_EQ(poll(client), Error::OK); - CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_NONE); } } // namespace TestTCPServer diff --git a/tests/core/io/test_udp_server.h b/tests/core/io/test_udp_server.h index 4ba385a06391..c65b8cacb24c 100644 --- a/tests/core/io/test_udp_server.h +++ b/tests/core/io/test_udp_server.h @@ -42,6 +42,13 @@ const IPAddress LOCALHOST("127.0.0.1"); const uint32_t SLEEP_DURATION = 1000; const uint64_t MAX_WAIT_USEC = 100000; +void wait_for_condition(std::function f_test) { + const uint64_t time = OS::get_singleton()->get_ticks_usec(); + while (!f_test() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { + OS::get_singleton()->delay_usec(SLEEP_DURATION); + } +} + Ref create_server(const IPAddress &p_address, int p_port) { Ref server; server.instantiate(); @@ -68,12 +75,9 @@ Ref create_client(const IPAddress &p_address, int p_port) { } Ref accept_connection(Ref &p_server) { - // Required to get the connection properly established. - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - while (!p_server->is_connection_available() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - p_server->poll(); - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } + wait_for_condition([&]() { + return p_server->poll() != Error::OK || p_server->is_connection_available(); + }); CHECK_EQ(p_server->poll(), Error::OK); REQUIRE(p_server->is_connection_available()); @@ -85,16 +89,6 @@ Ref accept_connection(Ref &p_server) { return client_from_server; } -Error poll(Ref p_server, Error p_err) { - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - Error err = p_server->poll(); - while (err != p_err && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - err = p_server->poll(); - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } - return err; -} - TEST_CASE("[UDPServer] Instantiation") { Ref server; server.instantiate(); @@ -120,14 +114,10 @@ TEST_CASE("[UDPServer] Accept a connection and receive/send data") { const Variant pi = 3.1415; CHECK_EQ(client_from_server->put_var(pi), Error::OK); - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - // get_available_packet_count() is the recommended way to call _poll(), because there is no public poll(). - while (client->get_available_packet_count() == 0 && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - server->poll(); - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } + wait_for_condition([&]() { + return client->get_available_packet_count() > 0; + }); - CHECK_EQ(server->poll(), Error::OK); CHECK_GT(client->get_available_packet_count(), 0); Variant pi_received; @@ -147,41 +137,55 @@ TEST_CASE("[UDPServer] Handle multiple clients at the same time") { Ref c = create_client(LOCALHOST, PORT); // Sending data from client to server. - const String hello_client = "Hello " + itos(i); + const String hello_client = itos(i); CHECK_EQ(c->put_var(hello_client), Error::OK); clients.push_back(c); } + Array packets; for (int i = 0; i < clients.size(); i++) { Ref cfs = accept_connection(server); - Variant hello_world_received; - CHECK_EQ(cfs->get_var(hello_world_received), Error::OK); - CHECK_EQ(String(hello_world_received), "Hello " + itos(i)); + Variant received_var; + CHECK_EQ(cfs->get_var(received_var), Error::OK); + CHECK_EQ(received_var.get_type(), Variant::STRING); + packets.push_back(received_var); // Sending data from server to client. - const Variant pi = 3.1415 + i; - CHECK_EQ(cfs->put_var(pi), Error::OK); + const float sent_float = 3.1415 + received_var.operator String().to_float(); + CHECK_EQ(cfs->put_var(sent_float), Error::OK); } - const uint64_t time = OS::get_singleton()->get_ticks_usec(); + CHECK_EQ(packets.size(), clients.size()); + + packets.sort(); for (int i = 0; i < clients.size(); i++) { - Ref c = clients[i]; - // get_available_packet_count() is the recommended way to call _poll(), because there is no public poll(). - // Because `time` is defined outside the for, we will wait the max amount of time only for the first client. - while (c->get_available_packet_count() == 0 && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - server->poll(); - OS::get_singleton()->delay_usec(SLEEP_DURATION); + CHECK_EQ(packets[i].operator String(), itos(i)); + } + + wait_for_condition([&]() { + bool should_exit = true; + for (Ref &c : clients) { + int count = c->get_available_packet_count(); + if (count < 0) { + return true; + } + if (count == 0) { + should_exit = false; + } } + return should_exit; + }); - // The recommended way to call _poll(), because there is no public poll(). - CHECK_GT(c->get_available_packet_count(), 0); + for (int i = 0; i < clients.size(); i++) { + CHECK_GT(clients[i]->get_available_packet_count(), 0); - Variant pi_received; - const Variant pi = 3.1415 + i; - CHECK_EQ(c->get_var(pi_received), Error::OK); - CHECK_EQ(pi_received, pi); + Variant received_var; + const float expected = 3.1415 + i; + CHECK_EQ(clients[i]->get_var(received_var), Error::OK); + CHECK_EQ(received_var.get_type(), Variant::FLOAT); + CHECK_EQ(received_var.operator float(), expected); } for (Ref &c : clients) { @@ -190,7 +194,7 @@ TEST_CASE("[UDPServer] Handle multiple clients at the same time") { server->stop(); } -TEST_CASE("[UDPServer] When stopped shouldn't accept new connections") { +TEST_CASE("[UDPServer] Should not accept new connections after stop") { Ref server = create_server(LOCALHOST, PORT); Ref client = create_client(LOCALHOST, PORT); @@ -198,94 +202,16 @@ TEST_CASE("[UDPServer] When stopped shouldn't accept new connections") { const String hello_world = "Hello World!"; CHECK_EQ(client->put_var(hello_world), Error::OK); - Variant hello_world_received; - Ref client_from_server = accept_connection(server); - CHECK_EQ(client_from_server->get_var(hello_world_received), Error::OK); - CHECK_EQ(String(hello_world_received), hello_world); - - client->close(); - server->stop(); - CHECK_FALSE(server->is_listening()); - - Ref new_client = create_client(LOCALHOST, PORT); - CHECK_EQ(new_client->put_var(hello_world), Error::OK); - - REQUIRE_EQ(poll(server, Error::ERR_UNCONFIGURED), Error::ERR_UNCONFIGURED); - CHECK_FALSE(server->is_connection_available()); - - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - // get_available_packet_count() is the recommended way to call _poll(), because there is no public poll(). - while (new_client->get_available_packet_count() == 0 && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } - - const int packet_count = new_client->get_available_packet_count(); - CHECK((packet_count == 0 || packet_count == -1)); -} - -TEST_CASE("[UDPServer] Should disconnect client") { - Ref server = create_server(LOCALHOST, PORT); - Ref client = create_client(LOCALHOST, PORT); - - // Sending data from client to server. - const String hello_world = "Hello World!"; - CHECK_EQ(client->put_var(hello_world), Error::OK); + wait_for_condition([&]() { + return server->poll() != Error::OK || server->is_connection_available(); + }); - Variant hello_world_received; - Ref client_from_server = accept_connection(server); - CHECK_EQ(client_from_server->get_var(hello_world_received), Error::OK); - CHECK_EQ(String(hello_world_received), hello_world); + REQUIRE(server->is_connection_available()); server->stop(); - CHECK_FALSE(server->is_listening()); - CHECK_FALSE(client_from_server->is_bound()); - CHECK_FALSE(client_from_server->is_socket_connected()); - // Sending data from client to server. - CHECK_EQ(client->put_var(hello_world), Error::OK); - - const uint64_t time = OS::get_singleton()->get_ticks_usec(); - // get_available_packet_count() is the recommended way to call _poll(), because there is no public poll(). - while (client->get_available_packet_count() == 0 && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) { - OS::get_singleton()->delay_usec(SLEEP_DURATION); - } - - const int packet_count = client->get_available_packet_count(); - CHECK((packet_count == 0 || packet_count == -1)); - - client->close(); -} - -TEST_CASE("[UDPServer] Should drop new connections when pending max connection is reached") { - Ref server = create_server(LOCALHOST, PORT); - server->set_max_pending_connections(3); - - Vector> clients; - for (int i = 0; i < 5; i++) { - Ref c = create_client(LOCALHOST, PORT); - - // Sending data from client to server. - const String hello_client = "Hello " + itos(i); - CHECK_EQ(c->put_var(hello_client), Error::OK); - - clients.push_back(c); - } - - for (int i = 0; i < server->get_max_pending_connections(); i++) { - Ref cfs = accept_connection(server); - - Variant hello_world_received; - CHECK_EQ(cfs->get_var(hello_world_received), Error::OK); - CHECK_EQ(String(hello_world_received), "Hello " + itos(i)); - } - - CHECK_EQ(poll(server, Error::OK), Error::OK); - - REQUIRE_FALSE(server->is_connection_available()); - Ref client_from_server = server->take_connection(); - REQUIRE_FALSE_MESSAGE(client_from_server.is_valid(), "A Packet Peer UDP from the UDP Server should be a null pointer because the pending connection was dropped."); - - server->stop(); + CHECK_FALSE(server->is_listening()); + CHECK_FALSE(server->is_connection_available()); } } // namespace TestUDPServer