Skip to content

Commit e5807b2

Browse files
FalessFaless
committed
[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).
1 parent aa8d9b8 commit e5807b2

File tree

2 files changed

+123
-165
lines changed

2 files changed

+123
-165
lines changed

‎tests/core/io/test_tcp_server.h

+70-38
Original file line numberDiff line numberDiff line change
@@ -35,12 +35,21 @@
3535
#include "core/io/tcp_server.h"
3636
#include "tests/test_macros.h"
3737

38+
#include <functional>
39+
3840
namespace TestTCPServer {
3941

4042
const int PORT = 12345;
4143
const IPAddress LOCALHOST("127.0.0.1");
4244
const uint32_t SLEEP_DURATION = 1000;
43-
const uint64_t MAX_WAIT_USEC = 100000;
45+
const uint64_t MAX_WAIT_USEC = 2000000;
46+
47+
void wait_for_condition(std::function<bool()> f_test) {
48+
const uint64_t time = OS::get_singleton()->get_ticks_usec();
49+
while (!f_test() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) {
50+
OS::get_singleton()->delay_usec(SLEEP_DURATION);
51+
}
52+
}
4453

4554
Ref<TCPServer> create_server(const IPAddress &p_address, int p_port) {
4655
Ref<TCPServer> server;
@@ -66,11 +75,9 @@ Ref<StreamPeerTCP> create_client(const IPAddress &p_address, int p_port) {
6675
}
6776

6877
Ref<StreamPeerTCP> accept_connection(Ref<TCPServer> &p_server) {
69-
// Required to get the connection properly established.
70-
const uint64_t time = OS::get_singleton()->get_ticks_usec();
71-
while (!p_server->is_connection_available() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) {
72-
OS::get_singleton()->delay_usec(SLEEP_DURATION);
73-
}
78+
wait_for_condition([&]() {
79+
return p_server->is_connection_available();
80+
});
7481

7582
REQUIRE(p_server->is_connection_available());
7683
Ref<StreamPeerTCP> client_from_server = p_server->take_connection();
@@ -81,16 +88,6 @@ Ref<StreamPeerTCP> accept_connection(Ref<TCPServer> &p_server) {
8188
return client_from_server;
8289
}
8390

84-
Error poll(Ref<StreamPeerTCP> p_client) {
85-
const uint64_t time = OS::get_singleton()->get_ticks_usec();
86-
Error err = p_client->poll();
87-
while (err != Error::OK && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) {
88-
OS::get_singleton()->delay_usec(SLEEP_DURATION);
89-
err = p_client->poll();
90-
}
91-
return err;
92-
}
93-
9491
TEST_CASE("[TCPServer] Instantiation") {
9592
Ref<TCPServer> server;
9693
server.instantiate();
@@ -104,7 +101,10 @@ TEST_CASE("[TCPServer] Accept a connection and receive/send data") {
104101
Ref<StreamPeerTCP> client = create_client(LOCALHOST, PORT);
105102
Ref<StreamPeerTCP> client_from_server = accept_connection(server);
106103

107-
REQUIRE_EQ(poll(client), Error::OK);
104+
wait_for_condition([&]() {
105+
return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_CONNECTED;
106+
});
107+
108108
CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_CONNECTED);
109109

110110
// Sending data from client to server.
@@ -135,9 +135,25 @@ TEST_CASE("[TCPServer] Handle multiple clients at the same time") {
135135
clients_from_server.push_back(accept_connection(server));
136136
}
137137

138-
// Calling poll() to update client status.
138+
wait_for_condition([&]() {
139+
bool should_exit = true;
140+
for (Ref<StreamPeerTCP> &c : clients) {
141+
if (c->poll() != Error::OK) {
142+
return true;
143+
}
144+
StreamPeerTCP::Status status = c->get_status();
145+
if (status != StreamPeerTCP::STATUS_CONNECTED && status != StreamPeerTCP::STATUS_CONNECTING) {
146+
return true;
147+
}
148+
if (status != StreamPeerTCP::STATUS_CONNECTED) {
149+
should_exit = false;
150+
}
151+
}
152+
return should_exit;
153+
});
154+
139155
for (Ref<StreamPeerTCP> &c : clients) {
140-
REQUIRE_EQ(poll(c), Error::OK);
156+
REQUIRE_EQ(c->get_status(), StreamPeerTCP::STATUS_CONNECTED);
141157
}
142158

143159
// Sending data from each client to server.
@@ -158,7 +174,10 @@ TEST_CASE("[TCPServer] When stopped shouldn't accept new connections") {
158174
Ref<StreamPeerTCP> client = create_client(LOCALHOST, PORT);
159175
Ref<StreamPeerTCP> client_from_server = accept_connection(server);
160176

161-
REQUIRE_EQ(poll(client), Error::OK);
177+
wait_for_condition([&]() {
178+
return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_CONNECTED;
179+
});
180+
162181
CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_CONNECTED);
163182

164183
// Sending data from client to server.
@@ -170,35 +189,37 @@ TEST_CASE("[TCPServer] When stopped shouldn't accept new connections") {
170189
server->stop();
171190
CHECK_FALSE(server->is_listening());
172191

192+
// Make sure the client times out in less than the wait time.
193+
int timeout = ProjectSettings::get_singleton()->get_setting("network/limits/tcp/connect_timeout_seconds");
194+
ProjectSettings::get_singleton()->set_setting("network/limits/tcp/connect_timeout_seconds", 1);
195+
173196
Ref<StreamPeerTCP> new_client = create_client(LOCALHOST, PORT);
174197

175-
// Required to get the connection properly established.
176-
uint64_t time = OS::get_singleton()->get_ticks_usec();
177-
while (!server->is_connection_available() && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) {
178-
OS::get_singleton()->delay_usec(SLEEP_DURATION);
179-
}
198+
// Reset the timeout setting.
199+
ProjectSettings::get_singleton()->set_setting("network/limits/tcp/connect_timeout_seconds", timeout);
180200

181201
CHECK_FALSE(server->is_connection_available());
182202

183-
time = OS::get_singleton()->get_ticks_usec();
184-
Error err = new_client->poll();
185-
while (err != Error::OK && err != Error::ERR_CONNECTION_ERROR && (OS::get_singleton()->get_ticks_usec() - time) < MAX_WAIT_USEC) {
186-
OS::get_singleton()->delay_usec(SLEEP_DURATION);
187-
err = new_client->poll();
188-
}
189-
REQUIRE((err == Error::OK || err == Error::ERR_CONNECTION_ERROR));
190-
StreamPeerTCP::Status status = new_client->get_status();
191-
CHECK((status == StreamPeerTCP::STATUS_CONNECTING || status == StreamPeerTCP::STATUS_ERROR));
203+
wait_for_condition([&]() {
204+
return new_client->poll() != Error::OK || new_client->get_status() == StreamPeerTCP::STATUS_ERROR;
205+
});
192206

207+
CHECK_FALSE(server->is_connection_available());
208+
209+
CHECK_EQ(new_client->get_status(), StreamPeerTCP::STATUS_ERROR);
193210
new_client->disconnect_from_host();
211+
CHECK_EQ(new_client->get_status(), StreamPeerTCP::STATUS_NONE);
194212
}
195213

196214
TEST_CASE("[TCPServer] Should disconnect client") {
197215
Ref<TCPServer> server = create_server(LOCALHOST, PORT);
198216
Ref<StreamPeerTCP> client = create_client(LOCALHOST, PORT);
199217
Ref<StreamPeerTCP> client_from_server = accept_connection(server);
200218

201-
REQUIRE_EQ(poll(client), Error::OK);
219+
wait_for_condition([&]() {
220+
return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_CONNECTED;
221+
});
222+
202223
CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_CONNECTED);
203224

204225
// Sending data from client to server.
@@ -210,12 +231,23 @@ TEST_CASE("[TCPServer] Should disconnect client") {
210231
server->stop();
211232
CHECK_FALSE(server->is_listening());
212233

213-
// Reading for a closed connection will print an error.
234+
// Wait for disconnection
235+
wait_for_condition([&]() {
236+
return client->poll() != Error::OK || client->get_status() == StreamPeerTCP::STATUS_NONE;
237+
});
238+
239+
// Wait for disconnection
240+
wait_for_condition([&]() {
241+
return client_from_server->poll() != Error::OK || client_from_server->get_status() == StreamPeerTCP::STATUS_NONE;
242+
});
243+
244+
CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_NONE);
245+
CHECK_EQ(client_from_server->get_status(), StreamPeerTCP::STATUS_NONE);
246+
214247
ERR_PRINT_OFF;
215248
CHECK_EQ(client->get_string(), String());
249+
CHECK_EQ(client_from_server->get_string(), String());
216250
ERR_PRINT_ON;
217-
REQUIRE_EQ(poll(client), Error::OK);
218-
CHECK_EQ(client->get_status(), StreamPeerTCP::STATUS_NONE);
219251
}
220252

221253
} // namespace TestTCPServer

0 commit comments

Comments
 (0)