Skip to content

TCP/UDP greentea tests refactoring and cleanup #9376

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jan 18, 2019
Merged

TCP/UDP greentea tests refactoring and cleanup #9376

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions TESTS/netsocket/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,6 +243,10 @@ content at minimum:
"help" : "Port of echo server",
"value" : "7"
}
"echo-server-discard-port" : {
"help" : "Discard port of echo server",
"value" : "9"
}
}
}
```
Expand Down
46 changes: 22 additions & 24 deletions TESTS/netsocket/tcp/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
using namespace utest::v1;

namespace {
NetworkInterface *net;
Timer tc_bucket; // Timer to limit a test cases run time
}

Expand All @@ -45,11 +44,6 @@ mbed_stats_socket_t tcp_stats[MBED_CONF_NSAPI_SOCKET_STATS_MAX_COUNT];
char tcp_global::rx_buffer[RX_BUFF_SIZE];
char tcp_global::tx_buffer[TX_BUFF_SIZE];

NetworkInterface *get_interface()
{
return net;
}

void drop_bad_packets(TCPSocket &sock, int orig_timeout)
{
nsapi_error_t err;
Expand All @@ -65,46 +59,50 @@ void drop_bad_packets(TCPSocket &sock, int orig_timeout)

static void _ifup()
{
net = NetworkInterface::get_default_instance();
NetworkInterface *net = NetworkInterface::get_default_instance();
nsapi_error_t err = net->connect();
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, err);
printf("MBED: TCPClient IP address is '%s'\n", net->get_ip_address());
}

static void _ifdown()
{
net->disconnect();
NetworkInterface::get_default_instance()->disconnect();
printf("MBED: ifdown\n");
}

nsapi_error_t tcpsocket_connect_to_echo_srv(TCPSocket &sock)
nsapi_error_t tcpsocket_connect_to_srv(TCPSocket &sock, uint16_t port)
{
SocketAddress tcp_addr;

get_interface()->gethostbyname(MBED_CONF_APP_ECHO_SERVER_ADDR, &tcp_addr);
tcp_addr.set_port(MBED_CONF_APP_ECHO_SERVER_PORT);
NetworkInterface::get_default_instance()->gethostbyname(MBED_CONF_APP_ECHO_SERVER_ADDR, &tcp_addr);
tcp_addr.set_port(port);

printf("MBED: Server '%s', port %d\n", tcp_addr.get_ip_address(), tcp_addr.get_port());

nsapi_error_t err = sock.open(get_interface());
nsapi_error_t err = sock.open(NetworkInterface::get_default_instance());
if (err != NSAPI_ERROR_OK) {
printf("Error from sock.open: %d\n", err);
return err;
}

return sock.connect(tcp_addr);
}

nsapi_error_t tcpsocket_connect_to_discard_srv(TCPSocket &sock)
{
SocketAddress tcp_addr;

get_interface()->gethostbyname(MBED_CONF_APP_ECHO_SERVER_ADDR, &tcp_addr);
tcp_addr.set_port(9);

nsapi_error_t err = sock.open(get_interface());
err = sock.connect(tcp_addr);
if (err != NSAPI_ERROR_OK) {
printf("Error from sock.connect: %d\n", err);
return err;
}

return sock.connect(tcp_addr);
return NSAPI_ERROR_OK;
}

nsapi_error_t tcpsocket_connect_to_echo_srv(TCPSocket &sock)
{
return tcpsocket_connect_to_srv(sock, MBED_CONF_APP_ECHO_SERVER_PORT);
}

nsapi_error_t tcpsocket_connect_to_discard_srv(TCPSocket &sock)
{
return tcpsocket_connect_to_srv(sock, MBED_CONF_APP_ECHO_SERVER_DISCARD_PORT);
}

void fill_tx_buffer_ascii(char *buff, size_t len)
Expand Down
5 changes: 3 additions & 2 deletions TESTS/netsocket/tcp/tcpsocket_bind_address.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ void TCPSOCKET_BIND_ADDRESS()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
SocketAddress sockAddr = SocketAddress(get_interface()->get_ip_address(), 80);
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
SocketAddress sockAddr = SocketAddress(NetworkInterface::get_default_instance()->get_ip_address(), 80);
nsapi_error_t bind_result = sock->bind(sockAddr);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
TEST_IGNORE_MESSAGE("bind() not supported");
Expand Down
3 changes: 2 additions & 1 deletion TESTS/netsocket/tcp/tcpsocket_bind_address_invalid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void TCPSOCKET_BIND_ADDRESS_INVALID()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
nsapi_error_t bind_result = sock->bind("190.2.3.4", 1024);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
TEST_IGNORE_MESSAGE("bind() not supported");
Expand Down
3 changes: 2 additions & 1 deletion TESTS/netsocket/tcp/tcpsocket_bind_address_null.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void TCPSOCKET_BIND_ADDRESS_NULL()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
nsapi_error_t bind_result = sock->bind(NULL, 1024);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
TEST_IGNORE_MESSAGE("bind() not supported");
Expand Down
5 changes: 3 additions & 2 deletions TESTS/netsocket/tcp/tcpsocket_bind_address_port.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,9 +36,10 @@ void TCPSOCKET_BIND_ADDRESS_PORT()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
nsapi_error_t bind_result = sock->bind(get_interface()->get_ip_address(), 80);
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
nsapi_error_t bind_result = sock->bind(NetworkInterface::get_default_instance()->get_ip_address(), 80);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
TEST_IGNORE_MESSAGE("bind() not supported");
} else {
Expand Down
3 changes: 2 additions & 1 deletion TESTS/netsocket/tcp/tcpsocket_bind_port.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void TCPSOCKET_BIND_PORT()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
nsapi_error_t bind_result = sock->bind(1024);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
TEST_IGNORE_MESSAGE("bind() not supported");
Expand Down
5 changes: 3 additions & 2 deletions TESTS/netsocket/tcp/tcpsocket_bind_port_fail.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void TCPSOCKET_BIND_PORT_FAIL()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
nsapi_error_t bind_result = sock->bind(1024);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
TEST_IGNORE_MESSAGE("bind() not supported");
Expand All @@ -51,7 +52,7 @@ void TCPSOCKET_BIND_PORT_FAIL()
if (!sock2) {
TEST_FAIL();
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock2->open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock2->open(NetworkInterface::get_default_instance()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_PARAMETER, sock2->bind(1024));

delete sock;
Expand Down
1 change: 1 addition & 0 deletions TESTS/netsocket/tcp/tcpsocket_bind_unopened.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ void TCPSOCKET_BIND_UNOPENED()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
nsapi_error_t bind_result = sock->bind(1024);
if (bind_result == NSAPI_ERROR_UNSUPPORTED) {
Expand Down
3 changes: 2 additions & 1 deletion TESTS/netsocket/tcp/tcpsocket_bind_wrong_type.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ void TCPSOCKET_BIND_WRONG_TYPE()
TCPSocket *sock = new TCPSocket;
if (!sock) {
TEST_FAIL();
return;
}
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock->open(NetworkInterface::get_default_instance()));
char addr_bytes[16] = {0xfe, 0x80, 0xff, 0x1, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0};
SocketAddress sockAddr = SocketAddress(addr_bytes, NSAPI_IPv4, 80);
nsapi_error_t bind_result = sock->bind(sockAddr);
Expand Down
2 changes: 1 addition & 1 deletion TESTS/netsocket/tcp/tcpsocket_connect_invalid.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ using namespace utest::v1;
void TCPSOCKET_CONNECT_INVALID()
{
TCPSocket sock;
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock.open(get_interface()));
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock.open(NetworkInterface::get_default_instance()));

TEST_ASSERT(sock.connect(NULL, 9) < 0);
TEST_ASSERT(sock.connect("", 9) < 0);
Expand Down
86 changes: 51 additions & 35 deletions TESTS/netsocket/tcp/tcpsocket_echotest.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,14 +36,25 @@ static const int pkt_sizes[PKTS] = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10, \
};
TCPSocket sock;
Semaphore tx_sem(0, 1);
events::EventQueue *event_queue;
int bytes2recv;
int bytes2recv_total;

Timer tc_exec_time;
int time_allotted;
bool receive_error;
}

void tcpsocket_echotest_nonblock_receive();

static void _sigio_handler(osThreadId id)
{
osSignalSet(id, SIGNAL_SIGIO);
if (event_queue != NULL) {
event_queue->call(tcpsocket_echotest_nonblock_receive);
} else {
TEST_FAIL_MESSAGE("_sigio_handler running when event_queue is NULL");
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should here either be an else-clause or the event_queue.call() doesn't need to be wrapped on if-clause

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, never mind... Once you register the SIGIO handler it might be that you don't have a handle to event queue yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the remark. I changed the order of test initialization, so that the event queue gets created before we register the sigio handler.
Nevertheless, to be on the safe side, I left the NULL check and added an else statement that raises an error in case the NULL check fails :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok :) This is not a suggestion to change anything but using TEST_FAIL_MESSAGE indicates there is a problem with the implementation under test. But, actually the issue would be with the test case implementation itself. MBED_ASSERT or MBED_ERROR might be good alternatives here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very good point, thanks for that, but I think it makes some sense to stay with TEST_FAIL_MESSAGE.
Firstly, it is not that obvious to me that the error must come from the test code. What if the handler didn't get unregistered correctly or if the EventQueue creation is erroneous?
Also, MBED_ASSERT/ERROR will crash the whole test suite and I find this reaction too serious, given the circumstances.
I hope you're ok with this? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fine by me :)


void TCPSOCKET_ECHOTEST()
Expand Down Expand Up @@ -83,33 +94,31 @@ void TCPSOCKET_ECHOTEST()
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock.close());
}

void tcpsocket_echotest_nonblock_receiver(void *receive_bytes)
void tcpsocket_echotest_nonblock_receive()
{
int bytes2recv = *(int *)receive_bytes;
int recvd;
while (bytes2recv) {
recvd = sock.recv(&(tcp_global::rx_buffer[*(int *)receive_bytes - bytes2recv]), bytes2recv);
if (recvd == NSAPI_ERROR_WOULD_BLOCK) {
if (tc_exec_time.read() >= time_allotted) {
TEST_FAIL();
break;
}
wait(1);
continue;
} else if (recvd < 0) {
TEST_FAIL();
break;
int recvd = sock.recv(&(tcp_global::rx_buffer[bytes2recv_total - bytes2recv]), bytes2recv);
if (recvd == NSAPI_ERROR_WOULD_BLOCK) {
if (tc_exec_time.read() >= time_allotted) {
receive_error = true;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tx_sem isn't released here. Sender might end up waiting forever as the timeout was removed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't really want to release the semaphore in case of WOULD_BLOCK - we want to wait until the data arrives. Therefore, I added the timeout back, but fixed it to the correct units.
I hope this is fine? :)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok, that's fine.

}
return;
} else if (recvd < 0) {
receive_error = true;
} else {
bytes2recv -= recvd;
}

TEST_ASSERT_EQUAL(0, memcmp(tcp_global::tx_buffer, tcp_global::rx_buffer, *(int *)receive_bytes));

static int round = 0;
printf("[Recevr#%02d] bytes received: %d\n", round++, *(int *)receive_bytes);

tx_sem.release();
if (bytes2recv == 0) {
TEST_ASSERT_EQUAL(0, memcmp(tcp_global::tx_buffer, tcp_global::rx_buffer, bytes2recv_total));

static int round = 0;
printf("[Recevr#%02d] bytes received: %d\n", round++, bytes2recv_total);
tx_sem.release();
} else if (receive_error || bytes2recv < 0) {
TEST_FAIL();
tx_sem.release();
}
// else - no error, not all bytes were received yet.
}

void TCPSOCKET_ECHOTEST_NONBLOCK()
Expand All @@ -124,24 +133,30 @@ void TCPSOCKET_ECHOTEST_NONBLOCK()
tc_exec_time.start();
time_allotted = split2half_rmng_tcp_test_time(); // [s]

EventQueue queue(2 * EVENTS_EVENT_SIZE);
event_queue = &queue;

tcpsocket_connect_to_echo_srv(sock);
sock.set_blocking(false);
sock.sigio(callback(_sigio_handler, ThisThread::get_id()));

int bytes2send;
int sent;
int s_idx = 0;
Thread *thread;
receive_error = false;
unsigned char *stack_mem = (unsigned char *)malloc(tcp_global::TCP_OS_STACK_SIZE);
TEST_ASSERT_NOT_NULL(stack_mem);
Thread *receiver_thread = new Thread(osPriorityNormal,
tcp_global::TCP_OS_STACK_SIZE,
stack_mem,
"receiver");

TEST_ASSERT_EQUAL(osOK, receiver_thread->start(callback(&queue, &EventQueue::dispatch_forever)));

for (int pkt_s = pkt_sizes[s_idx]; s_idx < PKTS; ++s_idx) {
pkt_s = pkt_sizes[s_idx];
thread = new Thread(osPriorityNormal,
tcp_global::TCP_OS_STACK_SIZE,
stack_mem,
"receiver");
TEST_ASSERT_EQUAL(osOK, thread->start(callback(tcpsocket_echotest_nonblock_receiver, &pkt_s)));
bytes2recv = pkt_s;
bytes2recv_total = pkt_s;

fill_tx_buffer_ascii(tcp_global::tx_buffer, pkt_s);

Expand All @@ -151,16 +166,12 @@ void TCPSOCKET_ECHOTEST_NONBLOCK()
if (sent == NSAPI_ERROR_WOULD_BLOCK) {
if (tc_exec_time.read() >= time_allotted ||
osSignalWait(SIGNAL_SIGIO, SIGIO_TIMEOUT).status == osEventTimeout) {
thread->terminate();
delete thread;
TEST_FAIL();
goto END;
}
continue;
} else if (sent <= 0) {
printf("[Sender#%02d] network error %d\n", s_idx, sent);
thread->terminate();
delete thread;
TEST_FAIL();
goto END;
}
Expand All @@ -176,12 +187,17 @@ void TCPSOCKET_ECHOTEST_NONBLOCK()
}
TEST_ASSERT_EQUAL(bytes2send, tcp_stats[j].sent_bytes);
#endif
tx_sem.wait(split2half_rmng_tcp_test_time());
thread->join();
delete thread;
tx_sem.wait(split2half_rmng_tcp_test_time() * 1000); // *1000 to convert s->ms
if (receive_error) {
break;
}
}
END:
sock.sigio(NULL);
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock.close());
receiver_thread->terminate();
delete receiver_thread;
receiver_thread = NULL;
tc_exec_time.stop();
free(stack_mem);
TEST_ASSERT_EQUAL(NSAPI_ERROR_OK, sock.close());
}
4 changes: 1 addition & 3 deletions TESTS/netsocket/tcp/tcpsocket_echotest_burst.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,7 @@ void TCPSOCKET_ECHOTEST_BURST()
}

if (bt_left != 0) {
drop_bad_packets(sock, 0);
TEST_FAIL();
TEST_FAIL_MESSAGE("bt_left != 0");
goto END;
}

Expand Down Expand Up @@ -142,7 +141,6 @@ void TCPSOCKET_ECHOTEST_BURST_NONBLOCK()

if (bt_left != 0) {
printf("network error %d, missing %d bytes from a burst\n", recvd, bt_left);
drop_bad_packets(sock, -1);
TEST_FAIL();
goto END;
}
Expand Down
4 changes: 2 additions & 2 deletions TESTS/netsocket/tcp/tcpsocket_endpoint_close.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,10 +38,10 @@ static nsapi_error_t _tcpsocket_connect_to_daytime_srv(TCPSocket &sock)
{
SocketAddress tcp_addr;

get_interface()->gethostbyname(MBED_CONF_APP_ECHO_SERVER_ADDR, &tcp_addr);
NetworkInterface::get_default_instance()->gethostbyname(MBED_CONF_APP_ECHO_SERVER_ADDR, &tcp_addr);
tcp_addr.set_port(13);

nsapi_error_t err = sock.open(get_interface());
nsapi_error_t err = sock.open(NetworkInterface::get_default_instance());
if (err != NSAPI_ERROR_OK) {
return err;
}
Expand Down
Loading