From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>,
tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long
Date: Mon, 18 Jan 2021 11:54:28 +0300 [thread overview]
Message-ID: <d4dc3f5b-b0da-901f-11b2-afd5fd6191f6@tarantool.org> (raw)
In-Reply-To: <7ab993b8-d4e6-ba30-3a11-ac1a440ed26a@tarantool.org>
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
> +}
next prev parent reply other threads:[~2021-01-18 8:54 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
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 [this message]
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
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=d4dc3f5b-b0da-901f-11b2-afd5fd6191f6@tarantool.org \
--to=tarantool-patches@dev.tarantool.org \
--cc=sergeyb@tarantool.org \
--cc=v.shpilevoy@tarantool.org \
--subject='Re: [Tarantool-patches] [PATCH v7 2/2] luacheck: fix warnings in test/engine_long' \
/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