Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Bronnikov <sergeyb@tarantool.org>,
	tarantool-patches@dev.tarantool.org, lvasiliev@tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v7 3/3] luacheck: fix warnings in test/box-tap
Date: Tue, 12 Jan 2021 15:52:28 +0100	[thread overview]
Message-ID: <a2376ed0-196c-4344-e656-483fc0ce79d6@tarantool.org> (raw)
In-Reply-To: <7fd3e42f-047b-e404-f481-689eb73acb8b@tarantool.org>

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:

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

  reply	other threads:[~2021-01-12 14:52 UTC|newest]

Thread overview: 18+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-12-22 14:54 [Tarantool-patches] [PATCH v7 0/3] Fix luacheck warnings in test/box-tap, test/box and test/box-py sergeyb
2020-12-22 14:54 ` [Tarantool-patches] [PATCH 1/3] luacheck: fix warnings in test/box sergeyb
2021-01-11 17:52   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-12 15:26     ` Sergey Bronnikov via Tarantool-patches
2021-01-13  7:58       ` Sergey Bronnikov via Tarantool-patches
2021-01-13 16:38       ` Vladislav Shpilevoy via Tarantool-patches
2021-01-14  7:49         ` Sergey Bronnikov via Tarantool-patches
2020-12-22 14:54 ` [Tarantool-patches] [PATCH v7 2/3] luacheck: fix warnings in test/box-py sergeyb
2020-12-22 14:54 ` [Tarantool-patches] [PATCH v7 3/3] luacheck: fix warnings in test/box-tap sergeyb
2021-01-11 17:52   ` Vladislav Shpilevoy via Tarantool-patches
2021-01-12 13:59     ` Sergey Bronnikov via Tarantool-patches
2021-01-12 14:52       ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-01-12 15:21         ` Sergey Bronnikov via Tarantool-patches
2020-12-25 10:37 ` [Tarantool-patches] [PATCH v7 0/3] Fix luacheck warnings in test/box-tap, test/box and test/box-py Kirill Yukhin
2020-12-30 10:17   ` Kirill Yukhin
2020-12-30 12:27 ` Leonid Vasiliev
2021-01-14 21:34 ` Vladislav Shpilevoy via Tarantool-patches
2021-01-15  9:48 ` Kirill Yukhin via Tarantool-patches

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=a2376ed0-196c-4344-e656-483fc0ce79d6@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=lvasiliev@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH v7 3/3] luacheck: fix warnings in test/box-tap' \
    /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