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