* [Tarantool-patches] [PATCH v3] test: fix flaky socket test
@ 2019-12-10 14:36 Ilya Kosarev
2019-12-17 0:03 ` Vladislav Shpilevoy
0 siblings, 1 reply; 3+ messages in thread
From: Ilya Kosarev @ 2019-12-10 14:36 UTC (permalink / raw)
To: tarantool-patches; +Cc: v.shpilevoy
test: fix flaky socket test
socket.test had a number of flaky problems:
- socket readiness expectation
- race conditions on socket shutdown in emulation test cases
- tcp_server stability in socket receive inconsistent behavior case
Now they are solved. Port randomization is improved.
Socket test is not fragile anymore.
Closes #4451, #4426, #4469
---
Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4426-4451-fix-socket-test
Issues: https://github.com/tarantool/tarantool/issues/4426
https://github.com/tarantool/tarantool/issues/4451
https://github.com/tarantool/tarantool/issues/4469
Changes in v2:
- reconsidered socket readiness expectation
- reduced conditions waiting time
Changes in v3:
- reconsidered expectations to unify them
- simplified randomization
test/app/socket.result | 159 +++++++++++++++++++++++++--------------
test/app/socket.test.lua | 123 +++++++++++++++++++-----------
test/app/suite.ini | 1 -
3 files changed, 178 insertions(+), 105 deletions(-)
diff --git a/test/app/socket.result b/test/app/socket.result
index fd299424c96..25ad9034374 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -42,7 +42,7 @@ test_run:cmd("push filter '(error: .builtin/.*[.]lua):[0-9]+' to '\\1'")
---
- true
...
-WAIT_COND_TIME = 10
+WAIT_COND_TIME = 100
---
...
socket('PF_INET', 'SOCK_STREAM', 'tcp121222');
@@ -107,11 +107,11 @@ s:nonblock(true)
---
- true
...
-s:readable(.1)
+s:readable(WAIT_COND_TIME)
---
- true
...
-s:wait(.1)
+s:wait(WAIT_COND_TIME)
---
- RW
...
@@ -183,7 +183,7 @@ s:writable(0)
---
- true
...
-s:wait(.01)
+s:wait(WAIT_COND_TIME)
---
- RW
...
@@ -227,11 +227,11 @@ s:syswrite(ffi.cast('const char *', ping), #ping)
---
- 6
...
-s:readable(1)
+s:readable(WAIT_COND_TIME)
---
- true
...
-s:wait(.01)
+s:wait(WAIT_COND_TIME)
---
- RW
...
@@ -278,14 +278,14 @@ s:error()
---
- null
...
-s:listen(128)
+s:listen(WAIT_COND_TIME)
---
- true
...
sevres = {}
---
...
-type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
+type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
---
- userdata
...
@@ -308,7 +308,7 @@ sc:nonblock(true)
---
- true
...
-sc:readable(.5)
+sc:readable(WAIT_COND_TIME)
---
- true
...
@@ -440,7 +440,7 @@ s:bind('127.0.0.1', 0)
---
- true
...
-s:listen(128)
+s:listen(WAIT_COND_TIME)
---
- true
...
@@ -451,7 +451,7 @@ sc:sysconnect('127.0.0.1', s:name().port) or errno() == errno.EINPROGRESS
---
- true
...
-sc:writable(10)
+sc:writable(WAIT_COND_TIME)
---
- true
...
@@ -485,7 +485,7 @@ sa:read(3)
---
- orl
...
-sc:writable()
+sc:writable(WAIT_COND_TIME)
---
- true
...
@@ -513,7 +513,7 @@ sa:read(1, .01)
---
- null
...
-sc:writable()
+sc:writable(WAIT_COND_TIME)
---
- true
...
@@ -546,7 +546,7 @@ sc:send('Hello')
---
- 5
...
-sa:readable()
+sa:readable(WAIT_COND_TIME)
---
- true
...
@@ -646,7 +646,7 @@ sc ~= nil
---
- true
...
-s:listen(1234)
+s:listen(WAIT_COND_TIME)
---
- true
...
@@ -665,7 +665,7 @@ sc:error()
---
- null
...
-s:readable()
+s:readable(WAIT_COND_TIME)
---
- true
...
@@ -754,7 +754,7 @@ string.match(tostring(sc), ', peer') == nil
---
- true
...
-sc:writable()
+sc:writable(WAIT_COND_TIME)
---
- true
...
@@ -830,7 +830,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, world')
---
- 12
...
-s:readable(10)
+s:readable(WAIT_COND_TIME)
---
- true
...
@@ -842,7 +842,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2')
---
- 15
...
-s:readable(10)
+s:readable(WAIT_COND_TIME)
---
- true
...
@@ -898,7 +898,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, World!')
---
- 13
...
-s:readable(1)
+s:readable(WAIT_COND_TIME)
---
- true
...
@@ -913,7 +913,7 @@ s:sendto(from.host, from.port, 'Hello, hello!')
---
- 13
...
-sc:readable(1)
+sc:readable(WAIT_COND_TIME)
---
- true
...
@@ -957,7 +957,7 @@ s:bind('127.0.0.1', 0)
port = s:name().port
---
...
-s:listen()
+s:listen(WAIT_COND_TIME)
---
- true
...
@@ -1004,7 +1004,7 @@ socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
- null
- true
...
-s:listen()
+s:listen(WAIT_COND_TIME)
---
- true
...
@@ -1074,6 +1074,9 @@ master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
---
- true
...
+math.randomseed(fiber.time())
+---
+...
port = 32768 + math.random(0, 32767)
---
...
@@ -1086,7 +1089,7 @@ test_run:cmd("setopt delimiter ';'")
...
test_run:wait_cond(function()
local ok = master:bind('127.0.0.1', port)
- local ok = ok and master:listen()
+ local ok = ok and master:listen(WAIT_COND_TIME)
if not ok then
port = 32768 + math.random(32768)
return false, master:error()
@@ -1099,7 +1102,7 @@ end, WAIT_COND_TIME);
function gh361()
local s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
s:sysconnect('127.0.0.1', port)
- s:wait()
+ s:wait(WAIT_COND_TIME)
res = s:read(1200)
end;
---
@@ -1146,7 +1149,7 @@ s:error()
---
- null
...
-s:listen(128)
+s:listen(WAIT_COND_TIME)
---
- true
...
@@ -1156,7 +1159,7 @@ test_run:cmd("setopt delimiter ';'")
...
f = fiber.create(function()
for i=1,2 do
- s:readable()
+ s:readable(WAIT_COND_TIME)
local sc = s:accept()
sc:write('ok!')
sc:shutdown()
@@ -1532,7 +1535,7 @@ s:bind('unix/', path)
---
- true
...
-s:listen()
+s:listen(WAIT_COND_TIME)
---
- true
...
@@ -1759,7 +1762,7 @@ s:bind('127.0.0.1', 0) -- error handling
- null
- Invalid argument
...
-s:listen(10)
+s:listen(WAIT_COND_TIME)
---
- 1
...
@@ -1822,8 +1825,14 @@ test_run:cmd("setopt delimiter ';'")
---
- true
...
+socket_opened = true
cfiber = fiber.create(function(sc, rch, wch)
- while sc:send(wch:get()) and rch:put(sc:receive("*l")) do end
+ while socket_opened do
+ sc:send(wch:get())
+ local data = sc:receive("*l")
+ if not socket_opened then sc:close() end
+ rch:put(data)
+ end
end, sc, rch, wch);
---
...
@@ -1936,6 +1945,9 @@ c:receive("*l")
---
-
...
+socket_opened = false
+---
+...
wch:put("Fu")
---
- true
@@ -1944,10 +1956,6 @@ c:send("354 Please type your message\n")
---
- 29
...
-sc:close()
----
-- 1
-...
c:receive("*l", "Line: ")
---
- null
@@ -2231,7 +2239,17 @@ test_run:cmd("setopt delimiter ''");
receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
---
...
-receiving_socket:bind('127.0.0.1', 0)
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+test_run:wait_cond(function()
+ return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
+end, WAIT_COND_TIME);
+---
+- true
+...
+test_run:cmd("setopt delimiter ''");
---
- true
...
@@ -2625,11 +2643,21 @@ listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
---
- true
...
-listening_socket:bind('127.0.0.1', 0)
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+test_run:wait_cond(function()
+ return listening_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
+end, WAIT_COND_TIME);
---
- true
...
-listening_socket:listen()
+test_run:cmd("setopt delimiter ''");
+---
+- true
+...
+listening_socket:listen(WAIT_COND_TIME)
---
- true
...
@@ -2643,8 +2671,18 @@ sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errn
---
- true
...
-receiving_socket = listening_socket:accept()
+test_run:cmd("setopt delimiter ';'")
+---
+- true
+...
+receiving_socket = test_run:wait_cond(function()
+ return listening_socket:accept()
+end, WAIT_COND_TIME);
+---
+...
+test_run:cmd("setopt delimiter ''");
---
+- true
...
sending_socket:write(message)
---
@@ -2816,7 +2854,7 @@ test_run:cmd("clear filter")
---
- true
...
--- case: sicket receive inconsistent behavior
+-- case: socket receive inconsistent behavior
chan = fiber.channel()
---
...
@@ -2826,41 +2864,46 @@ counter = 0
fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end
---
...
-srv = socket.tcp_server('0.0.0.0', 8888, fn)
+srv = nil
---
...
-s = socket.connect('localhost', 8888)
+test_run:cmd("setopt delimiter ';'")
---
+- true
...
-chan:put(5)
+test_run:wait_cond(function()
+ port = 32768 + math.random(0, 32767)
+ srv = socket.tcp_server('0.0.0.0', port, fn)
+ return srv ~= nil
+end, WAIT_COND_TIME);
---
- true
...
-chan:put(5)
+receive1 = nil; receive2 = nil;
---
-- true
...
-s:receive(5)
+if srv ~= nil then
+ s = socket.connect('localhost', port)
+ chan:put(5)
+ chan:put(5)
+ receive1 = s:receive(5)
+ chan:put(5)
+ s:settimeout(1)
+ receive2 = s:receive('*a')
+ s:close()
+ srv:close()
+end;
---
-- '00000'
...
-chan:put(5)
+test_run:cmd("setopt delimiter ''");
---
- true
...
-s:settimeout(1)
+receive1
---
-- 1
+- '00000'
...
-s:receive('*a')
+receive2
---
- '1111122222'
...
-s:close()
----
-- 1
-...
-srv:close()
----
-- true
-...
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index c72d41763f4..97dac794e37 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -13,7 +13,7 @@ env = require('test_run')
test_run = env.new()
test_run:cmd("push filter '(error: .builtin/.*[.]lua):[0-9]+' to '\\1'")
-WAIT_COND_TIME = 10
+WAIT_COND_TIME = 100
socket('PF_INET', 'SOCK_STREAM', 'tcp121222');
@@ -39,8 +39,8 @@ s:nonblock(false)
s:nonblock()
s:nonblock(true)
-s:readable(.1)
-s:wait(.1)
+s:readable(WAIT_COND_TIME)
+s:wait(WAIT_COND_TIME)
socket.iowait(s:fd(), 'RW')
socket.iowait(s:fd(), 3)
socket.iowait(s:fd(), 'R')
@@ -58,7 +58,7 @@ s:errno() > 0
s:error()
s:writable(.00000000000001)
s:writable(0)
-s:wait(.01)
+s:wait(WAIT_COND_TIME)
socket.iowait(nil, nil, -1)
socket.iowait(nil, nil, 0.0001)
socket.iowait(-1, nil, 0.0001)
@@ -75,8 +75,8 @@ ping = msgpack.encode(string.len(ping)) .. ping
-- test syswrite with char *
s:syswrite(ffi.cast('const char *', ping), #ping)
-s:readable(1)
-s:wait(.01)
+s:readable(WAIT_COND_TIME)
+s:wait(WAIT_COND_TIME)
pong = s:sysread()
string.len(pong)
@@ -91,16 +91,16 @@ s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
s:error()
s:bind('127.0.0.1', 0)
s:error()
-s:listen(128)
+s:listen(WAIT_COND_TIME)
sevres = {}
-type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
+type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
#sevres
sc = socket('PF_INET', 'SOCK_STREAM', 'tcp')
sc:nonblock(false)
sc:sysconnect('127.0.0.1', s:name().port)
sc:nonblock(true)
-sc:readable(.5)
+sc:readable(WAIT_COND_TIME)
sc:sysread()
string.match(tostring(sc), ', peer') ~= nil
#sevres
@@ -135,12 +135,12 @@ s:close()
s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
s:bind('127.0.0.1', 0)
-s:listen(128)
+s:listen(WAIT_COND_TIME)
sc = socket('PF_INET', 'SOCK_STREAM', 'tcp')
sc:sysconnect('127.0.0.1', s:name().port) or errno() == errno.EINPROGRESS
-sc:writable(10)
+sc:writable(WAIT_COND_TIME)
sc:write('Hello, world')
sa, addr = s:accept()
@@ -150,14 +150,14 @@ addr2.family == addr.family
sa:nonblock(1)
sa:read(8)
sa:read(3)
-sc:writable()
+sc:writable(WAIT_COND_TIME)
sc:write(', again')
sa:read(8)
sa:error()
string.len(sa:read(0))
type(sa:read(0))
sa:read(1, .01)
-sc:writable()
+sc:writable(WAIT_COND_TIME)
-- gh-3979 Check for errors when argument is negative.
@@ -170,7 +170,7 @@ sc:send('abc')
sa:read(3)
sc:send('Hello')
-sa:readable()
+sa:readable(WAIT_COND_TIME)
sa:recv()
sa:recv()
@@ -198,14 +198,14 @@ path = 'tarantool-test-socket'
os.remove(path)
s:bind('unix/', path)
sc ~= nil
-s:listen(1234)
+s:listen(WAIT_COND_TIME)
sc = socket('PF_UNIX', 'SOCK_STREAM', 0)
sc:nonblock(true)
sc:sysconnect('unix/', path)
sc:error()
-s:readable()
+s:readable(WAIT_COND_TIME)
sa = s:accept()
sa:nonblock(true)
sa:send('Hello, world')
@@ -243,7 +243,7 @@ sc:getsockopt('SOL_SOCKET', 'SO_ERROR')
sc:nonblock(true)
sc:sysconnect('127.0.0.1', 3458) or errno() == errno.EINPROGRESS or errno() == errno.ECONNREFUSED
string.match(tostring(sc), ', peer') == nil
-sc:writable()
+sc:writable(WAIT_COND_TIME)
string.match(tostring(sc), ', peer') == nil
socket_error = sc:getsockopt('SOL_SOCKET', 'SO_ERROR')
socket_error == errno.ECONNREFUSED or socket_error == 0
@@ -266,11 +266,11 @@ s = socket('AF_INET', 'SOCK_DGRAM', 'udp')
s:bind('127.0.0.1', 0)
sc = socket('AF_INET', 'SOCK_DGRAM', 'udp')
sc:sendto('127.0.0.1', s:name().port, 'Hello, world')
-s:readable(10)
+s:readable(WAIT_COND_TIME)
s:recv()
sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2')
-s:readable(10)
+s:readable(WAIT_COND_TIME)
d, from = s:recvfrom()
from.port > 0
from.port = 'Random port'
@@ -286,11 +286,11 @@ sc = socket('AF_INET', 'SOCK_DGRAM', 'udp')
sc:nonblock(true)
sc:sendto('127.0.0.1', s:name().port)
sc:sendto('127.0.0.1', s:name().port, 'Hello, World!')
-s:readable(1)
+s:readable(WAIT_COND_TIME)
data, from = s:recvfrom(10)
data
s:sendto(from.host, from.port, 'Hello, hello!')
-sc:readable(1)
+sc:readable(WAIT_COND_TIME)
data_r, from_r = sc:recvfrom()
data_r
from_r.host
@@ -307,7 +307,7 @@ socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
s:bind('127.0.0.1', 0)
port = s:name().port
-s:listen()
+s:listen(WAIT_COND_TIME)
sc, e = socket.tcp_connect('127.0.0.1', port), errno()
sc ~= nil
e == 0
@@ -321,7 +321,7 @@ _ = os.remove(path)
s = socket('AF_UNIX', 'SOCK_STREAM', 0)
s:bind('unix/', path)
socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
-s:listen()
+s:listen(WAIT_COND_TIME)
sc, e = socket.tcp_connect('unix/', path), errno()
sc ~= nil
e
@@ -343,6 +343,7 @@ s = nil
-- random port
master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
+math.randomseed(fiber.time())
port = 32768 + math.random(0, 32767)
-- SO_REUSEADDR allows to bind to the same source addr:port twice,
-- so listen() can return EADDRINUSE and so we check it within
@@ -350,7 +351,7 @@ port = 32768 + math.random(0, 32767)
test_run:cmd("setopt delimiter ';'")
test_run:wait_cond(function()
local ok = master:bind('127.0.0.1', port)
- local ok = ok and master:listen()
+ local ok = ok and master:listen(WAIT_COND_TIME)
if not ok then
port = 32768 + math.random(32768)
return false, master:error()
@@ -360,7 +361,7 @@ end, WAIT_COND_TIME);
function gh361()
local s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
s:sysconnect('127.0.0.1', port)
- s:wait()
+ s:wait(WAIT_COND_TIME)
res = s:read(1200)
end;
test_run:cmd("setopt delimiter ''");
@@ -377,11 +378,11 @@ s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
s:error()
s:bind('unix/', path)
s:error()
-s:listen(128)
+s:listen(WAIT_COND_TIME)
test_run:cmd("setopt delimiter ';'")
f = fiber.create(function()
for i=1,2 do
- s:readable()
+ s:readable(WAIT_COND_TIME)
local sc = s:accept()
sc:write('ok!')
sc:shutdown()
@@ -506,7 +507,7 @@ _ = os.remove(path)
-- Test that socket is closed on GC
s = socket('AF_UNIX', 'SOCK_STREAM', 0)
s:bind('unix/', path)
-s:listen()
+s:listen(WAIT_COND_TIME)
s = nil
while socket.tcp_connect('unix/', path) do collectgarbage('collect') end
_ = os.remove(path)
@@ -602,7 +603,7 @@ s:setoption('tcp-nodelay', true)
s:setoption('unknown', true)
s:bind('127.0.0.1', 0)
s:bind('127.0.0.1', 0) -- error handling
-s:listen(10)
+s:listen(WAIT_COND_TIME)
s -- transformed to tcp{server} socket
host, port, family = s:getsockname()
host == '127.0.0.1', type(port) == 'string', family == 'inet'
@@ -619,8 +620,14 @@ s:settimeout(100500)
rch, wch = fiber.channel(1), fiber.channel(1)
sc = socket.connect(host, port)
test_run:cmd("setopt delimiter ';'")
+socket_opened = true
cfiber = fiber.create(function(sc, rch, wch)
- while sc:send(wch:get()) and rch:put(sc:receive("*l")) do end
+ while socket_opened do
+ sc:send(wch:get())
+ local data = sc:receive("*l")
+ if not socket_opened then sc:close() end
+ rch:put(data)
+ end
end, sc, rch, wch);
test_run:cmd("setopt delimiter ''");
@@ -651,9 +658,9 @@ rch:get()
wch:put("DATA\n")
c:receive(4)
c:receive("*l")
+socket_opened = false
wch:put("Fu")
c:send("354 Please type your message\n")
-sc:close()
c:receive("*l", "Line: ")
c:receive()
c:receive(10)
@@ -756,7 +763,11 @@ end;
test_run:cmd("setopt delimiter ''");
receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
-receiving_socket:bind('127.0.0.1', 0)
+test_run:cmd("setopt delimiter ';'")
+test_run:wait_cond(function()
+ return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
+end, WAIT_COND_TIME);
+test_run:cmd("setopt delimiter ''");
receiving_socket_port = receiving_socket:name().port
sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
@@ -891,12 +902,20 @@ message = string.rep('x', message_len)
listening_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp')
listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
-listening_socket:bind('127.0.0.1', 0)
-listening_socket:listen()
+test_run:cmd("setopt delimiter ';'")
+test_run:wait_cond(function()
+ return listening_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
+end, WAIT_COND_TIME);
+test_run:cmd("setopt delimiter ''");
+listening_socket:listen(WAIT_COND_TIME)
listening_socket_port = listening_socket:name().port
sending_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp')
sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errno.EINPROGRESS
-receiving_socket = listening_socket:accept()
+test_run:cmd("setopt delimiter ';'")
+receiving_socket = test_run:wait_cond(function()
+ return listening_socket:accept()
+end, WAIT_COND_TIME);
+test_run:cmd("setopt delimiter ''");
sending_socket:write(message)
-- case: recvfrom reads first 512 bytes from the message with tcp
@@ -960,18 +979,30 @@ server:close()
test_run:cmd("clear filter")
--- case: sicket receive inconsistent behavior
+-- case: socket receive inconsistent behavior
chan = fiber.channel()
counter = 0
fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end
-srv = socket.tcp_server('0.0.0.0', 8888, fn)
-s = socket.connect('localhost', 8888)
-chan:put(5)
-chan:put(5)
-s:receive(5)
-chan:put(5)
-s:settimeout(1)
-s:receive('*a')
-s:close()
-srv:close()
+srv = nil
+test_run:cmd("setopt delimiter ';'")
+test_run:wait_cond(function()
+ port = 32768 + math.random(0, 32767)
+ srv = socket.tcp_server('0.0.0.0', port, fn)
+ return srv ~= nil
+end, WAIT_COND_TIME);
+receive1 = nil; receive2 = nil;
+if srv ~= nil then
+ s = socket.connect('localhost', port)
+ chan:put(5)
+ chan:put(5)
+ receive1 = s:receive(5)
+ chan:put(5)
+ s:settimeout(1)
+ receive2 = s:receive('*a')
+ s:close()
+ srv:close()
+end;
+test_run:cmd("setopt delimiter ''");
+receive1
+receive2
diff --git a/test/app/suite.ini b/test/app/suite.ini
index 79432e29a76..dd802d98cf9 100644
--- a/test/app/suite.ini
+++ b/test/app/suite.ini
@@ -7,4 +7,3 @@ use_unix_sockets = True
use_unix_sockets_iproto = True
is_parallel = True
pretest_clean = True
-fragile = socket.test.lua ; gh-4426 gh-4451
--
2.17.1
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Tarantool-patches] [PATCH v3] test: fix flaky socket test
2019-12-10 14:36 [Tarantool-patches] [PATCH v3] test: fix flaky socket test Ilya Kosarev
@ 2019-12-17 0:03 ` Vladislav Shpilevoy
2019-12-23 20:48 ` Ilya Kosarev
0 siblings, 1 reply; 3+ messages in thread
From: Vladislav Shpilevoy @ 2019-12-17 0:03 UTC (permalink / raw)
To: Ilya Kosarev, tarantool-patches
Thanks for the patch!
See 6 comments below, and 2 review fix commits in the
end.
On 10/12/2019 15:36, Ilya Kosarev wrote:
> test: fix flaky socket test
1. You've duplicated the commit title in the
commit message.
>
> socket.test had a number of flaky problems:
> - socket readiness expectation
> - race conditions on socket shutdown in emulation test cases
> - tcp_server stability in socket receive inconsistent behavior case
> Now they are solved. Port randomization is improved.
> Socket test is not fragile anymore.
2. It depends on definition of 'fragile test'. Any test using UDP
can fail under sufficiently high load. And this test is not an
exception. After your and my fixes it fails extremely rare, but
still fails. That happens under huge load when the kernel starts
dropping UDP packets. Not delivers them with a delay, but drops.
But I guess yeah, we can remove the 'fragile' flag. Unless our
test suite will ever try to run 64 socket tests in parallel a
couple of times.
>
> Closes #4451, #4426, #4469
> ---
> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4426-4451-fix-socket-test
> Issues: https://github.com/tarantool/tarantool/issues/4426
> https://github.com/tarantool/tarantool/issues/4451
> https://github.com/tarantool/tarantool/issues/4469
>
> Changes in v2:
> - reconsidered socket readiness expectation
> - reduced conditions waiting time
>
> Changes in v3:
> - reconsidered expectations to unify them
> - simplified randomization
>
> test/app/socket.result | 159 +++++++++++++++++++++++++--------------
> test/app/socket.test.lua | 123 +++++++++++++++++++-----------
> test/app/suite.ini | 1 -
> 3 files changed, 178 insertions(+), 105 deletions(-)
>
> diff --git a/test/app/socket.result b/test/app/socket.result
> index fd299424c96..25ad9034374 100644
> --- a/test/app/socket.result
> +++ b/test/app/socket.result
> @@ -278,14 +278,14 @@ s:error()
> ---
> - null
> ...
> -s:listen(128)
> +s:listen(WAIT_COND_TIME)
3. I suggest you to read socket.lua source code. Listen takes
backlog size, not a timeout. The same comment to many other
places.
> ---
> - true
> ...
> sevres = {}
> ---
> ...
> -type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
> +type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
4. :readable() has infinity timeout. When you made it WAIT_COND_TIME,
you certainly didn't improve the test stability. The same in other
similar places.
> ---
> - userdata
> ...
> @@ -1004,7 +1004,7 @@ socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
> - null
> - true
> ...
> -s:listen()
> +s:listen(WAIT_COND_TIME)
> ---
> - true
> ...
> @@ -1074,6 +1074,9 @@ master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
> ---
> - true
> ...
> +math.randomseed(fiber.time())
5. This line becomes useless when you've increased the timeout.
Moreover, the random port search below also does not make any
sense. You could use port 0 to bind to any free port instead of
manual guessing for the kernel.
> +---
> +...
> port = 32768 + math.random(0, 32767)
> ---
> ...
> @@ -2231,7 +2239,17 @@ test_run:cmd("setopt delimiter ''");
> receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
> ---
> ...
> -receiving_socket:bind('127.0.0.1', 0)
> +test_run:cmd("setopt delimiter ';'")
> +---
> +- true
> +...
> +test_run:wait_cond(function()
> + return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
> +end, WAIT_COND_TIME);
6. Why? Bind with 0 port finds a suitable port. You don't
need to do random guessing. The same in many other places.
> +---
> +- true
> +...
> +test_run:cmd("setopt delimiter ''");
> ---
> - true
> ...
Below are 2 my commits. First with review fixes to the patch. Second
with stabilization of UDP tests (well, not 100% stability, but much
better. 100% would require to rework the tests too much loosing their
purpose).
Please, review them, squahs if you agree, and send to a second review
to someone. For example to Alexander. But taking into account urgency
of 2.3.1 maybe we can't wait several weeks more, so ask Kirill.
===========================================================================
commit fc11ffbd9135554b2b91728483189ababa86c36e
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date: Mon Dec 16 23:33:21 2019 +0100
Review fixes
diff --git a/test/app/socket.result b/test/app/socket.result
index 25ad90343..563519315 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -278,14 +278,14 @@ s:error()
---
- null
...
-s:listen(WAIT_COND_TIME)
+s:listen(128)
---
- true
...
sevres = {}
---
...
-type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
+type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
---
- userdata
...
@@ -440,7 +440,7 @@ s:bind('127.0.0.1', 0)
---
- true
...
-s:listen(WAIT_COND_TIME)
+s:listen(128)
---
- true
...
@@ -485,7 +485,7 @@ sa:read(3)
---
- orl
...
-sc:writable(WAIT_COND_TIME)
+sc:writable()
---
- true
...
@@ -513,7 +513,7 @@ sa:read(1, .01)
---
- null
...
-sc:writable(WAIT_COND_TIME)
+sc:writable()
---
- true
...
@@ -546,7 +546,7 @@ sc:send('Hello')
---
- 5
...
-sa:readable(WAIT_COND_TIME)
+sa:readable()
---
- true
...
@@ -646,7 +646,7 @@ sc ~= nil
---
- true
...
-s:listen(WAIT_COND_TIME)
+s:listen(1234)
---
- true
...
@@ -665,7 +665,7 @@ sc:error()
---
- null
...
-s:readable(WAIT_COND_TIME)
+s:readable()
---
- true
...
@@ -754,7 +754,7 @@ string.match(tostring(sc), ', peer') == nil
---
- true
...
-sc:writable(WAIT_COND_TIME)
+sc:writable()
---
- true
...
@@ -957,7 +957,7 @@ s:bind('127.0.0.1', 0)
port = s:name().port
---
...
-s:listen(WAIT_COND_TIME)
+s:listen()
---
- true
...
@@ -1004,7 +1004,7 @@ socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
- null
- true
...
-s:listen(WAIT_COND_TIME)
+s:listen()
---
- true
...
@@ -1070,39 +1070,21 @@ s = nil
master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
---
...
-master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
+master:bind('127.0.0.1', 0)
---
- true
...
-math.randomseed(fiber.time())
----
-...
-port = 32768 + math.random(0, 32767)
+port = master:name().port
---
...
--- SO_REUSEADDR allows to bind to the same source addr:port twice,
--- so listen() can return EADDRINUSE and so we check it within
--- wait_cond().
test_run:cmd("setopt delimiter ';'")
---
- true
...
-test_run:wait_cond(function()
- local ok = master:bind('127.0.0.1', port)
- local ok = ok and master:listen(WAIT_COND_TIME)
- if not ok then
- port = 32768 + math.random(32768)
- return false, master:error()
- end
- return true
-end, WAIT_COND_TIME);
----
-- true
-...
function gh361()
local s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
s:sysconnect('127.0.0.1', port)
- s:wait(WAIT_COND_TIME)
+ s:wait()
res = s:read(1200)
end;
---
@@ -1149,7 +1131,7 @@ s:error()
---
- null
...
-s:listen(WAIT_COND_TIME)
+s:listen(128)
---
- true
...
@@ -1159,7 +1141,7 @@ test_run:cmd("setopt delimiter ';'")
...
f = fiber.create(function()
for i=1,2 do
- s:readable(WAIT_COND_TIME)
+ s:readable()
local sc = s:accept()
sc:write('ok!')
sc:shutdown()
@@ -1535,7 +1517,7 @@ s:bind('unix/', path)
---
- true
...
-s:listen(WAIT_COND_TIME)
+s:listen()
---
- true
...
@@ -1762,7 +1744,7 @@ s:bind('127.0.0.1', 0) -- error handling
- null
- Invalid argument
...
-s:listen(WAIT_COND_TIME)
+s:listen(10)
---
- 1
...
@@ -2239,17 +2221,7 @@ test_run:cmd("setopt delimiter ''");
receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
---
...
-test_run:cmd("setopt delimiter ';'")
----
-- true
-...
-test_run:wait_cond(function()
- return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
-end, WAIT_COND_TIME);
----
-- true
-...
-test_run:cmd("setopt delimiter ''");
+receiving_socket:bind('127.0.0.1', 0)
---
- true
...
@@ -2643,21 +2615,11 @@ listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
---
- true
...
-test_run:cmd("setopt delimiter ';'")
----
-- true
-...
-test_run:wait_cond(function()
- return listening_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
-end, WAIT_COND_TIME);
----
-- true
-...
-test_run:cmd("setopt delimiter ''");
+listening_socket:bind('127.0.0.1', 0)
---
- true
...
-listening_socket:listen(WAIT_COND_TIME)
+listening_socket:listen()
---
- true
...
@@ -2671,18 +2633,12 @@ sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errn
---
- true
...
-test_run:cmd("setopt delimiter ';'")
+listening_socket:readable(WAIT_COND_TIME)
---
- true
...
-receiving_socket = test_run:wait_cond(function()
- return listening_socket:accept()
-end, WAIT_COND_TIME);
----
-...
-test_run:cmd("setopt delimiter ''");
+receiving_socket = listening_socket:accept()
---
-- true
...
sending_socket:write(message)
---
@@ -2864,46 +2820,41 @@ counter = 0
fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end
---
...
-srv = nil
+srv = socket.tcp_server('0.0.0.0', 0, fn)
---
...
-test_run:cmd("setopt delimiter ';'")
+s = socket.connect('localhost', srv:name().port)
---
-- true
...
-test_run:wait_cond(function()
- port = 32768 + math.random(0, 32767)
- srv = socket.tcp_server('0.0.0.0', port, fn)
- return srv ~= nil
-end, WAIT_COND_TIME);
+chan:put(5)
---
- true
...
-receive1 = nil; receive2 = nil;
+chan:put(5)
---
+- true
...
-if srv ~= nil then
- s = socket.connect('localhost', port)
- chan:put(5)
- chan:put(5)
- receive1 = s:receive(5)
- chan:put(5)
- s:settimeout(1)
- receive2 = s:receive('*a')
- s:close()
- srv:close()
-end;
+s:receive(5)
---
+- '00000'
...
-test_run:cmd("setopt delimiter ''");
+chan:put(5)
---
- true
...
-receive1
+s:settimeout(1)
---
-- '00000'
+- 1
...
-receive2
+s:receive('*a')
---
- '1111122222'
...
+s:close()
+---
+- 1
+...
+srv:close()
+---
+- true
+...
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index 97dac794e..9ba6b2893 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -91,9 +91,9 @@ s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
s:error()
s:bind('127.0.0.1', 0)
s:error()
-s:listen(WAIT_COND_TIME)
+s:listen(128)
sevres = {}
-type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
+type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
#sevres
sc = socket('PF_INET', 'SOCK_STREAM', 'tcp')
@@ -135,7 +135,7 @@ s:close()
s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
s:bind('127.0.0.1', 0)
-s:listen(WAIT_COND_TIME)
+s:listen(128)
sc = socket('PF_INET', 'SOCK_STREAM', 'tcp')
@@ -150,14 +150,14 @@ addr2.family == addr.family
sa:nonblock(1)
sa:read(8)
sa:read(3)
-sc:writable(WAIT_COND_TIME)
+sc:writable()
sc:write(', again')
sa:read(8)
sa:error()
string.len(sa:read(0))
type(sa:read(0))
sa:read(1, .01)
-sc:writable(WAIT_COND_TIME)
+sc:writable()
-- gh-3979 Check for errors when argument is negative.
@@ -170,7 +170,7 @@ sc:send('abc')
sa:read(3)
sc:send('Hello')
-sa:readable(WAIT_COND_TIME)
+sa:readable()
sa:recv()
sa:recv()
@@ -198,14 +198,14 @@ path = 'tarantool-test-socket'
os.remove(path)
s:bind('unix/', path)
sc ~= nil
-s:listen(WAIT_COND_TIME)
+s:listen(1234)
sc = socket('PF_UNIX', 'SOCK_STREAM', 0)
sc:nonblock(true)
sc:sysconnect('unix/', path)
sc:error()
-s:readable(WAIT_COND_TIME)
+s:readable()
sa = s:accept()
sa:nonblock(true)
sa:send('Hello, world')
@@ -243,7 +243,7 @@ sc:getsockopt('SOL_SOCKET', 'SO_ERROR')
sc:nonblock(true)
sc:sysconnect('127.0.0.1', 3458) or errno() == errno.EINPROGRESS or errno() == errno.ECONNREFUSED
string.match(tostring(sc), ', peer') == nil
-sc:writable(WAIT_COND_TIME)
+sc:writable()
string.match(tostring(sc), ', peer') == nil
socket_error = sc:getsockopt('SOL_SOCKET', 'SO_ERROR')
socket_error == errno.ECONNREFUSED or socket_error == 0
@@ -307,7 +307,7 @@ socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
s:bind('127.0.0.1', 0)
port = s:name().port
-s:listen(WAIT_COND_TIME)
+s:listen()
sc, e = socket.tcp_connect('127.0.0.1', port), errno()
sc ~= nil
e == 0
@@ -321,7 +321,7 @@ _ = os.remove(path)
s = socket('AF_UNIX', 'SOCK_STREAM', 0)
s:bind('unix/', path)
socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
-s:listen(WAIT_COND_TIME)
+s:listen()
sc, e = socket.tcp_connect('unix/', path), errno()
sc ~= nil
e
@@ -342,26 +342,13 @@ s = nil
-- random port
master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
-master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
-math.randomseed(fiber.time())
-port = 32768 + math.random(0, 32767)
--- SO_REUSEADDR allows to bind to the same source addr:port twice,
--- so listen() can return EADDRINUSE and so we check it within
--- wait_cond().
+master:bind('127.0.0.1', 0)
+port = master:name().port
test_run:cmd("setopt delimiter ';'")
-test_run:wait_cond(function()
- local ok = master:bind('127.0.0.1', port)
- local ok = ok and master:listen(WAIT_COND_TIME)
- if not ok then
- port = 32768 + math.random(32768)
- return false, master:error()
- end
- return true
-end, WAIT_COND_TIME);
function gh361()
local s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
s:sysconnect('127.0.0.1', port)
- s:wait(WAIT_COND_TIME)
+ s:wait()
res = s:read(1200)
end;
test_run:cmd("setopt delimiter ''");
@@ -378,11 +365,11 @@ s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
s:error()
s:bind('unix/', path)
s:error()
-s:listen(WAIT_COND_TIME)
+s:listen(128)
test_run:cmd("setopt delimiter ';'")
f = fiber.create(function()
for i=1,2 do
- s:readable(WAIT_COND_TIME)
+ s:readable()
local sc = s:accept()
sc:write('ok!')
sc:shutdown()
@@ -507,7 +494,7 @@ _ = os.remove(path)
-- Test that socket is closed on GC
s = socket('AF_UNIX', 'SOCK_STREAM', 0)
s:bind('unix/', path)
-s:listen(WAIT_COND_TIME)
+s:listen()
s = nil
while socket.tcp_connect('unix/', path) do collectgarbage('collect') end
_ = os.remove(path)
@@ -603,7 +590,7 @@ s:setoption('tcp-nodelay', true)
s:setoption('unknown', true)
s:bind('127.0.0.1', 0)
s:bind('127.0.0.1', 0) -- error handling
-s:listen(WAIT_COND_TIME)
+s:listen(10)
s -- transformed to tcp{server} socket
host, port, family = s:getsockname()
host == '127.0.0.1', type(port) == 'string', family == 'inet'
@@ -763,11 +750,7 @@ end;
test_run:cmd("setopt delimiter ''");
receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
-test_run:cmd("setopt delimiter ';'")
-test_run:wait_cond(function()
- return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
-end, WAIT_COND_TIME);
-test_run:cmd("setopt delimiter ''");
+receiving_socket:bind('127.0.0.1', 0)
receiving_socket_port = receiving_socket:name().port
sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
@@ -902,20 +885,13 @@ message = string.rep('x', message_len)
listening_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp')
listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
-test_run:cmd("setopt delimiter ';'")
-test_run:wait_cond(function()
- return listening_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
-end, WAIT_COND_TIME);
-test_run:cmd("setopt delimiter ''");
-listening_socket:listen(WAIT_COND_TIME)
+listening_socket:bind('127.0.0.1', 0)
+listening_socket:listen()
listening_socket_port = listening_socket:name().port
sending_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp')
sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errno.EINPROGRESS
-test_run:cmd("setopt delimiter ';'")
-receiving_socket = test_run:wait_cond(function()
- return listening_socket:accept()
-end, WAIT_COND_TIME);
-test_run:cmd("setopt delimiter ''");
+listening_socket:readable(WAIT_COND_TIME)
+receiving_socket = listening_socket:accept()
sending_socket:write(message)
-- case: recvfrom reads first 512 bytes from the message with tcp
@@ -983,26 +959,14 @@ test_run:cmd("clear filter")
chan = fiber.channel()
counter = 0
fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end
-srv = nil
-test_run:cmd("setopt delimiter ';'")
-test_run:wait_cond(function()
- port = 32768 + math.random(0, 32767)
- srv = socket.tcp_server('0.0.0.0', port, fn)
- return srv ~= nil
-end, WAIT_COND_TIME);
-receive1 = nil; receive2 = nil;
-if srv ~= nil then
- s = socket.connect('localhost', port)
- chan:put(5)
- chan:put(5)
- receive1 = s:receive(5)
- chan:put(5)
- s:settimeout(1)
- receive2 = s:receive('*a')
- s:close()
- srv:close()
-end;
-test_run:cmd("setopt delimiter ''");
-receive1
-receive2
+srv = socket.tcp_server('0.0.0.0', 0, fn)
+s = socket.connect('localhost', srv:name().port)
+chan:put(5)
+chan:put(5)
+s:receive(5)
+chan:put(5)
+s:settimeout(1)
+s:receive('*a')
+s:close()
+srv:close()
===========================================================================
commit 5a5cd01a0c665f2438e05feb46dde964828f6c01
Author: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Date: Tue Dec 17 00:20:01 2019 +0100
Make UDP more table on Mac
diff --git a/test/app/socket.result b/test/app/socket.result
index 563519315..cb3897ee1 100644
--- a/test/app/socket.result
+++ b/test/app/socket.result
@@ -2282,7 +2282,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
---
- 0
...
-received_message = receiving_socket:recv()
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv() \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2305,7 +2307,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
---
- 0
...
-received_message, from = receiving_socket:recvfrom()
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom() \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2382,7 +2386,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
---
- 0
...
-received_message = receiving_socket:recv(512)
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv(512) \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2405,7 +2411,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
---
- 0
...
-received_message, from = receiving_socket:recvfrom(512)
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom(512) \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2442,7 +2450,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
---
- 1025
...
-received_message = receiving_socket:recv()
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv() \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2474,7 +2484,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
---
- 1025
...
-received_message, from = receiving_socket:recvfrom()
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom() \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2513,7 +2525,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
---
- 1025
...
-received_message = receiving_socket:recv(512)
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv(512) \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2556,7 +2570,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
---
- 1025
...
-received_message, from = receiving_socket:recvfrom(512)
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom(512) \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
@@ -2645,7 +2661,9 @@ sending_socket:write(message)
- 513
...
-- case: recvfrom reads first 512 bytes from the message with tcp
-received_message, from = receiving_socket:recvfrom()
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom() \
+end, WAIT_COND_TIME)
---
...
e = receiving_socket:errno()
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index 9ba6b2893..a0803519d 100644
--- a/test/app/socket.test.lua
+++ b/test/app/socket.test.lua
@@ -772,7 +772,9 @@ e == errno.EAGAIN -- expected true
-- case: recv, zero datagram
sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
-received_message = receiving_socket:recv()
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv() \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == '' -- expected true
received_message
@@ -780,7 +782,9 @@ e == 0 -- expected true
-- case: recvfrom, zero datagram
sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
-received_message, from = receiving_socket:recvfrom()
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom() \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == '' -- expected true
received_message
@@ -806,7 +810,9 @@ e == errno.EAGAIN -- expected true
-- case: recv, zero datagram, explicit size
sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
-received_message = receiving_socket:recv(512)
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv(512) \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == '' -- expected true
received_message
@@ -814,7 +820,9 @@ e == 0 -- expected true
-- case: recvfrom, zero datagram, explicit size
sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
-received_message, from = receiving_socket:recvfrom(512)
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom(512) \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == '' -- expected true
received_message
@@ -827,7 +835,9 @@ message = string.rep('x', message_len)
-- case: recv, non-zero length datagram, the buffer size should be evaluated
sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
-received_message = receiving_socket:recv()
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv() \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == message -- expected true
received_message:len()
@@ -838,7 +848,9 @@ e
-- case: recvfrom, non-zero length datagram, the buffer size should be
-- evaluated
sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
-received_message, from = receiving_socket:recvfrom()
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom() \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == message -- expected true
received_message:len()
@@ -850,7 +862,9 @@ e
-- case: recv truncates a datagram larger then the buffer of an explicit size
sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
-received_message = receiving_socket:recv(512)
+received_message = test_run:wait_cond(function() \
+ return receiving_socket:recv(512) \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == message:sub(1, 512) -- expected true
received_message:len() == 512 -- expected true
@@ -866,7 +880,9 @@ message = string.rep('y', message_len)
-- case: recvfrom truncates a datagram larger then the buffer of an explicit size
sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
-received_message, from = receiving_socket:recvfrom(512)
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom(512) \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == message:sub(1, 512) -- expected true
received_message:len() == 512 -- expected true
@@ -895,7 +911,9 @@ receiving_socket = listening_socket:accept()
sending_socket:write(message)
-- case: recvfrom reads first 512 bytes from the message with tcp
-received_message, from = receiving_socket:recvfrom()
+received_message, from = test_run:wait_cond(function() \
+ return receiving_socket:recvfrom() \
+end, WAIT_COND_TIME)
e = receiving_socket:errno()
received_message == message:sub(1, 512) -- expected true
received_message:len() == 512 -- expected true
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Tarantool-patches] [PATCH v3] test: fix flaky socket test
2019-12-17 0:03 ` Vladislav Shpilevoy
@ 2019-12-23 20:48 ` Ilya Kosarev
0 siblings, 0 replies; 3+ messages in thread
From: Ilya Kosarev @ 2019-12-23 20:48 UTC (permalink / raw)
To: Vladislav Shpilevoy; +Cc: tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 27589 bytes --]
Hi!
Thanks for your review.
3 answers are below.
Sent v4 including all proposed changes with minor fixes.
There are 2 comments in that section.
>Вторник, 17 декабря 2019, 3:03 +03:00 от Vladislav Shpilevoy <v.shpilevoy@tarantool.org>:
>
>Thanks for the patch!
>
>See 6 comments below, and 2 review fix commits in the
>end.
>
>On 10/12/2019 15:36, Ilya Kosarev wrote:
>> test: fix flaky socket test
>1. You've duplicated the commit title in the
>commit message.
>
>>
>> socket.test had a number of flaky problems:
>> - socket readiness expectation
>> - race conditions on socket shutdown in emulation test cases
>> - tcp_server stability in socket receive inconsistent behavior case
>> Now they are solved. Port randomization is improved.
>> Socket test is not fragile anymore.
>
>2. It depends on definition of 'fragile test'. Any test using UDP
>can fail under sufficiently high load. And this test is not an
>exception. After your and my fixes it fails extremely rare, but
>still fails. That happens under huge load when the kernel starts
>dropping UDP packets. Not delivers them with a delay, but drops.
>
>But I guess yeah, we can remove the 'fragile' flag. Unless our
>test suite will ever try to run 64 socket tests in parallel a
>couple of times.
>
>>
>> Closes #4451, #4426, #4469
>> ---
>> Branch: https://github.com/tarantool/tarantool/tree/i.kosarev/gh-4426-4451-fix-socket-test
>> Issues: https://github.com/tarantool/tarantool/issues/4426
>> https://github.com/tarantool/tarantool/issues/4451
>> https://github.com/tarantool/tarantool/issues/4469
>>
>> Changes in v2:
>> - reconsidered socket readiness expectation
>> - reduced conditions waiting time
>>
>> Changes in v3:
>> - reconsidered expectations to unify them
>> - simplified randomization
>>
>> test/app/socket.result | 159 +++++++++++++++++++++++++--------------
>> test/app/socket.test.lua | 123 +++++++++++++++++++-----------
>> test/app/suite.ini | 1 -
>> 3 files changed, 178 insertions(+), 105 deletions(-)
>>
>> diff --git a/test/app/socket.result b/test/app/socket.result
>> index fd299424c96..25ad9034374 100644
>> --- a/test/app/socket.result
>> +++ b/test/app/socket.result
>> @@ -278,14 +278,14 @@ s:error()
>> ---
>> - null
>> ...
>> -s:listen(128)
>> +s:listen(WAIT_COND_TIME)
>
>3. I suggest you to read socket.lua source code. Listen takes
>backlog size, not a timeout. The same comment to many other
>places.
3. Right. My bad.
>
>
>> ---
>> - true
>> ...
>> sevres = {}
>> ---
>> ...
>> -type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
>> +type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
>
>4. :readable() has infinity timeout. When you made it WAIT_COND_TIME,
>you certainly didn't improve the test stability. The same in other
>similar places.
4. Ok, i see, we don't need to touch infinite timeout cases.
>
>
>> ---
>> - userdata
>> ...
>> @@ -1004,7 +1004,7 @@ socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
>> - null
>> - true
>> ...
>> -s:listen()
>> +s:listen(WAIT_COND_TIME)
>> ---
>> - true
>> ...
>> @@ -1074,6 +1074,9 @@ master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
>> ---
>> - true
>> ...
>> +math.randomseed(fiber.time())
>
>5. This line becomes useless when you've increased the timeout.
>Moreover, the random port search below also does not make any
>sense. You could use port 0 to bind to any free port instead of
>manual guessing for the kernel.
5. Nice, i didn't consider this possibility. Therefore we don't
need to perform manual random port search.
>
>
>> +---
>> +...
>> port = 32768 + math.random(0, 32767)
>> ---
>> ...
>> @@ -2231,7 +2239,17 @@ test_run:cmd("setopt delimiter ''");
>> receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
>> ---
>> ...
>> -receiving_socket:bind('127.0.0.1', 0)
>> +test_run:cmd("setopt delimiter ';'")
>> +---
>> +- true
>> +...
>> +test_run:wait_cond(function()
>> + return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
>> +end, WAIT_COND_TIME);
>
>6. Why? Bind with 0 port finds a suitable port. You don't
>need to do random guessing. The same in many other places.
>
>> +---
>> +- true
>> +...
>> +test_run:cmd("setopt delimiter ''");
>> ---
>> - true
>> ...
>Below are 2 my commits. First with review fixes to the patch. Second
>with stabilization of UDP tests (well, not 100% stability, but much
>better. 100% would require to rework the tests too much loosing their
>purpose).
>
>Please, review them, squahs if you agree, and send to a second review
>to someone. For example to Alexander. But taking into account urgency
>of 2.3.1 maybe we can't wait several weeks more, so ask Kirill.
>
>===========================================================================
>
>commit fc11ffbd9135554b2b91728483189ababa86c36e
>Author: Vladislav Shpilevoy < v.shpilevoy@tarantool.org >
>Date: Mon Dec 16 23:33:21 2019 +0100
>
> Review fixes
>
>diff --git a/test/app/socket.result b/test/app/socket.result
>index 25ad90343..563519315 100644
>--- a/test/app/socket.result
>+++ b/test/app/socket.result
>@@ -278,14 +278,14 @@ s:error()
> ---
> - null
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen(128)
> ---
> - true
> ...
> sevres = {}
> ---
> ...
>-type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
>+type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
> ---
> - userdata
> ...
>@@ -440,7 +440,7 @@ s:bind('127.0.0.1', 0)
> ---
> - true
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen(128)
> ---
> - true
> ...
>@@ -485,7 +485,7 @@ sa:read(3)
> ---
> - orl
> ...
>-sc:writable(WAIT_COND_TIME)
>+sc:writable()
> ---
> - true
> ...
>@@ -513,7 +513,7 @@ sa:read(1, .01)
> ---
> - null
> ...
>-sc:writable(WAIT_COND_TIME)
>+sc:writable()
> ---
> - true
> ...
>@@ -546,7 +546,7 @@ sc:send('Hello')
> ---
> - 5
> ...
>-sa:readable(WAIT_COND_TIME)
>+sa:readable()
> ---
> - true
> ...
>@@ -646,7 +646,7 @@ sc ~= nil
> ---
> - true
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen(1234)
> ---
> - true
> ...
>@@ -665,7 +665,7 @@ sc:error()
> ---
> - null
> ...
>-s:readable(WAIT_COND_TIME)
>+s:readable()
> ---
> - true
> ...
>@@ -754,7 +754,7 @@ string.match(tostring(sc), ', peer') == nil
> ---
> - true
> ...
>-sc:writable(WAIT_COND_TIME)
>+sc:writable()
> ---
> - true
> ...
>@@ -957,7 +957,7 @@ s:bind('127.0.0.1', 0)
> port = s:name().port
> ---
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen()
> ---
> - true
> ...
>@@ -1004,7 +1004,7 @@ socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
> - null
> - true
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen()
> ---
> - true
> ...
>@@ -1070,39 +1070,21 @@ s = nil
> master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
> ---
> ...
>-master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
>+master:bind('127.0.0.1', 0)
> ---
> - true
> ...
>-math.randomseed(fiber.time())
>----
>-...
>-port = 32768 + math.random(0, 32767)
>+port = master:name().port
> ---
> ...
>--- SO_REUSEADDR allows to bind to the same source addr:port twice,
>--- so listen() can return EADDRINUSE and so we check it within
>--- wait_cond().
> test_run:cmd("setopt delimiter ';'")
> ---
> - true
> ...
>-test_run:wait_cond(function()
>- local ok = master:bind('127.0.0.1', port)
>- local ok = ok and master:listen(WAIT_COND_TIME)
>- if not ok then
>- port = 32768 + math.random(32768)
>- return false, master:error()
>- end
>- return true
>-end, WAIT_COND_TIME);
>----
>-- true
>-...
> function gh361()
> local s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
> s:sysconnect('127.0.0.1', port)
>- s:wait(WAIT_COND_TIME)
>+ s:wait()
> res = s:read(1200)
> end;
> ---
>@@ -1149,7 +1131,7 @@ s:error()
> ---
> - null
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen(128)
> ---
> - true
> ...
>@@ -1159,7 +1141,7 @@ test_run:cmd("setopt delimiter ';'")
> ...
> f = fiber.create(function()
> for i=1,2 do
>- s:readable(WAIT_COND_TIME)
>+ s:readable()
> local sc = s:accept()
> sc:write('ok!')
> sc:shutdown()
>@@ -1535,7 +1517,7 @@ s:bind('unix/', path)
> ---
> - true
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen()
> ---
> - true
> ...
>@@ -1762,7 +1744,7 @@ s:bind('127.0.0.1', 0) -- error handling
> - null
> - Invalid argument
> ...
>-s:listen(WAIT_COND_TIME)
>+s:listen(10)
> ---
> - 1
> ...
>@@ -2239,17 +2221,7 @@ test_run:cmd("setopt delimiter ''");
> receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
> ---
> ...
>-test_run:cmd("setopt delimiter ';'")
>----
>-- true
>-...
>-test_run:wait_cond(function()
>- return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
>-end, WAIT_COND_TIME);
>----
>-- true
>-...
>-test_run:cmd("setopt delimiter ''");
>+receiving_socket:bind('127.0.0.1', 0)
> ---
> - true
> ...
>@@ -2643,21 +2615,11 @@ listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
> ---
> - true
> ...
>-test_run:cmd("setopt delimiter ';'")
>----
>-- true
>-...
>-test_run:wait_cond(function()
>- return listening_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
>-end, WAIT_COND_TIME);
>----
>-- true
>-...
>-test_run:cmd("setopt delimiter ''");
>+listening_socket:bind('127.0.0.1', 0)
> ---
> - true
> ...
>-listening_socket:listen(WAIT_COND_TIME)
>+listening_socket:listen()
> ---
> - true
> ...
>@@ -2671,18 +2633,12 @@ sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errn
> ---
> - true
> ...
>-test_run:cmd("setopt delimiter ';'")
>+listening_socket:readable(WAIT_COND_TIME)
> ---
> - true
> ...
>-receiving_socket = test_run:wait_cond(function()
>- return listening_socket:accept()
>-end, WAIT_COND_TIME);
>----
>-...
>-test_run:cmd("setopt delimiter ''");
>+receiving_socket = listening_socket:accept()
> ---
>-- true
> ...
> sending_socket:write(message)
> ---
>@@ -2864,46 +2820,41 @@ counter = 0
> fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end
> ---
> ...
>-srv = nil
>+srv = socket.tcp_server('0.0.0.0', 0, fn)
> ---
> ...
>-test_run:cmd("setopt delimiter ';'")
>+s = socket.connect('localhost', srv:name().port)
> ---
>-- true
> ...
>-test_run:wait_cond(function()
>- port = 32768 + math.random(0, 32767)
>- srv = socket.tcp_server('0.0.0.0', port, fn)
>- return srv ~= nil
>-end, WAIT_COND_TIME);
>+chan:put(5)
> ---
> - true
> ...
>-receive1 = nil; receive2 = nil;
>+chan:put(5)
> ---
>+- true
> ...
>-if srv ~= nil then
>- s = socket.connect('localhost', port)
>- chan:put(5)
>- chan:put(5)
>- receive1 = s:receive(5)
>- chan:put(5)
>- s:settimeout(1)
>- receive2 = s:receive('*a')
>- s:close()
>- srv:close()
>-end;
>+s:receive(5)
> ---
>+- '00000'
> ...
>-test_run:cmd("setopt delimiter ''");
>+chan:put(5)
> ---
> - true
> ...
>-receive1
>+s:settimeout(1)
> ---
>-- '00000'
>+- 1
> ...
>-receive2
>+s:receive('*a')
> ---
> - '1111122222'
> ...
>+s:close()
>+---
>+- 1
>+...
>+srv:close()
>+---
>+- true
>+...
>diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
>index 97dac794e..9ba6b2893 100644
>--- a/test/app/socket.test.lua
>+++ b/test/app/socket.test.lua
>@@ -91,9 +91,9 @@ s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
> s:error()
> s:bind('127.0.0.1', 0)
> s:error()
>-s:listen(WAIT_COND_TIME)
>+s:listen(128)
> sevres = {}
>-type(require('fiber').create(function() s:readable(WAIT_COND_TIME) do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
>+type(require('fiber').create(function() s:readable() do local sc = s:accept() table.insert(sevres, sc) sc:syswrite('ok') sc:close() end end))
> #sevres
>
> sc = socket('PF_INET', 'SOCK_STREAM', 'tcp')
>@@ -135,7 +135,7 @@ s:close()
> s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
> s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
> s:bind('127.0.0.1', 0)
>-s:listen(WAIT_COND_TIME)
>+s:listen(128)
>
> sc = socket('PF_INET', 'SOCK_STREAM', 'tcp')
>
>@@ -150,14 +150,14 @@ addr2.family == addr.family
> sa:nonblock(1)
> sa:read(8)
> sa:read(3)
>-sc:writable(WAIT_COND_TIME)
>+sc:writable()
> sc:write(', again')
> sa:read(8)
> sa:error()
> string.len(sa:read(0))
> type(sa:read(0))
> sa:read(1, .01)
>-sc:writable(WAIT_COND_TIME)
>+sc:writable()
>
> -- gh-3979 Check for errors when argument is negative.
>
>@@ -170,7 +170,7 @@ sc:send('abc')
> sa:read(3)
>
> sc:send('Hello')
>-sa:readable(WAIT_COND_TIME)
>+sa:readable()
> sa:recv()
> sa:recv()
>
>@@ -198,14 +198,14 @@ path = 'tarantool-test-socket'
> os.remove(path)
> s:bind('unix/', path)
> sc ~= nil
>-s:listen(WAIT_COND_TIME)
>+s:listen(1234)
>
> sc = socket('PF_UNIX', 'SOCK_STREAM', 0)
> sc:nonblock(true)
> sc:sysconnect('unix/', path)
> sc:error()
>
>-s:readable(WAIT_COND_TIME)
>+s:readable()
> sa = s:accept()
> sa:nonblock(true)
> sa:send('Hello, world')
>@@ -243,7 +243,7 @@ sc:getsockopt('SOL_SOCKET', 'SO_ERROR')
> sc:nonblock(true)
> sc:sysconnect('127.0.0.1', 3458) or errno() == errno.EINPROGRESS or errno() == errno.ECONNREFUSED
> string.match(tostring(sc), ', peer') == nil
>-sc:writable(WAIT_COND_TIME)
>+sc:writable()
> string.match(tostring(sc), ', peer') == nil
> socket_error = sc:getsockopt('SOL_SOCKET', 'SO_ERROR')
> socket_error == errno.ECONNREFUSED or socket_error == 0
>@@ -307,7 +307,7 @@ socket.tcp_connect('127.0.0.1', 80, 0.00000000001)
> s = socket('AF_INET', 'SOCK_STREAM', 'tcp')
> s:bind('127.0.0.1', 0)
> port = s:name().port
>-s:listen(WAIT_COND_TIME)
>+s:listen()
> sc, e = socket.tcp_connect('127.0.0.1', port), errno()
> sc ~= nil
> e == 0
>@@ -321,7 +321,7 @@ _ = os.remove(path)
> s = socket('AF_UNIX', 'SOCK_STREAM', 0)
> s:bind('unix/', path)
> socket.tcp_connect('unix/', path), errno() == errno.ECONNREFUSED
>-s:listen(WAIT_COND_TIME)
>+s:listen()
> sc, e = socket.tcp_connect('unix/', path), errno()
> sc ~= nil
> e
>@@ -342,26 +342,13 @@ s = nil
>
> -- random port
> master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
>-master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
>-math.randomseed(fiber.time())
>-port = 32768 + math.random(0, 32767)
>--- SO_REUSEADDR allows to bind to the same source addr:port twice,
>--- so listen() can return EADDRINUSE and so we check it within
>--- wait_cond().
>+master:bind('127.0.0.1', 0)
>+port = master:name().port
1. We need to start listening here.
>
> test_run:cmd("setopt delimiter ';'")
>-test_run:wait_cond(function()
>- local ok = master:bind('127.0.0.1', port)
>- local ok = ok and master:listen(WAIT_COND_TIME)
>- if not ok then
>- port = 32768 + math.random(32768)
>- return false, master:error()
>- end
>- return true
>-end, WAIT_COND_TIME);
> function gh361()
> local s = socket('PF_INET', 'SOCK_STREAM', 'tcp')
> s:sysconnect('127.0.0.1', port)
>- s:wait(WAIT_COND_TIME)
>+ s:wait()
> res = s:read(1200)
> end;
> test_run:cmd("setopt delimiter ''");
>@@ -378,11 +365,11 @@ s:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
> s:error()
> s:bind('unix/', path)
> s:error()
>-s:listen(WAIT_COND_TIME)
>+s:listen(128)
> test_run:cmd("setopt delimiter ';'")
> f = fiber.create(function()
> for i=1,2 do
>- s:readable(WAIT_COND_TIME)
>+ s:readable()
> local sc = s:accept()
> sc:write('ok!')
> sc:shutdown()
>@@ -507,7 +494,7 @@ _ = os.remove(path)
> -- Test that socket is closed on GC
> s = socket('AF_UNIX', 'SOCK_STREAM', 0)
> s:bind('unix/', path)
>-s:listen(WAIT_COND_TIME)
>+s:listen()
> s = nil
> while socket.tcp_connect('unix/', path) do collectgarbage('collect') end
> _ = os.remove(path)
>@@ -603,7 +590,7 @@ s:setoption('tcp-nodelay', true)
> s:setoption('unknown', true)
> s:bind('127.0.0.1', 0)
> s:bind('127.0.0.1', 0) -- error handling
>-s:listen(WAIT_COND_TIME)
>+s:listen(10)
> s -- transformed to tcp{server} socket
> host, port, family = s:getsockname()
> host == '127.0.0.1', type(port) == 'string', family == 'inet'
>@@ -763,11 +750,7 @@ end;
> test_run:cmd("setopt delimiter ''");
>
> receiving_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
>-test_run:cmd("setopt delimiter ';'")
>-test_run:wait_cond(function()
>- return receiving_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
>-end, WAIT_COND_TIME);
>-test_run:cmd("setopt delimiter ''");
>+receiving_socket:bind('127.0.0.1', 0)
> receiving_socket_port = receiving_socket:name().port
> sending_socket = socket('AF_INET', 'SOCK_DGRAM', 'udp')
>
>@@ -902,20 +885,13 @@ message = string.rep('x', message_len)
>
> listening_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp')
> listening_socket:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
>-test_run:cmd("setopt delimiter ';'")
>-test_run:wait_cond(function()
>- return listening_socket:bind('127.0.0.1', 32768 + math.random(0, 32767))
>-end, WAIT_COND_TIME);
>-test_run:cmd("setopt delimiter ''");
>-listening_socket:listen(WAIT_COND_TIME)
>+listening_socket:bind('127.0.0.1', 0)
>+listening_socket:listen()
> listening_socket_port = listening_socket:name().port
> sending_socket = socket('AF_INET', 'SOCK_STREAM', 'tcp')
> sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errno.EINPROGRESS
>-test_run:cmd("setopt delimiter ';'")
>-receiving_socket = test_run:wait_cond(function()
>- return listening_socket:accept()
>-end, WAIT_COND_TIME);
>-test_run:cmd("setopt delimiter ''");
>+listening_socket:readable(WAIT_COND_TIME)
>+receiving_socket = listening_socket:accept()
> sending_socket:write(message)
>
> -- case: recvfrom reads first 512 bytes from the message with tcp
>@@ -983,26 +959,14 @@ test_run:cmd("clear filter")
> chan = fiber.channel()
> counter = 0
> fn = function(s) counter = 0; while true do s:write((tostring(counter)):rep(chan:get())); counter = counter + 1 end end
>-srv = nil
>-test_run:cmd("setopt delimiter ';'")
>-test_run:wait_cond(function()
>- port = 32768 + math.random(0, 32767)
>- srv = socket.tcp_server('0.0.0.0', port, fn)
>- return srv ~= nil
>-end, WAIT_COND_TIME);
>-receive1 = nil; receive2 = nil;
>-if srv ~= nil then
>- s = socket.connect('localhost', port)
>- chan:put(5)
>- chan:put(5)
>- receive1 = s:receive(5)
>- chan:put(5)
>- s:settimeout(1)
>- receive2 = s:receive('*a')
>- s:close()
>- srv:close()
>-end;
>-test_run:cmd("setopt delimiter ''");
>-receive1
>-receive2
>+srv = socket.tcp_server('0.0.0.0', 0, fn)
>+s = socket.connect('localhost', srv:name().port)
>+chan:put(5)
>+chan:put(5)
>+s:receive(5)
>+chan:put(5)
>+s:settimeout(1)
>+s:receive('*a')
>+s:close()
>+srv:close()
2. It still seems to me that we don't want to hang on chan:put() even
in case server fails to start, as far as it makes output less clear.
However, alternative construction also seems to be quite confusing.
Considering that it shouldn't also fail any more, i see why it is
better to leave unwrapped construction.
>
>
>===========================================================================
>
>commit 5a5cd01a0c665f2438e05feb46dde964828f6c01
>Author: Vladislav Shpilevoy < v.shpilevoy@tarantool.org >
>Date: Tue Dec 17 00:20:01 2019 +0100
>
> Make UDP more table on Mac
>
>diff --git a/test/app/socket.result b/test/app/socket.result
>index 563519315..cb3897ee1 100644
>--- a/test/app/socket.result
>+++ b/test/app/socket.result
>@@ -2282,7 +2282,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
> ---
> - 0
> ...
>-received_message = receiving_socket:recv()
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv() \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2305,7 +2307,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
> ---
> - 0
> ...
>-received_message, from = receiving_socket:recvfrom()
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom() \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2382,7 +2386,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
> ---
> - 0
> ...
>-received_message = receiving_socket:recv(512)
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv(512) \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2405,7 +2411,9 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
> ---
> - 0
> ...
>-received_message, from = receiving_socket:recvfrom(512)
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom(512) \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2442,7 +2450,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
> ---
> - 1025
> ...
>-received_message = receiving_socket:recv()
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv() \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2474,7 +2484,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
> ---
> - 1025
> ...
>-received_message, from = receiving_socket:recvfrom()
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom() \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2513,7 +2525,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
> ---
> - 1025
> ...
>-received_message = receiving_socket:recv(512)
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv(512) \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2556,7 +2570,9 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
> ---
> - 1025
> ...
>-received_message, from = receiving_socket:recvfrom(512)
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom(512) \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>@@ -2645,7 +2661,9 @@ sending_socket:write(message)
> - 513
> ...
> -- case: recvfrom reads first 512 bytes from the message with tcp
>-received_message, from = receiving_socket:recvfrom()
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom() \
>+end, WAIT_COND_TIME)
> ---
> ...
> e = receiving_socket:errno()
>diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
>index 9ba6b2893..a0803519d 100644
>--- a/test/app/socket.test.lua
>+++ b/test/app/socket.test.lua
>@@ -772,7 +772,9 @@ e == errno.EAGAIN -- expected true
>
> -- case: recv, zero datagram
> sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
>-received_message = receiving_socket:recv()
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv() \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == '' -- expected true
> received_message
>@@ -780,7 +782,9 @@ e == 0 -- expected true
>
> -- case: recvfrom, zero datagram
> sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
>-received_message, from = receiving_socket:recvfrom()
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom() \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == '' -- expected true
> received_message
>@@ -806,7 +810,9 @@ e == errno.EAGAIN -- expected true
>
> -- case: recv, zero datagram, explicit size
> sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
>-received_message = receiving_socket:recv(512)
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv(512) \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == '' -- expected true
> received_message
>@@ -814,7 +820,9 @@ e == 0 -- expected true
>
> -- case: recvfrom, zero datagram, explicit size
> sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
>-received_message, from = receiving_socket:recvfrom(512)
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom(512) \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == '' -- expected true
> received_message
>@@ -827,7 +835,9 @@ message = string.rep('x', message_len)
>
> -- case: recv, non-zero length datagram, the buffer size should be evaluated
> sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
>-received_message = receiving_socket:recv()
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv() \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == message -- expected true
> received_message:len()
>@@ -838,7 +848,9 @@ e
> -- case: recvfrom, non-zero length datagram, the buffer size should be
> -- evaluated
> sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
>-received_message, from = receiving_socket:recvfrom()
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom() \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == message -- expected true
> received_message:len()
>@@ -850,7 +862,9 @@ e
>
> -- case: recv truncates a datagram larger then the buffer of an explicit size
> sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
>-received_message = receiving_socket:recv(512)
>+received_message = test_run:wait_cond(function() \
>+ return receiving_socket:recv(512) \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == message:sub(1, 512) -- expected true
> received_message:len() == 512 -- expected true
>@@ -866,7 +880,9 @@ message = string.rep('y', message_len)
>
> -- case: recvfrom truncates a datagram larger then the buffer of an explicit size
> sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
>-received_message, from = receiving_socket:recvfrom(512)
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom(512) \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == message:sub(1, 512) -- expected true
> received_message:len() == 512 -- expected true
>@@ -895,7 +911,9 @@ receiving_socket = listening_socket:accept()
> sending_socket:write(message)
>
> -- case: recvfrom reads first 512 bytes from the message with tcp
>-received_message, from = receiving_socket:recvfrom()
>+received_message, from = test_run:wait_cond(function() \
>+ return receiving_socket:recvfrom() \
>+end, WAIT_COND_TIME)
> e = receiving_socket:errno()
> received_message == message:sub(1, 512) -- expected true
> received_message:len() == 512 -- expected true
--
Ilya Kosarev
[-- Attachment #2: Type: text/html, Size: 34217 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2019-12-23 20:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-12-10 14:36 [Tarantool-patches] [PATCH v3] test: fix flaky socket test Ilya Kosarev
2019-12-17 0:03 ` Vladislav Shpilevoy
2019-12-23 20:48 ` Ilya Kosarev
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox