Tarantool development patches archive
 help / color / mirror / Atom feed
From: Alexander Turenko <alexander.turenko@tarantool.org>
To: Ilya Kosarev <i.kosarev@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v5] test: fix flaky socket test
Date: Thu, 26 Dec 2019 02:43:27 +0300	[thread overview]
Message-ID: <20191225234327.hfecptmxityb7fko@tkn_work_nb> (raw)
In-Reply-To: <20191225210407.32120-1-i.kosarev@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).

> @@ -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.

  reply	other threads:[~2019-12-25 23:43 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-12-25 21:04 Ilya Kosarev
2019-12-25 23:43 ` Alexander Turenko [this message]
2019-12-27 12:37   ` Ilya Kosarev

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20191225234327.hfecptmxityb7fko@tkn_work_nb \
    --to=alexander.turenko@tarantool.org \
    --cc=i.kosarev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v5] test: fix flaky socket test' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox