Re: [Tarantool-patches] [PATCH v2] test: set unix sockets for iproto at core = app

["Oleg Piskunov" <o.piskunov@tarantool.org>]

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


LGTM.

  
>Четверг, 21 мая 2020, 14:38 +03:00 от Alexander V. Tikhonov <avtikhon@tarantool.org>:
> 
>Alexander, thanks for the review, I've corrected all as you suggested.
>Sure I'll control that test-run must be pushed before this patch.
>
>On Thu, May 21, 2020 at 02:29:33PM +0300, Alexander Turenko wrote:
>> LGTM.
>>
>> (But should NOT be pushed to master until I'll push test-run fix from PR
>> #211.)
>>
>> Left a couple of nits, however. See below.
>>
>> WBR, Alexander Turenko.
>>
>> > 60f84cbf ('test: use unix sockets for iproto connections')
>>
>> Nit: We mention commits in the form
>> 0000000000000000000000000000000000000000 ('commit message header'): full
>> 40-digits hash and the commit message header. You'll need to carry the
>> header when will use full commit hash and it is okay.
>>
>> Personally I think that short hashes lookes better when used inline
>> within a text and that our style will recommend them sooner or later.
>> But now it is not so.
>>
>> > diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
>> > index 4d7059559..dd90c8a25 100755
>> > --- a/test/app-tap/tarantoolctl.test.lua
>> > +++ b/test/app-tap/tarantoolctl.test.lua
>> > @@ -465,12 +465,9 @@ else
>> > local remote_path = create_script(dir, 'remote.lua', remote_code)
>> > test_run:cmd(("create server remote with script='%s'"):format(remote_path))
>> > test_run:cmd("start server remote")
>> > - local port = tonumber(
>> > - test_run:eval("remote",
>> > - "return require('uri').parse(box.cfg.listen).service")[1]
>> > - )
>> > + local admin_socket = test_run:eval("remote", "return box.cfg.listen")[1]
>>
>> Nit: Admin port / socket usually refer a console port / socket. I would
>> name the variable `listen_url` or `remote_uri`. The former is used
>> several times across tests and also is part of `return_listen_uri`
>> option name. 
 
 
--
Oleg Piskunov
 

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