* [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