[Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/replication and test/replication-py

[Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/replication and test/replication-py

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Changelog v7:

- rebased to master branch

Changelog v6:

- splitted patch in test/ for patches per sub-directory
- adjusted suppressions in .luacheckrc
- fixed formatting issues in .luacheckrc

Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/244921393
Branch: ligurio/gh-5461-luacheck-warnings-test-replication
Issues:
  - https://github.com/tarantool/tarantool/issues/5461
  - https://github.com/tarantool/tarantool/issues/5462

Sergey Bronnikov (2):
  test: fix luacheck warnings in test/replication
  test: fix luacheck warnings in test/replication-py

 .luacheckrc                                 |  9 ++++--
 test/replication-py/master.lua              |  2 +-
 test/replication-py/panic.lua               |  2 +-
 test/replication-py/replica.lua             |  4 ---
 test/replication/lua/fast_replica.lua       | 33 +++++++++++----------
 test/replication/lua/rlimit.lua             |  2 +-
 test/replication/master.lua                 |  2 +-
 test/replication/master_quorum.lua          |  4 +--
 test/replication/on_replace.lua             |  6 ++--
 test/replication/replica.lua                |  4 +--
 test/replication/replica_on_schema_init.lua |  4 +--
 test/replication/replica_quorum.lua         |  4 +--
 12 files changed, 39 insertions(+), 37 deletions(-)

-- 
2.25.1

[Tarantool-patches] [PATCH v7 1/2] test: fix luacheck warnings in test/replication

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Closes #5461

Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Reviewed-by: Igor Munkin <imun@tarantool.org>

Co-authored-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Co-authored-by: Igor Munkin <imun@tarantool.org>
---
 .luacheckrc                                 |  8 ++++-
 test/replication/lua/fast_replica.lua       | 33 +++++++++++----------
 test/replication/lua/rlimit.lua             |  2 +-
 test/replication/master.lua                 |  2 +-
 test/replication/master_quorum.lua          |  4 +--
 test/replication/on_replace.lua             |  6 ++--
 test/replication/replica.lua                |  4 +--
 test/replication/replica_on_schema_init.lua |  4 +--
 test/replication/replica_quorum.lua         |  4 +--
 9 files changed, 37 insertions(+), 30 deletions(-)

diff --git a/.luacheckrc b/.luacheckrc
index b427f6475..c8c54776e 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -36,7 +36,7 @@ exclude_files = {
     "test/engine/**/*.lua",
     "test/engine_long/**/*.lua",
     "test/luajit-tap/**/*.lua",
-    "test/replication/**/*.lua",
+    "test/replication/*.test.lua",
     "test/replication-py/**/*.lua",
     "test/sql/**/*.lua",
     "test/swim/**/*.lua",
@@ -124,3 +124,9 @@ files["test/box-tap/extended_error.test.lua"] = {
         "forbidden_function",
     },
 }
+files["test/replication/replica_quorum.lua"] = {
+    globals = {
+        "INSTANCE_URI",
+        "nonexistent_uri",
+    },
+}
diff --git a/test/replication/lua/fast_replica.lua b/test/replication/lua/fast_replica.lua
index 8c772c41f..1e8603a3c 100644
--- a/test/replication/lua/fast_replica.lua
+++ b/test/replication/lua/fast_replica.lua
@@ -1,5 +1,5 @@
 
-function join(inspector, n)
+local function join(inspector, n)
     local path = os.getenv('TARANTOOL_SRC_DIR')
     for i=1,n do
         local rid = tostring(i)
@@ -7,13 +7,14 @@ function join(inspector, n)
         os.execute('cp '..path..'/test/replication/replica.lua ./tmp/replica'..rid..'.lua')
         os.execute('chmod +x ./tmp/replica'..rid..'.lua')
         local out_dir = box.cfg.wal_dir
-        inspector:cmd("create server replica"..rid.." with rpl_master=default, script='"..out_dir.."/../tmp/replica"..rid..".lua'")
+        inspector:cmd("create server replica"..rid.." with rpl_master=default, script='"
+			..out_dir.."/../tmp/replica"..rid..".lua'")
         inspector:cmd("start server replica"..rid)
     end
 end
 
 
-function call_all(callback)
+local function call_all(callback)
     local all = box.space._cluster:select{}
     for _, tuple in pairs(all) do
         local id = tuple[1]
@@ -23,54 +24,54 @@ function call_all(callback)
     end
 end
 
-function unregister(inspector, id)
+local function unregister(inspector, id) -- luacheck: no unused args
     box.space._cluster:delete{id}
 end
 
-function start(inspector, id)
+local function start(inspector, id)
     inspector:cmd('start server replica'..tostring(id - 1))
 end
 
-function stop(inspector, id)
+local function stop(inspector, id)
     inspector:cmd('stop server replica'..tostring(id - 1))
 end
 
-function wait(inspector, id)
+local function wait(inspector, id)
     inspector:wait_lsn('replica'..tostring(id - 1), 'default')
 end
 
-function delete(inspector, id)
+local function delete(inspector, id)
     inspector:cmd('stop server replica'..tostring(id - 1))
     inspector:cmd('delete server replica'..tostring(id - 1))
 end
 
-function drop(inspector, id)
+local function drop(inspector, id)
     unregister(inspector, id)
     delete(inspector, id)
 end
 
-function start_all(inspector)
+local function start_all(inspector)
     call_all(function (id) start(inspector, id) end)
 end
 
-function stop_all(inspector)
+local function stop_all(inspector)
     call_all(function (id) stop(inspector, id) end)
 end
 
-function wait_all(inspector)
+local function wait_all(inspector)
     call_all(function (id) wait(inspector, id) end)
 end
 
-function drop_all(inspector)
+local function drop_all(inspector)
     call_all(function (id) drop(inspector, id) end)
 end
 
-function vclock_diff(left, right)
+local function vclock_diff(left, right)
     local diff = 0
-    for id, lsn in ipairs(left) do
+    for id, lsn in ipairs(left) do -- luacheck: no unused
         diff = diff + (right[id] or 0) - left[id]
     end
-    for id, lsn in ipairs(right) do
+    for id, lsn in ipairs(right) do -- luacheck: no unused
         if left[id] == nil then
             diff = diff + right[id]
         end
diff --git a/test/replication/lua/rlimit.lua b/test/replication/lua/rlimit.lua
index de9f86a35..1bd9e333f 100644
--- a/test/replication/lua/rlimit.lua
+++ b/test/replication/lua/rlimit.lua
@@ -1,5 +1,5 @@
 
-ffi = require('ffi')
+local ffi = require('ffi')
 pcall(ffi.cdef, [[
 typedef long rlim_t;
 struct rlimit {
diff --git a/test/replication/master.lua b/test/replication/master.lua
index e924b5495..b43bafd54 100644
--- a/test/replication/master.lua
+++ b/test/replication/master.lua
@@ -1,5 +1,5 @@
 #!/usr/bin/env tarantool
-os = require('os')
+local os = require('os')
 box.cfg({
     listen              = os.getenv("LISTEN"),
     memtx_memory        = 107374182,
diff --git a/test/replication/master_quorum.lua b/test/replication/master_quorum.lua
index 20f161cc0..8b7ddc5eb 100644
--- a/test/replication/master_quorum.lua
+++ b/test/replication/master_quorum.lua
@@ -26,8 +26,8 @@ box.cfg({
     replication_timeout = TIMEOUT;
 })
 
-test_run = require('test_run').new()
-engine = test_run:get_cfg('engine')
+local test_run = require('test_run').new()
+local engine = test_run:get_cfg('engine')
 
 box.once("bootstrap", function()
     box.schema.user.grant("guest", 'replication')
diff --git a/test/replication/on_replace.lua b/test/replication/on_replace.lua
index 71e63d3f9..df65832c4 100644
--- a/test/replication/on_replace.lua
+++ b/test/replication/on_replace.lua
@@ -15,9 +15,9 @@ end
 
 -- start console first
 require('console').listen(os.getenv('ADMIN'))
-env = require('test_run')
-test_run = env.new()
-engine = test_run:get_cfg('engine')
+local env = require('test_run')
+local test_run = env.new()
+local engine = test_run:get_cfg('engine')
 
 box.cfg({
     listen = instance_uri(INSTANCE_ID);
diff --git a/test/replication/replica.lua b/test/replication/replica.lua
index 76949a568..d53671735 100644
--- a/test/replication/replica.lua
+++ b/test/replication/replica.lua
@@ -1,7 +1,7 @@
 #!/usr/bin/env tarantool
 
-repl_include_self = arg[1] and arg[1] == 'true' or false
-repl_list = nil
+local repl_include_self = arg[1] and arg[1] == 'true' or false
+local repl_list
 
 if repl_include_self then
     repl_list = {os.getenv("MASTER"), os.getenv("LISTEN")}
diff --git a/test/replication/replica_on_schema_init.lua b/test/replication/replica_on_schema_init.lua
index 8a221681b..eb5a5c992 100644
--- a/test/replication/replica_on_schema_init.lua
+++ b/test/replication/replica_on_schema_init.lua
@@ -1,12 +1,12 @@
 #!/usr/bin/env tarantool
 
-function trig_local(old, new)
+local function trig_local(old, new) -- luacheck: no unused args
     if new and new[3] == 'test_local' and new[6]['group_id'] ~= 1 then
         return new:update{{'=', 6, {group_id = 1}}}
     end
 end
 
-function trig_engine(old, new)
+local function trig_engine(old, new) -- luacheck: no unused args
     if new and new[3] == 'test_engine' and new[4] ~= 'vinyl' then
         return new:update{{'=', 4, 'vinyl'}}
     end
diff --git a/test/replication/replica_quorum.lua b/test/replication/replica_quorum.lua
index 8dfad8b6e..80886e1c4 100644
--- a/test/replication/replica_quorum.lua
+++ b/test/replication/replica_quorum.lua
@@ -5,9 +5,9 @@ local SOCKET_DIR = require('fio').cwd()
 local QUORUM = tonumber(arg[1])
 local TIMEOUT = arg[2] and tonumber(arg[2]) or 0.1
 local CONNECT_TIMEOUT = arg[3] and tonumber(arg[3]) or 10
-INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock'
+INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock' -- luacheck: ignore
 
-function nonexistent_uri(id)
+function nonexistent_uri(id) -- luacheck: ignore
     return SOCKET_DIR .. '/replica_quorum' .. (1000 + id) .. '.sock'
 end
 
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v7 1/2] test: fix luacheck warnings in test/replication

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the patch!

> diff --git a/test/replication/replica_quorum.lua b/test/replication/replica_quorum.lua
> index 8dfad8b6e..80886e1c4 100644
> --- a/test/replication/replica_quorum.lua
> +++ b/test/replication/replica_quorum.lua
> @@ -5,9 +5,9 @@ local SOCKET_DIR = require('fio').cwd()
>  local QUORUM = tonumber(arg[1])
>  local TIMEOUT = arg[2] and tonumber(arg[2]) or 0.1
>  local CONNECT_TIMEOUT = arg[3] and tonumber(arg[3]) or 10
> -INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock'
> +INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock' -- luacheck: ignore
>  
> -function nonexistent_uri(id)
> +function nonexistent_uri(id) -- luacheck: ignore
>      return SOCKET_DIR .. '/replica_quorum' .. (1000 + id) .. '.sock'
>  end

I reverted this file entirely, and the check passed.

Re: [Tarantool-patches] [PATCH v7 1/2] test: fix luacheck warnings in test/replication

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 28.01.2021 01:24, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
>> diff --git a/test/replication/replica_quorum.lua b/test/replication/replica_quorum.lua
>> index 8dfad8b6e..80886e1c4 100644
>> --- a/test/replication/replica_quorum.lua
>> +++ b/test/replication/replica_quorum.lua
>> @@ -5,9 +5,9 @@ local SOCKET_DIR = require('fio').cwd()
>>   local QUORUM = tonumber(arg[1])
>>   local TIMEOUT = arg[2] and tonumber(arg[2]) or 0.1
>>   local CONNECT_TIMEOUT = arg[3] and tonumber(arg[3]) or 10
>> -INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock'
>> +INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock' -- luacheck: ignore
>>   
>> -function nonexistent_uri(id)
>> +function nonexistent_uri(id) -- luacheck: ignore
>>       return SOCKET_DIR .. '/replica_quorum' .. (1000 + id) .. '.sock'
>>   end
> I reverted this file entirely, and the check passed.

Reverted changes and force-pushed as we already have suppressions for 
INSTANCE_URI and nonexistent_uri in .luacheckrc.

--- a/test/replication/replica_quorum.lua
+++ b/test/replication/replica_quorum.lua
@@ -5,9 +5,9 @@ local SOCKET_DIR = require('fio').cwd()
  local QUORUM = tonumber(arg[1])
  local TIMEOUT = arg[2] and tonumber(arg[2]) or 0.1
  local CONNECT_TIMEOUT = arg[3] and tonumber(arg[3]) or 10
-INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock' -- luacheck: ignore
+INSTANCE_URI = SOCKET_DIR .. '/replica_quorum.sock'

-function nonexistent_uri(id) -- luacheck: ignore
+function nonexistent_uri(id)
      return SOCKET_DIR .. '/replica_quorum' .. (1000 + id) .. '.sock'
  end

Re: [Tarantool-patches] [PATCH v7 1/2] test: fix luacheck warnings in test/replication

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the fixes!

Here is the diff --stat of the commit:

====================
.luacheckrc                                 |   8 +++++++-
 .luacheckrc_                                | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 test/replication/lua/fast_replica.lua       |  33 +++++++++++++++++----------------
 test/replication/lua/rlimit.lua             |   2 +-
 test/replication/master.lua                 |   2 +-
 test/replication/master_quorum.lua          |   4 ++--
 test/replication/on_replace.lua             |   6 +++---
 test/replication/replica.lua                |   4 ++--
 test/replication/replica_on_schema_init.lua |   4 ++--
 9 files changed, 162 insertions(+), 28 deletions(-)
====================

What is the second luacheck file?

Re: [Tarantool-patches] [PATCH v7 1/2] test: fix luacheck warnings in test/replication

[Sergey Bronnikov via Tarantool-patches]

Thanks for review!

On 30.01.2021 22:01, Vladislav Shpilevoy wrote:
> Hi! Thanks for the fixes!
>
> Here is the diff --stat of the commit:
>
> ====================
> .luacheckrc                                 |   8 +++++++-
>   .luacheckrc_                                | 127 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   test/replication/lua/fast_replica.lua       |  33 +++++++++++++++++----------------
>   test/replication/lua/rlimit.lua             |   2 +-
>   test/replication/master.lua                 |   2 +-
>   test/replication/master_quorum.lua          |   4 ++--
>   test/replication/on_replace.lua             |   6 +++---
>   test/replication/replica.lua                |   4 ++--
>   test/replication/replica_on_schema_init.lua |   4 ++--
>   9 files changed, 162 insertions(+), 28 deletions(-)
> ====================
>
> What is the second luacheck file?
Accidentally committed. Removed and force-pushed.

[Tarantool-patches] [PATCH v7 2/2] test: fix luacheck warnings in test/replication-py

[Sergey Bronnikov via Tarantool-patches]

From: Sergey Bronnikov <sergeyb@tarantool.org>

Closes #5462

Reviewed-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Reviewed-by: Igor Munkin <imun@tarantool.org>

Co-authored-by: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Co-authored-by: Igor Munkin <imun@tarantool.org>
---
 .luacheckrc                     | 1 -
 test/replication-py/master.lua  | 2 +-
 test/replication-py/panic.lua   | 2 +-
 test/replication-py/replica.lua | 4 ----
 4 files changed, 2 insertions(+), 7 deletions(-)

diff --git a/.luacheckrc b/.luacheckrc
index c8c54776e..47770defb 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -37,7 +37,6 @@ exclude_files = {
     "test/engine_long/**/*.lua",
     "test/luajit-tap/**/*.lua",
     "test/replication/*.test.lua",
-    "test/replication-py/**/*.lua",
     "test/sql/**/*.lua",
     "test/swim/**/*.lua",
     "test/var/**/*.lua",
diff --git a/test/replication-py/master.lua b/test/replication-py/master.lua
index e924b5495..b43bafd54 100644
--- a/test/replication-py/master.lua
+++ b/test/replication-py/master.lua
@@ -1,5 +1,5 @@
 #!/usr/bin/env tarantool
-os = require('os')
+local os = require('os')
 box.cfg({
     listen              = os.getenv("LISTEN"),
     memtx_memory        = 107374182,
diff --git a/test/replication-py/panic.lua b/test/replication-py/panic.lua
index 75a738cbb..e72d11419 100644
--- a/test/replication-py/panic.lua
+++ b/test/replication-py/panic.lua
@@ -1,5 +1,5 @@
 #!/usr/bin/env tarantool
-os = require('os')
+local os = require('os')
 box.cfg({
     listen              = os.getenv("LISTEN"),
     memtx_memory        = 107374182,
diff --git a/test/replication-py/replica.lua b/test/replication-py/replica.lua
index 32d888eff..a5fa292d2 100644
--- a/test/replication-py/replica.lua
+++ b/test/replication-py/replica.lua
@@ -1,6 +1,4 @@
 #!/usr/bin/env tarantool
-box_cfg_done = false
-
 require('console').listen(os.getenv('ADMIN'))
 
 box.cfg({
@@ -9,5 +7,3 @@ box.cfg({
     memtx_memory        = 107374182,
     replication_timeout = 0.1
 })
-
-box_cfg_done = true
-- 
2.25.1

Re: [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/replication and test/replication-py

[Vladislav Shpilevoy via Tarantool-patches]

Hi! Thanks for the patchset!

LGTM.

Re: [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/replication and test/replication-py

[Kirill Yukhin via Tarantool-patches]

Hello,

On 21 янв 17:47, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Changelog v7:
> 
> - rebased to master branch
> 
> Changelog v6:
> 
> - splitted patch in test/ for patches per sub-directory
> - adjusted suppressions in .luacheckrc
> - fixed formatting issues in .luacheckrc
> 
> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/244921393
> Branch: ligurio/gh-5461-luacheck-warnings-test-replication
> Issues:
>   - https://github.com/tarantool/tarantool/issues/5461
>   - https://github.com/tarantool/tarantool/issues/5462
> 
> Sergey Bronnikov (2):
>   test: fix luacheck warnings in test/replication
>   test: fix luacheck warnings in test/replication-py

I've checked your patch into 2.6, 2.7 and master.

--
Regards, Kirill Yukhin