Skip to content

Commit

Permalink
tests/net_inet: Add tests for accept and connect in nonblocking mode.
Browse files Browse the repository at this point in the history
Some of these tests don't require an Internet connection, but here is a
good place to put them for now.
  • Loading branch information
dpgeorge committed Jun 21, 2017
1 parent 4caa27a commit 458cbac
Show file tree
Hide file tree
Showing 6 changed files with 55 additions and 0 deletions.
16 changes: 16 additions & 0 deletions tests/net_inet/accept_nonblock.py
@@ -0,0 +1,16 @@
# test that socket.accept() on a non-blocking socket raises EAGAIN

try:
import usocket as socket
except:
import socket

s = socket.socket()
s.bind(socket.getaddrinfo('127.0.0.1', 8123)[0][-1])
s.setblocking(False)
s.listen(1)
try:
s.accept()
except OSError as er:
print(er.args[0] == 11) # 11 is EAGAIN
s.close()
1 change: 1 addition & 0 deletions tests/net_inet/accept_nonblock.py.exp
@@ -0,0 +1 @@
True
22 changes: 22 additions & 0 deletions tests/net_inet/accept_timeout.py
@@ -0,0 +1,22 @@
# test that socket.accept() on a socket with timeout raises ETIMEDOUT

try:
import usocket as socket
except:
import socket

try:
socket.socket.settimeout
except AttributeError:
print('SKIP')
raise SystemExit

s = socket.socket()
s.bind(socket.getaddrinfo('127.0.0.1', 8123)[0][-1])
s.settimeout(1)
s.listen(1)
try:
s.accept()
except OSError as er:
print(er.args[0] in (110, 'timed out')) # 110 is ETIMEDOUT; CPython uses a string
s.close()
1 change: 1 addition & 0 deletions tests/net_inet/accept_timeout.py.exp
@@ -0,0 +1 @@
True
14 changes: 14 additions & 0 deletions tests/net_inet/connect_nonblock.py
@@ -0,0 +1,14 @@
# test that socket.connect() on a non-blocking socket raises EINPROGRESS

try:
import usocket as socket
except:
import socket

s = socket.socket()
s.setblocking(False)
try:
s.connect(socket.getaddrinfo('micropython.org', 80)[0][-1])
except OSError as er:
print(er.args[0] == 115) # 115 is EINPROGRESS
s.close()
1 change: 1 addition & 0 deletions tests/net_inet/connect_nonblock.py.exp
@@ -0,0 +1 @@
True

6 comments on commit 458cbac

@pfalcon
Copy link
Contributor

Choose a reason for hiding this comment

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

@dpgeorge : Well, so, as its README says, net_inet/ was intended for tests which require connection to wide Internet, vs tests which require just limited "device-host" connection. The tests you committed are such, would be nice to move them to tests/net_local or something.

@dpgeorge
Copy link
Member Author

Choose a reason for hiding this comment

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

Yes I understand, as I wrote in the commit message above some don't require the Internet (but one does, connect_nonblock).

@pfalcon
Copy link
Contributor

Choose a reason for hiding this comment

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

So, it would be nice to follow the ideas in #3014 from the start, if at least from categorization point of view. If for a test it's enough to have a local TCP/UDP server running on a host and DUT connecting to it, it's "local" test (feel free to suggest better terminology). All your tests are such. OTOH, test_tls_sites.py requires connection to Internet, because it test connections to random sites on it, which we will never be able to reproduce locally.

Two groups of tests will have different setup and precondtions for testing, so should be separated.

@pfalcon
Copy link
Contributor

Choose a reason for hiding this comment

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

@dpgeorge : Let me know if you're agree and then if you'd like me to do that change, and via a PR or direct commit.

@dpgeorge
Copy link
Member Author

Choose a reason for hiding this comment

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

If you have time, please go for it. Tests can be further separated into ones that don't need any kind of server (eg net_dev for "device"), and ones that do, but not the full Internet (eg net_local).

But, regarding connect_nonblock.py test: it's arguably simpler to have such a test rely on the Internet rather than a local server. If that test is converted to rely on a local server then it becomes more complex to run because you need to set up the local test server, whereas right now it can just use the Internet.

@pfalcon
Copy link
Contributor

Choose a reason for hiding this comment

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

But we discussed all these approaches in #3014 , it was your argument that we won't be able to test much locally, relying on loopback etc., and would need connecting to something else, and in the end, I agreed that we'd rather have a test server (#3014 (comment)). So, let's try to stick with that plan - to categorize net tests into ones which require full inet connection and once which would work with a test server running on a host, even though few tests can work without such server, and though we don't have the mentioned test server so far (I switched to working on other ways of Zephyr testing during dayjob, so that's in backlog).

Please sign in to comment.