* [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long @ 2021-01-15 12:00 Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 1/2] luacheck: fix warnings in test/engine Sergey Bronnikov via Tarantool-patches ` (2 more replies) 0 siblings, 3 replies; 7+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2021-01-15 12:00 UTC (permalink / raw) To: tarantool-patches, v.shpilevoy From: Sergey Bronnikov <sergeyb@tarantool.org> Changelog v7: - rebased to a master branch (and includes fixes for test/box and test/box-tap to pass ci [1]) - fixed new warnings Changelog v6: - splitted patch in test/ for patches per sub-directory - adjusted supressions in .luacheckrc - fixed formatting issues in .luacheckrc 1. https://lists.tarantool.org/tarantool-patches/cover.1610709207.git.sergeyb@tarantool.org/T/#t Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/242152198 GH branch: ligurio/gh-5458-luacheck-warnings-test-engine GH issues: - https://github.com/tarantool/tarantool/issues/5458 - https://github.com/tarantool/tarantool/issues/5650 Sergey Bronnikov (2): luacheck: fix warnings in test/engine luacheck: fix warnings in test/engine_long .luacheckrc | 10 ++++++++-- test/engine/box.lua | 8 +------- test/engine/conflict.lua | 12 ++++++++---- test/engine/conflict.test.lua | 4 ++-- test/engine_long/suite.lua | 5 +---- 5 files changed, 20 insertions(+), 19 deletions(-) -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Tarantool-patches] [PATCH v7 1/2] luacheck: fix warnings in test/engine 2021-01-15 12:00 [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Sergey Bronnikov via Tarantool-patches @ 2021-01-15 12:00 ` Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long Sergey Bronnikov via Tarantool-patches 2021-01-21 15:36 ` [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Kirill Yukhin via Tarantool-patches 2 siblings, 0 replies; 7+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2021-01-15 12:00 UTC (permalink / raw) To: tarantool-patches, v.shpilevoy From: Sergey Bronnikov <sergeyb@tarantool.org> Closes #5458 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 | 2 +- test/engine/box.lua | 8 +------- test/engine/conflict.lua | 12 ++++++++---- test/engine/conflict.test.lua | 4 ++-- 4 files changed, 12 insertions(+), 14 deletions(-) diff --git a/.luacheckrc b/.luacheckrc index b7f9abb45..d58fd57b0 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -39,7 +39,7 @@ exclude_files = { "test/box/lua/require_mod.lua", -- Unused source file, to be dropped (gh-5169). "test/box/lua/test_init.lua", - "test/engine/**/*.lua", + "test/engine/*.test.lua", "test/engine_long/**/*.lua", "test/long_run-py/**/*.lua", "test/luajit-tap/**/*.lua", diff --git a/test/engine/box.lua b/test/engine/box.lua index e2f04cba2..8558c9ac0 100644 --- a/test/engine/box.lua +++ b/test/engine/box.lua @@ -1,11 +1,5 @@ #!/usr/bin/env tarantool -os = require('os') - -local vinyl = { - threads = 3, - range_size=1024*64, - page_size=1024, -} +local os = require('os') box.cfg{ listen = os.getenv("LISTEN"), diff --git a/test/engine/conflict.lua b/test/engine/conflict.lua index b757b81d2..1f9fe5a45 100644 --- a/test/engine/conflict.lua +++ b/test/engine/conflict.lua @@ -1,11 +1,11 @@ -function test_conflict() +local function test_conflict() local test_run = require('test_run') local inspector = test_run.new() local engine = inspector:get_cfg('engine') local s = box.schema.space.create('tester', {engine=engine}); - local i = s:create_index('test_index', {type = 'tree', parts = {1, 'string'}}); + s:create_index('test_index', {type = 'tree', parts = {1, 'string'}}); local commits = 0 local function conflict() @@ -16,10 +16,14 @@ function test_conflict() end; local fiber = require('fiber'); - local f0 = fiber.create(conflict); - local f1 = fiber.create(conflict); -- conflict + fiber.create(conflict); + fiber.create(conflict); -- conflict fiber.sleep(0); s:drop(); return commits end + +return { + test_conflict = test_conflict; +} diff --git a/test/engine/conflict.test.lua b/test/engine/conflict.test.lua index d83c03a7a..7744b41b4 100644 --- a/test/engine/conflict.test.lua +++ b/test/engine/conflict.test.lua @@ -1,4 +1,4 @@ -dofile('conflict.lua') +conflict = require('conflict') -test_conflict() +conflict.test_conflict() -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long 2021-01-15 12:00 [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 1/2] luacheck: fix warnings in test/engine Sergey Bronnikov via Tarantool-patches @ 2021-01-15 12:00 ` Sergey Bronnikov via Tarantool-patches 2021-01-17 16:22 ` Vladislav Shpilevoy via Tarantool-patches 2021-01-21 15:36 ` [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Kirill Yukhin via Tarantool-patches 2 siblings, 1 reply; 7+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2021-01-15 12:00 UTC (permalink / raw) To: tarantool-patches, v.shpilevoy From: Sergey Bronnikov <sergeyb@tarantool.org> Closes #5650 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/engine_long/suite.lua | 5 +---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/.luacheckrc b/.luacheckrc index d58fd57b0..b581136b2 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -40,7 +40,7 @@ exclude_files = { -- Unused source file, to be dropped (gh-5169). "test/box/lua/test_init.lua", "test/engine/*.test.lua", - "test/engine_long/**/*.lua", + "test/engine_long/*.test.lua", "test/long_run-py/**/*.lua", "test/luajit-tap/**/*.lua", "test/replication/**/*.lua", @@ -176,3 +176,9 @@ files["test/box-tap/extended_error.test.lua"] = { "forbidden_function", }, } +files["test/engine_long/suite.lua"] = { + globals = { + "delete_replace_update", + "delete_insert", + } +} diff --git a/test/engine_long/suite.lua b/test/engine_long/suite.lua index 9ac2bff9f..586160a1a 100644 --- a/test/engine_long/suite.lua +++ b/test/engine_long/suite.lua @@ -2,7 +2,7 @@ local function string_function() local random_number local random_string random_string = "" - for x = 1,20,1 do + for _ = 1,20,1 do random_number = math.random(65, 90) random_string = random_string .. string.char(random_number) end @@ -10,7 +10,6 @@ local function string_function() end function delete_replace_update(engine_name, iterations) - local string_value if (box.space._space.index.name:select{'tester'}[1] ~= nil) then box.space.tester:drop() end @@ -40,7 +39,6 @@ function delete_replace_update(engine_name, iterations) random_number = math.random(1,6) string_value_3 = string_function() --- print('<'..counter..'> [' .. random_number .. '] value_2: ' .. string_value_2 .. ' value_3: ' .. string_value_3) if random_number == 1 then box.space.tester:delete{string_value_2} end @@ -69,7 +67,6 @@ function delete_replace_update(engine_name, iterations) end function delete_insert(engine_name, iterations) - local string_value if (box.space._space.index.name:select{'tester'}[1] ~= nil) then box.space.tester:drop() end -- 2.25.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long Sergey Bronnikov via Tarantool-patches @ 2021-01-17 16:22 ` Vladislav Shpilevoy via Tarantool-patches 2021-01-18 8:54 ` Sergey Bronnikov via Tarantool-patches 0 siblings, 1 reply; 7+ messages in thread From: Vladislav Shpilevoy via Tarantool-patches @ 2021-01-17 16:22 UTC (permalink / raw) To: sergeyb, tarantool-patches Hi! Thanks for the patch! > diff --git a/.luacheckrc b/.luacheckrc > index d58fd57b0..b581136b2 100644 > --- a/.luacheckrc > +++ b/.luacheckrc > @@ -176,3 +176,9 @@ files["test/box-tap/extended_error.test.lua"] = { > "forbidden_function", > }, > } > +files["test/engine_long/suite.lua"] = { > + globals = { > + "delete_replace_update", > + "delete_insert", There is no reason for these functions to be global. The only reason is being able to call a function via IPROTO_CALL, which is not the case here. I proper fix should make them local and returned as a result of require(): ==================== diff --git a/.luacheckrc b/.luacheckrc index 02349331e..8c67f9d47 100644 --- a/.luacheckrc +++ b/.luacheckrc @@ -131,9 +131,3 @@ files["test/box-tap/extended_error.test.lua"] = { "forbidden_function", }, } -files["test/engine_long/suite.lua"] = { - globals = { - "delete_replace_update", - "delete_insert", - } -} diff --git a/test/engine_long/box.lua b/test/engine_long/box.lua index 28a1560d5..94ae81301 100644 --- a/test/engine_long/box.lua +++ b/test/engine_long/box.lua @@ -1,7 +1,5 @@ #!/usr/bin/env tarantool -require('suite') - os.execute("rm -rf vinyl_test") os.execute("mkdir -p vinyl_test") diff --git a/test/engine_long/delete_insert.result b/test/engine_long/delete_insert.result index b1d504271..06085ca78 100644 --- a/test/engine_long/delete_insert.result +++ b/test/engine_long/delete_insert.result @@ -1,6 +1,9 @@ test_run = require('test_run') --- ... +delete_insert = require('suite').delete_insert +--- +... inspector = test_run.new() --- ... diff --git a/test/engine_long/delete_insert.test.lua b/test/engine_long/delete_insert.test.lua index 275aaa23e..b2c4ec308 100644 --- a/test/engine_long/delete_insert.test.lua +++ b/test/engine_long/delete_insert.test.lua @@ -1,4 +1,5 @@ test_run = require('test_run') +delete_insert = require('suite').delete_insert inspector = test_run.new() engine = inspector:get_cfg('engine') iterations = 100000 diff --git a/test/engine_long/delete_replace_update.result b/test/engine_long/delete_replace_update.result index 66cb9c82c..b12e00688 100644 --- a/test/engine_long/delete_replace_update.result +++ b/test/engine_long/delete_replace_update.result @@ -1,3 +1,6 @@ +delete_replace_update = require('suite').delete_replace_update +--- +... engine_name = 'memtx' --- ... diff --git a/test/engine_long/delete_replace_update.test.lua b/test/engine_long/delete_replace_update.test.lua index 466b8f007..e6906a94b 100644 --- a/test/engine_long/delete_replace_update.test.lua +++ b/test/engine_long/delete_replace_update.test.lua @@ -1,3 +1,4 @@ +delete_replace_update = require('suite').delete_replace_update engine_name = 'memtx' iterations = 100000 diff --git a/test/engine_long/suite.lua b/test/engine_long/suite.lua index 586160a1a..995257382 100644 --- a/test/engine_long/suite.lua +++ b/test/engine_long/suite.lua @@ -9,7 +9,7 @@ local function string_function() return random_string end -function delete_replace_update(engine_name, iterations) +local function delete_replace_update(engine_name, iterations) if (box.space._space.index.name:select{'tester'}[1] ~= nil) then box.space.tester:drop() end @@ -66,7 +66,7 @@ function delete_replace_update(engine_name, iterations) return {counter, random_number, string_value_2, string_value_3} end -function delete_insert(engine_name, iterations) +local function delete_insert(engine_name, iterations) if (box.space._space.index.name:select{'tester'}[1] ~= nil) then box.space.tester:drop() end @@ -104,4 +104,7 @@ function delete_insert(engine_name, iterations) return {counter, string_value_2} end -_G.protected_globals = {'delete_replace_update', 'delete_insert'} +return { + delete_replace_update = delete_replace_update, + delete_insert = delete_insert +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long 2021-01-17 16:22 ` Vladislav Shpilevoy via Tarantool-patches @ 2021-01-18 8:54 ` Sergey Bronnikov via Tarantool-patches 2021-01-20 22:40 ` Vladislav Shpilevoy via Tarantool-patches 0 siblings, 1 reply; 7+ messages in thread From: Sergey Bronnikov via Tarantool-patches @ 2021-01-18 8:54 UTC (permalink / raw) To: Vladislav Shpilevoy, tarantool-patches Thanks for review and proposed diff! It was applied without changes from my side and force pushed. CI: https://gitlab.com/tarantool/tarantool/-/pipelines/243025033 On 17.01.2021 19:22, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > >> diff --git a/.luacheckrc b/.luacheckrc >> index d58fd57b0..b581136b2 100644 >> --- a/.luacheckrc >> +++ b/.luacheckrc >> @@ -176,3 +176,9 @@ files["test/box-tap/extended_error.test.lua"] = { >> "forbidden_function", >> }, >> } >> +files["test/engine_long/suite.lua"] = { >> + globals = { >> + "delete_replace_update", >> + "delete_insert", > There is no reason for these functions to be global. The only reason > is being able to call a function via IPROTO_CALL, which is not the > case here. > > I proper fix should make them local and returned as a result of > require(): > > ==================== > diff --git a/.luacheckrc b/.luacheckrc > index 02349331e..8c67f9d47 100644 > --- a/.luacheckrc > +++ b/.luacheckrc > @@ -131,9 +131,3 @@ files["test/box-tap/extended_error.test.lua"] = { > "forbidden_function", > }, > } > -files["test/engine_long/suite.lua"] = { > - globals = { > - "delete_replace_update", > - "delete_insert", > - } > -} > diff --git a/test/engine_long/box.lua b/test/engine_long/box.lua > index 28a1560d5..94ae81301 100644 > --- a/test/engine_long/box.lua > +++ b/test/engine_long/box.lua > @@ -1,7 +1,5 @@ > #!/usr/bin/env tarantool > > -require('suite') > - > os.execute("rm -rf vinyl_test") > os.execute("mkdir -p vinyl_test") > > diff --git a/test/engine_long/delete_insert.result b/test/engine_long/delete_insert.result > index b1d504271..06085ca78 100644 > --- a/test/engine_long/delete_insert.result > +++ b/test/engine_long/delete_insert.result > @@ -1,6 +1,9 @@ > test_run = require('test_run') > --- > ... > +delete_insert = require('suite').delete_insert > +--- > +... > inspector = test_run.new() > --- > ... > diff --git a/test/engine_long/delete_insert.test.lua b/test/engine_long/delete_insert.test.lua > index 275aaa23e..b2c4ec308 100644 > --- a/test/engine_long/delete_insert.test.lua > +++ b/test/engine_long/delete_insert.test.lua > @@ -1,4 +1,5 @@ > test_run = require('test_run') > +delete_insert = require('suite').delete_insert > inspector = test_run.new() > engine = inspector:get_cfg('engine') > iterations = 100000 > diff --git a/test/engine_long/delete_replace_update.result b/test/engine_long/delete_replace_update.result > index 66cb9c82c..b12e00688 100644 > --- a/test/engine_long/delete_replace_update.result > +++ b/test/engine_long/delete_replace_update.result > @@ -1,3 +1,6 @@ > +delete_replace_update = require('suite').delete_replace_update > +--- > +... > engine_name = 'memtx' > --- > ... > diff --git a/test/engine_long/delete_replace_update.test.lua b/test/engine_long/delete_replace_update.test.lua > index 466b8f007..e6906a94b 100644 > --- a/test/engine_long/delete_replace_update.test.lua > +++ b/test/engine_long/delete_replace_update.test.lua > @@ -1,3 +1,4 @@ > +delete_replace_update = require('suite').delete_replace_update > engine_name = 'memtx' > iterations = 100000 > > diff --git a/test/engine_long/suite.lua b/test/engine_long/suite.lua > index 586160a1a..995257382 100644 > --- a/test/engine_long/suite.lua > +++ b/test/engine_long/suite.lua > @@ -9,7 +9,7 @@ local function string_function() > return random_string > end > > -function delete_replace_update(engine_name, iterations) > +local function delete_replace_update(engine_name, iterations) > if (box.space._space.index.name:select{'tester'}[1] ~= nil) then > box.space.tester:drop() > end > @@ -66,7 +66,7 @@ function delete_replace_update(engine_name, iterations) > return {counter, random_number, string_value_2, string_value_3} > end > > -function delete_insert(engine_name, iterations) > +local function delete_insert(engine_name, iterations) > if (box.space._space.index.name:select{'tester'}[1] ~= nil) then > box.space.tester:drop() > end > @@ -104,4 +104,7 @@ function delete_insert(engine_name, iterations) > return {counter, string_value_2} > end > > -_G.protected_globals = {'delete_replace_update', 'delete_insert'} > +return { > + delete_replace_update = delete_replace_update, > + delete_insert = delete_insert > +} ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long 2021-01-18 8:54 ` Sergey Bronnikov via Tarantool-patches @ 2021-01-20 22:40 ` Vladislav Shpilevoy via Tarantool-patches 0 siblings, 0 replies; 7+ messages in thread From: Vladislav Shpilevoy via Tarantool-patches @ 2021-01-20 22:40 UTC (permalink / raw) To: Sergey Bronnikov, tarantool-patches Hi! Thanks for the fixes! The patchset LGTM. ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long 2021-01-15 12:00 [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 1/2] luacheck: fix warnings in test/engine Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long Sergey Bronnikov via Tarantool-patches @ 2021-01-21 15:36 ` Kirill Yukhin via Tarantool-patches 2 siblings, 0 replies; 7+ messages in thread From: Kirill Yukhin via Tarantool-patches @ 2021-01-21 15:36 UTC (permalink / raw) To: sergeyb; +Cc: tarantool-patches, v.shpilevoy Hello, On 15 янв 15:00, Sergey Bronnikov via Tarantool-patches wrote: > From: Sergey Bronnikov <sergeyb@tarantool.org> > > Changelog v7: > > - rebased to a master branch (and includes fixes for test/box and test/box-tap to pass ci [1]) > - fixed new warnings > > Changelog v6: > > - splitted patch in test/ for patches per sub-directory > - adjusted supressions in .luacheckrc > - fixed formatting issues in .luacheckrc > > 1. https://lists.tarantool.org/tarantool-patches/cover.1610709207.git.sergeyb@tarantool.org/T/#t > > Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/242152198 > GH branch: ligurio/gh-5458-luacheck-warnings-test-engine > GH issues: > - https://github.com/tarantool/tarantool/issues/5458 > - https://github.com/tarantool/tarantool/issues/5650 I've checked your patchset into 2.6, 2.7 and master. -- Regards, Kirill Yukhin ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2021-01-21 15:36 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-01-15 12:00 [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 1/2] luacheck: fix warnings in test/engine Sergey Bronnikov via Tarantool-patches 2021-01-15 12:00 ` [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long Sergey Bronnikov via Tarantool-patches 2021-01-17 16:22 ` Vladislav Shpilevoy via Tarantool-patches 2021-01-18 8:54 ` Sergey Bronnikov via Tarantool-patches 2021-01-20 22:40 ` Vladislav Shpilevoy via Tarantool-patches 2021-01-21 15:36 ` [Tarantool-patches] [PATCH v7 0/2] Fix luacheck warnings in test/engine and test/engine_long Kirill Yukhin via Tarantool-patches
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox