[Tarantool-patches] [PATCH v5] test: fix flaky socket test

[Tarantool-patches] [PATCH v5] test: fix flaky socket test

[Ilya Kosarev]

socket.test had a number of flaky problems:
- socket readiness expectation
- race conditions on socket shutdown in emulation test cases
- UDP datagrams losses on mac os
- excessive random port searches
Now they are solved. We are also using 127.0.0.1 instead of 0.0.0.0 or
localhost to prevent wrong connections. Socket test is not fragile
anymore.

Closes #4426
Closes #4451
Closes #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

Changes in v4:
- left infinite timeouts alone
- wrapped UDP datagrams awaiting with wait_cond
- replaced manual port randomization with zero bind
- removed extra wait_cond wrappers

Changes in v5:
- used 127.0.0.1 instead of 0.0.0.0 or localhost
- replaced wait_cond with readableness awaiting for UDP datagrams
- renamed WAIT_COND_TIME to TIMEOUT
- refactored race conditions on socket shutdown to use channel instead
  of global flag

 test/app/socket.result   | 120 ++++++++++++++++++++++++++-------------
 test/app/socket.test.lua |  83 +++++++++++++++------------
 test/app/suite.ini       |   1 -
 3 files changed, 127 insertions(+), 77 deletions(-)

diff --git a/test/app/socket.result b/test/app/socket.result
index fd299424c96..f3230a2ffc9 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
+TIMEOUT = 100
 ---
 ...
 socket('PF_INET', 'SOCK_STREAM', 'tcp121222');
@@ -107,11 +107,11 @@ s:nonblock(true)
 ---
 - true
 ...
-s:readable(.1)
+s:readable(TIMEOUT)
 ---
 - true
 ...
-s:wait(.1)
+s:wait(TIMEOUT)
 ---
 - RW
 ...
@@ -183,7 +183,7 @@ s:writable(0)
 ---
 - true
 ...
-s:wait(.01)
+s:wait(TIMEOUT)
 ---
 - RW
 ...
