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

Alexander Turenko alexander.turenko at tarantool.org
Thu Dec 26 02:43:27 MSK 2019


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.


More information about the Tarantool-patches mailing list