[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

@@ -35,12 +35,21 @@
 #include "core/io/tcp_server.h"
 #include "tests/test_macros.h"
 
+#include <functional>
+
 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<bool()> 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<TCPServer> create_server(const IPAddress &p_address, int p_port) {
 	Ref<TCPServer> server;
@@ -66,11 +75,9 @@ Ref<StreamPeerTCP> create_client(const IPAddress &p_address, int p_port) {
 }
 
 Ref<StreamPeerTCP> accept_connection(Ref<TCPServer> &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<StreamPeerTCP> client_from_server = p_server->take_connection();
@@ -81,16 +88,6 @@ Ref<StreamPeerTCP> accept_connection(Ref<TCPServer> &p_server) {
 	return client_from_server;
 }
 
-Error poll(Ref<StreamPeerTCP> 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<TCPServer> server;
 	server.instantiate();
@@ -104,7 +101,10 @@ TEST_CASE("[TCPServer] Accept a connection and receive/send data") {
 	Ref<StreamPeerTCP> client = create_client(LOCALHOST, PORT);
 	Ref<StreamPeerTCP> 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<StreamPeerTCP> &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<StreamPeerTCP> &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<StreamPeerTCP> client = create_client(LOCALHOST, PORT);
 	Ref<StreamPeerTCP> 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<StreamPeerTCP> 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<StreamPeerTCP> client = create_client(LOCALHOST, PORT);
 	Ref<StreamPeerTCP> 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

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<bool()> 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<UDPServer> create_server(const IPAddress &p_address, int p_port) {
 	Ref<UDPServer> server;
 	server.instantiate();
@@ -68,12 +75,9 @@ Ref<PacketPeerUDP> create_client(const IPAddress &p_address, int p_port) {
 }
 
 Ref<PacketPeerUDP> accept_connection(Ref<UDPServer> &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<PacketPeerUDP> accept_connection(Ref<UDPServer> &p_server) {
 	return client_from_server;
 }
 
-Error poll(Ref<UDPServer> 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<UDPServer> 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<PacketPeerUDP> 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<PacketPeerUDP> 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<PacketPeerUDP> 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<PacketPeerUDP> &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<PacketPeerUDP> &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<UDPServer> server = create_server(LOCALHOST, PORT);
 	Ref<PacketPeerUDP> 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<PacketPeerUDP> 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<PacketPeerUDP> 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<UDPServer> server = create_server(LOCALHOST, PORT);
-	Ref<PacketPeerUDP> 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<PacketPeerUDP> 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<UDPServer> server = create_server(LOCALHOST, PORT);
-	server->set_max_pending_connections(3);
-
-	Vector<Ref<PacketPeerUDP>> clients;
-	for (int i = 0; i < 5; i++) {
-		Ref<PacketPeerUDP> 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<PacketPeerUDP> 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<PacketPeerUDP> 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