@@ -227,11 +227,11 @@ s:syswrite(ffi.cast('const char *', ping), #ping)
 ---
 - 6
 ...
-s:readable(1)
+s:readable(TIMEOUT)
 ---
 - true
 ...
-s:wait(.01)
+s:wait(TIMEOUT)
 ---
 - RW
 ...
@@ -308,7 +308,7 @@ sc:nonblock(true)
 ---
 - true
 ...
-sc:readable(.5)
+sc:readable(TIMEOUT)
 ---
 - true
 ...
@@ -451,7 +451,7 @@ sc:sysconnect('127.0.0.1', s:name().port) or errno() == errno.EINPROGRESS
 ---
 - true
 ...
-sc:writable(10)
+sc:writable(TIMEOUT)
 ---
 - true
 ...
@@ -715,7 +715,7 @@ function aexitst(ai, hostnames, port)
 end;
 ---
 ...
-aexitst( socket.getaddrinfo('localhost', 'http', {  protocol = 'tcp',
+aexitst( socket.getaddrinfo('127.0.0.1', 'http', {  protocol = 'tcp',
     type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 );
 ---
 - true
@@ -830,7 +830,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, world')
 ---
 - 12
 ...
-s:readable(10)
+s:readable(TIMEOUT)
 ---
 - true
 ...
@@ -842,7 +842,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2')
 ---
 - 15
 ...
-s:readable(10)
+s:readable(TIMEOUT)
 ---
 - true
 ...
@@ -898,7 +898,7 @@ sc:sendto('127.0.0.1', s:name().port, 'Hello, World!')
 ---
 - 13
 ...
-s:readable(1)
+s:readable(TIMEOUT)
 ---
 - true
 ...
@@ -913,7 +913,7 @@ s:sendto(from.host, from.port, 'Hello, hello!')
 ---
 - 13
 ...
-sc:readable(1)
+sc:readable(TIMEOUT)
 ---
 - true
 ...
@@ -1070,29 +1070,18 @@ s = nil
 master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
 ---
 ...
-master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
+master:bind('127.0.0.1', 0)
 ---
 - true
 ...
-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 ';'")
+master:listen()
 ---
 - true
 ...
-test_run:wait_cond(function()
-    local ok = master:bind('127.0.0.1', port)
-    local ok = ok and master:listen()
-    if not ok then
-        port = 32768 + math.random(32768)
-        return false, master:error()
-    end
-    return true
-end, WAIT_COND_TIME);
+test_run:cmd("setopt delimiter ';'")
 ---
 - true
 ...
@@ -1322,7 +1311,7 @@ test_run:cmd("setopt delimiter ';'")
 ---
 - true
 ...
-server, addr = socket.tcp_server('localhost', 0, { handler = function(s)
+server, addr = socket.tcp_server('127.0.0.1', 0, { handler = function(s)
     s:read(2)
     s:write('Hello, world')
 end, name = 'testserv'});
@@ -1632,7 +1621,7 @@ test_run:cmd("setopt delimiter ';'")
 ...
 f = fiber.create(function()
     while true do
-        local result = socket.getaddrinfo('localhost', '80')
+        local result = socket.getaddrinfo('127.0.0.1', '80')
         fiber.sleep(0)
     end
 end);
@@ -1823,7 +1812,16 @@ test_run:cmd("setopt delimiter ';'")
 - true
 ...
 cfiber = fiber.create(function(sc, rch, wch)
-    while sc:send(wch:get()) and rch:put(sc:receive("*l")) do end
+    while true do
+        local outgoing_data = wch:get()
+        if outgoing_data == nil then
+            sc:close()
+            break
+        end
+        sc:send(outgoing_data)
+        local incoming_data = sc:receive("*l")
+        rch:put(incoming_data)
+    end
 end, sc, rch, wch);
 ---
 ...
@@ -1936,6 +1934,10 @@ c:receive("*l")
 ---
 - 
 ...
+rch:get()
+---
+- $250 OK
+...
 wch:put("Fu")
 ---
 - true
@@ -1944,9 +1946,9 @@ c:send("354 Please type your message\n")
 ---
 - 29
 ...
-sc:close()
+wch:put(nil) -- EOF
 ---
-- 1
+- true
 ...
 c:receive("*l", "Line: ")
 ---
@@ -2155,7 +2157,7 @@ s:close()
 - 1
 ...
 -- socket.bind / socket.connect
-s = socket.bind('0.0.0.0', 0)
+s = socket.bind('127.0.0.1', 0)
 ---
 ...
 s
@@ -2292,6 +2294,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
 ---
 - 0
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message = receiving_socket:recv()
 ---
 ...
@@ -2315,6 +2321,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
 ---
 - 0
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message, from = receiving_socket:recvfrom()
 ---
 ...
@@ -2392,6 +2402,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
 ---
 - 0
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message = receiving_socket:recv(512)
 ---
 ...
@@ -2415,6 +2429,10 @@ sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
 ---
 - 0
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message, from = receiving_socket:recvfrom(512)
 ---
 ...
@@ -2452,6 +2470,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
 ---
 - 1025
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message = receiving_socket:recv()
 ---
 ...
@@ -2484,6 +2506,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
 ---
 - 1025
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message, from = receiving_socket:recvfrom()
 ---
 ...
@@ -2523,6 +2549,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
 ---
 - 1025
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message = receiving_socket:recv(512)
 ---
 ...
@@ -2566,6 +2596,10 @@ sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
 ---
 - 1025
 ...
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message, from = receiving_socket:recvfrom(512)
 ---
 ...
@@ -2643,6 +2677,10 @@ sending_socket:sysconnect('127.0.0.1', listening_socket_port) or errno() == errn
 ---
 - true
 ...
+listening_socket:readable(TIMEOUT)
+---
+- true
+...
 receiving_socket = listening_socket:accept()
 ---
 ...
@@ -2651,6 +2689,10 @@ sending_socket:write(message)
 - 513
 ...
 -- case: recvfrom reads first 512 bytes from the message with tcp
+receiving_socket:readable(TIMEOUT)
+---
+- true
+...
 received_message, from = receiving_socket:recvfrom()
 ---
 ...
@@ -2747,7 +2789,7 @@ test_run:cmd("setopt delimiter ';'")
 ...
 echo_fiber = nil
 channel = fiber.channel()
-server = socket.tcp_server('localhost', 0, { handler = function(s)
+server = socket.tcp_server('127.0.0.1', 0, { handler = function(s)
     echo_fiber = fiber.self()
     channel:put(true)
     while true do
@@ -2804,7 +2846,7 @@ client:read(5, 5) == 'world'
 fiber.cancel(echo_fiber)
 ---
 ...
-client:read(1, 5) == ''
+client:read(1, TIMEOUT) == ''
 ---
 - true
 ...
@@ -2816,7 +2858,7 @@ test_run:cmd("clear filter")
 ---
 - true
 ...
--- case: sicket receive inconsistent behavior
+-- case: socket receive inconsistent behavior
 chan = fiber.channel()
 ---
 ...
@@ -2826,10 +2868,10 @@ 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 = socket.tcp_server('127.0.0.1', 0, fn)
 ---
 ...
-s = socket.connect('localhost', 8888)
+s = socket.connect('127.0.0.1', srv:name().port)
 ---
 ...
 chan:put(5)
diff --git a/test/app/socket.test.lua b/test/app/socket.test.lua
index c72d41763f4..9873b6a32d4 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
+TIMEOUT = 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(TIMEOUT)
+s:wait(TIMEOUT)
 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(TIMEOUT)
 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(TIMEOUT)
+s:wait(TIMEOUT)
 
 pong = s:sysread()
 string.len(pong)
@@ -100,7 +100,7 @@ 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(TIMEOUT)
 sc:sysread()
 string.match(tostring(sc), ', peer') ~= nil
 #sevres
@@ -140,7 +140,7 @@ s:listen(128)
 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(TIMEOUT)
 sc:write('Hello, world')
 
 sa, addr = s:accept()
@@ -230,7 +230,7 @@ function aexitst(ai, hostnames, port)
 end;
 
 
-aexitst( socket.getaddrinfo('localhost', 'http', {  protocol = 'tcp',
+aexitst( socket.getaddrinfo('127.0.0.1', 'http', {  protocol = 'tcp',
     type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 );
 test_run:cmd("setopt delimiter ''");
 
@@ -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(TIMEOUT)
 s:recv()
 
 sc:sendto('127.0.0.1', s:name().port, 'Hello, world, 2')
-s:readable(10)
+s:readable(TIMEOUT)
 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(TIMEOUT)
 data, from = s:recvfrom(10)
 data
 s:sendto(from.host, from.port, 'Hello, hello!')
-sc:readable(1)
+sc:readable(TIMEOUT)
 data_r, from_r = sc:recvfrom()
 data_r
 from_r.host
@@ -342,21 +342,10 @@ s = nil
 
 -- random port
 master = socket('PF_INET', 'SOCK_STREAM', 'tcp')
-master:setsockopt('SOL_SOCKET', 'SO_REUSEADDR', true)
-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
+master:listen()
 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()
-    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)
@@ -434,7 +423,7 @@ server:close()
 while fio.stat(path) ~= nil do fiber.sleep(0.001) end
 
 test_run:cmd("setopt delimiter ';'")
-server, addr = socket.tcp_server('localhost', 0, { handler = function(s)
+server, addr = socket.tcp_server('127.0.0.1', 0, { handler = function(s)
     s:read(2)
     s:write('Hello, world')
 end, name = 'testserv'});
@@ -546,7 +535,7 @@ socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == er
 test_run:cmd("setopt delimiter ';'")
 f = fiber.create(function()
     while true do
-        local result = socket.getaddrinfo('localhost', '80')
+        local result = socket.getaddrinfo('127.0.0.1', '80')
         fiber.sleep(0)
     end
 end);
@@ -620,7 +609,16 @@ rch, wch = fiber.channel(1), fiber.channel(1)
 sc = socket.connect(host, port)
 test_run:cmd("setopt delimiter ';'")
 cfiber = fiber.create(function(sc, rch, wch)
-    while sc:send(wch:get()) and rch:put(sc:receive("*l")) do end
+    while true do
+        local outgoing_data = wch:get()
+        if outgoing_data == nil then
+            sc:close()
+            break
+        end
+        sc:send(outgoing_data)
+        local incoming_data = sc:receive("*l")
+        rch:put(incoming_data)
+    end
 end, sc, rch, wch);
 test_run:cmd("setopt delimiter ''");
 
@@ -651,9 +649,10 @@ rch:get()
 wch:put("DATA\n")
 c:receive(4)
 c:receive("*l")
+rch:get()
 wch:put("Fu")
 c:send("354 Please type your message\n")
-sc:close()
+wch:put(nil) -- EOF
 c:receive("*l", "Line: ")
 c:receive()
 c:receive(10)
@@ -713,7 +712,7 @@ sc:close()
 s:close()
 
 -- socket.bind / socket.connect
-s = socket.bind('0.0.0.0', 0)
+s = socket.bind('127.0.0.1', 0)
 s
 host, port, family = s:getsockname()
 sc = socket.connect(host, port)
@@ -778,6 +777,7 @@ e == errno.EAGAIN -- expected true
 
 -- case: recv, zero datagram
 sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
+receiving_socket:readable(TIMEOUT)
 received_message = receiving_socket:recv()
 e = receiving_socket:errno()
 received_message == '' -- expected true
@@ -786,6 +786,7 @@ e == 0 -- expected true
 
 -- case: recvfrom, zero datagram
 sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
+receiving_socket:readable(TIMEOUT)
 received_message, from = receiving_socket:recvfrom()
 e = receiving_socket:errno()
 received_message == '' -- expected true
@@ -812,6 +813,7 @@ e == errno.EAGAIN -- expected true
 
 -- case: recv, zero datagram, explicit size
 sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
+receiving_socket:readable(TIMEOUT)
 received_message = receiving_socket:recv(512)
 e = receiving_socket:errno()
 received_message == '' -- expected true
@@ -820,6 +822,7 @@ e == 0 -- expected true
 
 -- case: recvfrom, zero datagram, explicit size
 sendto_zero(sending_socket, '127.0.0.1', receiving_socket_port)
+receiving_socket:readable(TIMEOUT)
 received_message, from = receiving_socket:recvfrom(512)
 e = receiving_socket:errno()
 received_message == '' -- expected true
@@ -833,6 +836,7 @@ 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)
+receiving_socket:readable(TIMEOUT)
 received_message = receiving_socket:recv()
 e = receiving_socket:errno()
 received_message == message -- expected true
@@ -844,6 +848,7 @@ e
 -- case: recvfrom, non-zero length datagram, the buffer size should be
 -- evaluated
 sending_socket:sendto('127.0.0.1', receiving_socket_port, message)
+receiving_socket:readable(TIMEOUT)
 received_message, from = receiving_socket:recvfrom()
 e = receiving_socket:errno()
 received_message == message -- expected true
@@ -856,6 +861,7 @@ 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)
+receiving_socket:readable(TIMEOUT)
 received_message = receiving_socket:recv(512)
 e = receiving_socket:errno()
 received_message == message:sub(1, 512) -- expected true
@@ -872,6 +878,7 @@ 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)
+receiving_socket:readable(TIMEOUT)
 received_message, from = receiving_socket:recvfrom(512)
 e = receiving_socket:errno()
 received_message == message:sub(1, 512) -- expected true
@@ -896,10 +903,12 @@ 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
+listening_socket:readable(TIMEOUT)
 receiving_socket = listening_socket:accept()
 sending_socket:write(message)
 
 -- case: recvfrom reads first 512 bytes from the message with tcp
+receiving_socket:readable(TIMEOUT)
 received_message, from = receiving_socket:recvfrom()
 e = receiving_socket:errno()
 received_message == message:sub(1, 512) -- expected true
@@ -931,7 +940,7 @@ listening_socket:close()
 test_run:cmd("setopt delimiter ';'")
 echo_fiber = nil
 channel = fiber.channel()
-server = socket.tcp_server('localhost', 0, { handler = function(s)
+server = socket.tcp_server('127.0.0.1', 0, { handler = function(s)
     echo_fiber = fiber.self()
     channel:put(true)
     while true do
@@ -955,17 +964,17 @@ client:write('world')
 client:read(5, 5) == 'world'
 -- cancel fiber
 fiber.cancel(echo_fiber)
-client:read(1, 5) == ''
+client:read(1, TIMEOUT) == ''
 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)
+srv = socket.tcp_server('127.0.0.1', 0, fn)
+s = socket.connect('127.0.0.1', srv:name().port)
 chan:put(5)
 chan:put(5)
 s:receive(5)
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

Re: [Tarantool-patches] [PATCH v5] test: fix flaky socket test

[Alexander Turenko]

Now it works like a charm with 30 jobs and mostly works with 50 jobs in
parallel on my machine. This is the good change.

A few notes below.

WBR, Alexander Turenko.

> 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

> @@ -230,7 +230,7 @@ function aexitst(ai, hostnames, port)
>  end;
>  
>  
> -aexitst( socket.getaddrinfo('localhost', 'http', {  protocol = 'tcp',
> +aexitst( socket.getaddrinfo('127.0.0.1', 'http', {  protocol = 'tcp',
>      type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 );

This is the test case for getaddrinfo() itself. It is better to check
here that 'localhost' actually resolved to either 127.0.0.1 or ::1
(typically both, but it depends of IPv4 / IPv6 support on a particular
system).

> @@ -546,7 +535,7 @@ socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == er
>  test_run:cmd("setopt delimiter ';'")
>  f = fiber.create(function()
>      while true do
> -        local result = socket.getaddrinfo('localhost', '80')
> +        local result = socket.getaddrinfo('127.0.0.1', '80')
>          fiber.sleep(0)
>      end
>  end);

Same here.

> @@ -955,17 +964,17 @@ client:write('world')
>  client:read(5, 5) == 'world'
>  -- cancel fiber
>  fiber.cancel(echo_fiber)
> -client:read(1, 5) == ''
> +client:read(1, TIMEOUT) == ''
>  server:close()

It worth to use TIMEOUT constant instead of 5 in two near :read() calls
above this one:

 | client:read(5, 5) == 'hello'
 | client:read(5, 5) == 'world'

There is also other case where it is applicable:

 | sc:send('Hello')
 | sc:send(', world')
 | sc:send("\\nnew line")
 | sa:read('\\n', 1)                           -- !!
 | sa:read({ chunk = 1, delimiter = 'ine'}, 1) -- !!
 | sa:read('ine', 1)                           -- !!
 | sa:read('ine', 0.1)

All reads except the last one should use the common TIMEOUT. The last
one expected to block on 0.1 seconds and then return nil, so don't
increase timeout here.

And the case right after this one:

 | sc:send('Hello, world')
 | sa:read(',', 1)                             -- !!
 | sc:shutdown('W')
 | sa:close()
 | sc:close()

This way the change will be consistent within the file, at least in
context of :read() calls with a timeout.

Those cases do not fail for me (I don't know why), so it is purely for
consistency. Existing test cases will show our practices, that's good
for ones who'll write more cases in a future.

Re: [Tarantool-patches] [PATCH v5] test: fix flaky socket test

[Ilya Kosarev]

[-- Attachment #1: Type: text/plain, Size: 3061 bytes --]

Hi!

Thanks for the comments.

Sent v6 of the patch considering them.

>Четверг, 26 декабря 2019, 2:43 +03:00 от Alexander Turenko <alexander.turenko@tarantool.org>:
>
>Now it works like a charm with 30 jobs and mostly works with 50 jobs in
>parallel on my machine. This is the good change.
>
>A few notes below.
>
>WBR, Alexander Turenko.
>
>> 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
>> @@ -230,7 +230,7 @@ function aexitst(ai, hostnames, port)
>>  end;
>> 
>> 
>> -aexitst( socket.getaddrinfo('localhost', 'http', {  protocol = 'tcp',
>> +aexitst( socket.getaddrinfo('127.0.0.1', 'http', {  protocol = 'tcp',
>>      type = 'SOCK_STREAM'}), {'127.0.0.1', '::1'}, 80 );
>
>This is the test case for getaddrinfo() itself. It is better to check
>here that 'localhost' actually resolved to either 127.0.0.1 or ::1
>(typically both, but it depends of IPv4 / IPv6 support on a particular
>system). 
Right, got too far here.
>
>
>> @@ -546,7 +535,7 @@ socket.getaddrinfo('host', 'port', { flags = 'WRONG' }) == nil and errno() == er
>>  test_run:cmd("setopt delimiter ';'")
>>  f = fiber.create(function()
>>      while true do
>> -        local result = socket.getaddrinfo('localhost', '80')
>> +        local result = socket.getaddrinfo('127.0.0.1', '80')
>>          fiber.sleep(0)
>>      end
>>  end);
>
>Same here.
>
>> @@ -955,17 +964,17 @@ client:write('world')
>>  client:read(5, 5) == 'world'
>>  -- cancel fiber
>>  fiber.cancel(echo_fiber)
>> -client:read(1, 5) == ''
>> +client:read(1, TIMEOUT) == ''
>>  server:close()
>
>It worth to use TIMEOUT constant instead of 5 in two near :read() calls
>above this one:
>
> | client:read(5, 5) == 'hello'
> | client:read(5, 5) == 'world'
>
>There is also other case where it is applicable:
>
> | sc:send('Hello')
> | sc:send(', world')
> | sc:send("\\nnew line")
> | sa:read('\\n', 1)                           -- !!
> | sa:read({ chunk = 1, delimiter = 'ine'}, 1) -- !!
> | sa:read('ine', 1)                           -- !!
> | sa:read('ine', 0.1)
>
>All reads except the last one should use the common TIMEOUT. The last
>one expected to block on 0.1 seconds and then return nil, so don't
>increase timeout here.
>
>And the case right after this one:
>
> | sc:send('Hello, world')
> | sa:read(',', 1)                             -- !!
> | sc:shutdown('W')
> | sa:close()
> | sc:close()
>
>This way the change will be consistent within the file, at least in
>context of :read() calls with a timeout.
>
>Those cases do not fail for me (I don't know why), so it is purely for
>consistency. Existing test cases will show our practices, that's good
>for ones who'll write more cases in a future.
Right, i also didn't see any fails for those reads and decided not to
interfere with them. However i agree that consistency is also important.
>


-- 
Ilya Kosarev

[-- Attachment #2: Type: text/html, Size: 4520 bytes --]