Re: [Tarantool-patches] [PATCH v7 3/3] luacheck: fix warnings in test/box-tap

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

On 12.01.2021 17:52, Vladislav Shpilevoy wrote:
> On 12.01.2021 14:59, Sergey Bronnikov wrote:
>> Thanks for review!
>>
>> On 11.01.2021 20:52, Vladislav Shpilevoy wrote:
>>> Thanks for the patch!
>>>
>>>> diff --git a/.luacheckrc b/.luacheckrc
>>>> index 52625bb35..b7f9abb45 100644
>>>> --- a/.luacheckrc
>>>> +++ b/.luacheckrc
>>>> @@ -157,3 +156,23 @@ files["test/box/lua/fifo.lua"] = {
>>>>    files["test/box/lua/identifier.lua"] = {
>>>>         globals = {"run_test"}
>>>>    }
>>>> +files["test/box-tap/session.test.lua"] = {
>>>> +    globals = {
>>>> +        "active_connections",
>>> This can be declared 'local'.
>> and with removed a single suppression in .luacheckrc we have got another warnings,
>>
>> that should be suppressed too (inline or in luacheckrc):
>>
>> Checking test/box-tap/session.test.lua            4 warnings
>>
>>      test/box-tap/session.test.lua:73:22: setting non-standard global variable active_connections
>>      test/box-tap/session.test.lua:73:43: accessing undefined variable active_connections
>>      test/box-tap/session.test.lua:74:22: setting non-standard global variable active_connections
>>      test/box-tap/session.test.lua:74:43: accessing undefined variable active_connections
>>
>> I think it is better to keep it as is.
> It seems you didn't bother with applying my diff from the previous
> email, did you? I kept it below:


You are right, missed that active_connections must be moved above.

Now luacheck is green.

>
>>> ====================
>>> @@ -70,6 +70,7 @@ session.on_disconnect(nil, fail)
>>>      -- check how connect/disconnect triggers work
>>>    local peer_name = "peer_name"
>>> +local active_connections = 0
>>>    local function inc() active_connections = active_connections + 1 end
>>>    local function dec() active_connections = active_connections - 1 end
>>>    local function peer() peer_name = box.session.peer() end
>>> @@ -77,7 +78,6 @@ local net = { box = require('net.box') }
>>>    test:is(type(session.on_connect(inc)), "function", "type of trigger inc on_connect")
>>>    test:is(type(session.on_disconnect(dec)), "function", "type of trigger dec on_disconnect")
>>>    test:is(type(session.on_disconnect(peer)), "function", "type of trigger peer on_disconnect")
>>> -active_connections = 0
>>>    local c = net.box.connect(HOST, PORT)
>>> ====================
> I will paste here the full diff now, with the line removed from
> .luacheck:
>
> ====================
> diff --git a/.luacheckrc b/.luacheckrc
> index b7f9abb45..6043722f4 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -158,7 +158,6 @@ files["test/box/lua/identifier.lua"] = {
>   }
>   files["test/box-tap/session.test.lua"] = {
>       globals = {
> -        "active_connections",
>           "session",
>           "space",
>           "f1",
> diff --git a/test/box-tap/session.test.lua b/test/box-tap/session.test.lua
> index 160f047bc..3dd159ebb 100755
> --- a/test/box-tap/session.test.lua
> +++ b/test/box-tap/session.test.lua
> @@ -70,6 +70,7 @@ session.on_disconnect(nil, fail)
>   
>   -- check how connect/disconnect triggers work
>   local peer_name = "peer_name"
> +local active_connections = 0
>   local function inc() active_connections = active_connections + 1 end
>   local function dec() active_connections = active_connections - 1 end
>   local function peer() peer_name = box.session.peer() end
> @@ -77,7 +78,6 @@ local net = { box = require('net.box') }
>   test:is(type(session.on_connect(inc)), "function", "type of trigger inc on_connect")
>   test:is(type(session.on_disconnect(dec)), "function", "type of trigger dec on_disconnect")
>   test:is(type(session.on_disconnect(peer)), "function", "type of trigger peer on_disconnect")
> -active_connections = 0
>   local c = net.box.connect(HOST, PORT)
> ====================
>
> You can copy-paste it into a file and use `git apply` to repeat
> it exactly. From the errors you have shown above it looks like
> you didn't move the declaration above its usages (I tried it and
> got the same errors, of course). You just added 'local', and didn't
> try to move it.
>
> With this diff I run luacheck command and get 0 errors/warnings.
>
> ====================
> $ ./.rocks/bin/luacheck --codes --config .luacheckrc test/box-tap/session.test.lua
> Checking test/box-tap/session.test.lua            OK
>
> Total: 0 warnings / 0 errors in 1 file
> ====================