Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
@ 2021-01-13 14:35 Sergey Bronnikov via Tarantool-patches
  2021-01-13 17:04 ` Vladislav Shpilevoy via Tarantool-patches
                   ` (2 more replies)
  0 siblings, 3 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2021-01-13 14:35 UTC (permalink / raw)
  To: tarantool-patches, v.shpilevoy

From: Sergey Bronnikov <sergeyb@tarantool.org>

Closes #5460

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

Changelog v7:

- updated an exclusion mask in .luacheckrc

Changelog v6:

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

Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/241108315
Issue: https://github.com/tarantool/tarantool/issues/5460
Branch: ligurio/gh-5460-luacheck-warnings-test-long_run-py

 .luacheckrc                         |  2 +-
 test/long_run-py/lua/finalizers.lua |  8 +++-----
 test/long_run-py/suite.lua          | 16 +++++++++-------
 3 files changed, 13 insertions(+), 13 deletions(-)

diff --git a/.luacheckrc b/.luacheckrc
index 4b829f3dc..68736d8db 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -37,7 +37,7 @@ exclude_files = {
     "test/box-tap/**/*.lua",
     "test/engine/**/*.lua",
     "test/engine_long/**/*.lua",
-    "test/long_run-py/**/*.lua",
+    "test/long_run-py/lua/finalizers.lua",
     "test/luajit-tap/**/*.lua",
     "test/replication/**/*.lua",
     "test/replication-py/**/*.lua",
diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
index 69146a323..cb6400363 100644
--- a/test/long_run-py/lua/finalizers.lua
+++ b/test/long_run-py/lua/finalizers.lua
@@ -1,19 +1,17 @@
 #!/usr/bin/env tarantool
 
-function on_gc(t)
+local function on_gc()
 end;
 
-function test_finalizers()
+local function test_finalizers()
     local result = {}
     local i = 1
     local ffi = require('ffi')
     while true do
-        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
+        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
         i = i + 1
     end
-    return "done"
 end;
 
 test_finalizers()
 test_finalizers()
-
diff --git a/test/long_run-py/suite.lua b/test/long_run-py/suite.lua
index 0b33dec7d..7a09dd2b8 100644
--- a/test/long_run-py/suite.lua
+++ b/test/long_run-py/suite.lua
@@ -1,17 +1,16 @@
 
-function string_function()
+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
     return random_string
 end
 
-function delete_replace_update(engine_name)
-    local string_value
+local function delete_replace_update(engine_name)
     if (box.space._space.index.name:select{'tester'}[1] ~= nil) then
         box.space.tester:drop()
     end
@@ -41,7 +40,6 @@ function delete_replace_update(engine_name)
         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
@@ -71,8 +69,7 @@ function delete_replace_update(engine_name)
     return {counter, random_number, string_value_2, string_value_3}
 end
 
-function delete_insert(engine_name)
-    local string_value
+local function delete_insert(engine_name)
     if (box.space._space.index.name:select{'tester'}[1] ~= nil) then
         box.space.tester:drop()
     end
@@ -109,3 +106,8 @@ function delete_insert(engine_name)
     box.space.tester:drop()
     return {counter, string_value_2}
 end
+
+return {
+    delete_replace_update = delete_replace_update;
+    delete_insert = delete_insert;
+}
-- 
2.25.1


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-13 14:35 [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py Sergey Bronnikov via Tarantool-patches
@ 2021-01-13 17:04 ` Vladislav Shpilevoy via Tarantool-patches
  2021-01-14  8:13   ` Sergey Bronnikov via Tarantool-patches
  2021-01-15 22:20 ` Vladislav Shpilevoy via Tarantool-patches
  2021-01-18 13:43 ` Kirill Yukhin via Tarantool-patches
  2 siblings, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-01-13 17:04 UTC (permalink / raw)
  To: sergeyb, tarantool-patches

Thanks for the patch!

See 3 comments below.

On 13.01.2021 15:35, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Closes #5460
> 
> 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>
> ---
> 
> Changelog v7:
> 
> - updated an exclusion mask in .luacheckrc
> 
> Changelog v6:
> 
> - splitted patch in test/ for patches per sub-directory
> - adjusted supressions in .luacheckrc
> - fixed formatting issues in .luacheckrc
> 
> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/241108315
> Issue: https://github.com/tarantool/tarantool/issues/5460
> Branch: ligurio/gh-5460-luacheck-warnings-test-long_run-py
> 
>  .luacheckrc                         |  2 +-
>  test/long_run-py/lua/finalizers.lua |  8 +++-----
>  test/long_run-py/suite.lua          | 16 +++++++++-------
>  3 files changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/.luacheckrc b/.luacheckrc
> index 4b829f3dc..68736d8db 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -37,7 +37,7 @@ exclude_files = {
>      "test/box-tap/**/*.lua",
>      "test/engine/**/*.lua",
>      "test/engine_long/**/*.lua",
> -    "test/long_run-py/**/*.lua",
> +    "test/long_run-py/lua/finalizers.lua",

1. Why did you leave it in 'exclude' and yet you fix it?

I tried to remove it, and found that it says your change
in the next hunk is not correct. When I fixed it, I saw it
says the 'result' variable is not used but only mutated.

Then I tried to 'read' result somehow after the loop, but
it says the code is unreachable. Then I made the loop look
like it can end, and now it seems to be working.

====================
diff --git a/.luacheckrc b/.luacheckrc
index 68736d8db..3f4913df3 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -37,7 +37,6 @@ exclude_files = {
     "test/box-tap/**/*.lua",
     "test/engine/**/*.lua",
     "test/engine_long/**/*.lua",
-    "test/long_run-py/lua/finalizers.lua",
     "test/luajit-tap/**/*.lua",
     "test/replication/**/*.lua",
     "test/replication-py/**/*.lua",
diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
index cb6400363..88bd7734c 100644
--- a/test/long_run-py/lua/finalizers.lua
+++ b/test/long_run-py/lua/finalizers.lua
@@ -7,10 +7,14 @@ local function test_finalizers()
     local result = {}
     local i = 1
     local ffi = require('ffi')
-    while true do
-        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
+    while i ~= 0 do
+        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
         i = i + 1
     end
+    -- Fake-read 'result' to calm down 'luacheck' complaining that the variable
+    -- is never used.
+    assert(#result ~= 0)
+    return "done"
 end;
 
 test_finalizers()
====================

It looks a bit ugly maybe, because i ~= 0 is always true, and because
there is unreachable code after the loop.

I suggest to either find a better way to fix the warning; or don't change
finalizers.lua at all and keep the whole file ignored; or apply my diff,
or revert your 'local' usage in the next diff hunk, and silence this single
warning about 'result' being not used.

>      "test/luajit-tap/**/*.lua",
>      "test/replication/**/*.lua",
>      "test/replication-py/**/*.lua",
> diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
> index 69146a323..cb6400363 100644
> --- a/test/long_run-py/lua/finalizers.lua
> +++ b/test/long_run-py/lua/finalizers.lua
> @@ -1,19 +1,17 @@
>  #!/usr/bin/env tarantool
>  
> -function on_gc(t)
> +local function on_gc()
>  end;
>  
> -function test_finalizers()
> +local function test_finalizers()
>      local result = {}
>      local i = 1
>      local ffi = require('ffi')
>      while true do
> -        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
> +        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)

2. This change is not correct. Even luacheck tells it, if you don't
ignore this file. You assign a value to a table member, not
declare a variable.

>          i = i + 1
>      end
> -    return "done"
>  end;
>  
>  test_finalizers()
>  test_finalizers()
> -
> diff --git a/test/long_run-py/suite.lua b/test/long_run-py/suite.lua
> index 0b33dec7d..7a09dd2b8 100644
> --- a/test/long_run-py/suite.lua
> +++ b/test/long_run-py/suite.lua
> @@ -109,3 +106,8 @@ function delete_insert(engine_name)
>      box.space.tester:drop()
>      return {counter, string_value_2}
>  end
> +
> +return {
> +    delete_replace_update = delete_replace_update;
> +    delete_insert = delete_insert;

3. Please, use ',' instead of ';'.

> +}
> 

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-13 17:04 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-01-14  8:13   ` Sergey Bronnikov via Tarantool-patches
  2021-01-14  8:24     ` Sergey Bronnikov via Tarantool-patches
  2021-01-14 21:46     ` Vladislav Shpilevoy via Tarantool-patches
  0 siblings, 2 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2021-01-14  8:13 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Thanks for review!

On 13.01.2021 20:04, Vladislav Shpilevoy wrote:
> Thanks for the patch!
>
> See 3 comments below.
>
> On 13.01.2021 15:35, sergeyb@tarantool.org wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> Closes #5460
>>
>> 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>
>> ---
>>
>> Changelog v7:
>>
>> - updated an exclusion mask in .luacheckrc
>>
>> Changelog v6:
>>
>> - splitted patch in test/ for patches per sub-directory
>> - adjusted supressions in .luacheckrc
>> - fixed formatting issues in .luacheckrc
>>
>> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/241108315
>> Issue: https://github.com/tarantool/tarantool/issues/5460
>> Branch: ligurio/gh-5460-luacheck-warnings-test-long_run-py
>>
>>   .luacheckrc                         |  2 +-
>>   test/long_run-py/lua/finalizers.lua |  8 +++-----
>>   test/long_run-py/suite.lua          | 16 +++++++++-------
>>   3 files changed, 13 insertions(+), 13 deletions(-)
>>
>> diff --git a/.luacheckrc b/.luacheckrc
>> index 4b829f3dc..68736d8db 100644
>> --- a/.luacheckrc
>> +++ b/.luacheckrc
>> @@ -37,7 +37,7 @@ exclude_files = {
>>       "test/box-tap/**/*.lua",
>>       "test/engine/**/*.lua",
>>       "test/engine_long/**/*.lua",
>> -    "test/long_run-py/**/*.lua",
>> +    "test/long_run-py/lua/finalizers.lua",
> 1. Why did you leave it in 'exclude' and yet you fix it?

Change in a file was done to fix luacheck warning, but I didn't know how 
to fix

other warnings so decided to exclude it at all.

> I tried to remove it, and found that it says your change
> in the next hunk is not correct. When I fixed it, I saw it
> says the 'result' variable is not used but only mutated.
>
> Then I tried to 'read' result somehow after the loop, but
> it says the code is unreachable. Then I made the loop look
> like it can end, and now it seems to be working.
>
> ====================
> diff --git a/.luacheckrc b/.luacheckrc
> index 68736d8db..3f4913df3 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -37,7 +37,6 @@ exclude_files = {
>       "test/box-tap/**/*.lua",
>       "test/engine/**/*.lua",
>       "test/engine_long/**/*.lua",
> -    "test/long_run-py/lua/finalizers.lua",
>       "test/luajit-tap/**/*.lua",
>       "test/replication/**/*.lua",
>       "test/replication-py/**/*.lua",
> diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
> index cb6400363..88bd7734c 100644
> --- a/test/long_run-py/lua/finalizers.lua
> +++ b/test/long_run-py/lua/finalizers.lua
> @@ -7,10 +7,14 @@ local function test_finalizers()
>       local result = {}
>       local i = 1
>       local ffi = require('ffi')
> -    while true do
> -        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
> +    while i ~= 0 do
> +        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
>           i = i + 1
>       end
> +    -- Fake-read 'result' to calm down 'luacheck' complaining that the variable
> +    -- is never used.
> +    assert(#result ~= 0)
> +    return "done"
>   end;
>   
>   test_finalizers()
> ====================
>
> It looks a bit ugly maybe, because i ~= 0 is always true, and because
> there is unreachable code after the loop.
>
> I suggest to either find a better way to fix the warning; or don't change
> finalizers.lua at all and keep the whole file ignored; or apply my diff,
> or revert your 'local' usage in the next diff hunk, and silence this single
> warning about 'result' being not used.

Agree, it's looks a bit ugly, although I think it is better than keep 
suppression in .luacheckrc for the whole file.

So I applied your change and squashed to commit. Thanks.

>>       "test/luajit-tap/**/*.lua",
>>       "test/replication/**/*.lua",
>>       "test/replication-py/**/*.lua",
>> diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
>> index 69146a323..cb6400363 100644
>> --- a/test/long_run-py/lua/finalizers.lua
>> +++ b/test/long_run-py/lua/finalizers.lua
>> @@ -1,19 +1,17 @@
>>   #!/usr/bin/env tarantool
>>   
>> -function on_gc(t)
>> +local function on_gc()
>>   end;
>>   
>> -function test_finalizers()
>> +local function test_finalizers()
>>       local result = {}
>>       local i = 1
>>       local ffi = require('ffi')
>>       while true do
>> -        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
>> +        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
> 2. This change is not correct. Even luacheck tells it, if you don't
> ignore this file. You assign a value to a table member, not
> declare a variable.

It is still not unclear for me why luacheck complains here.

result table declared before a loop and scope for it is a whole function 
body, what's wrong?

Similar lua chunk successfully executed by puc lua:

local result = {}
local i = 0
while i == 0 do
     result[i] = 1
end

>
>>           i = i + 1
>>       end
>> -    return "done"
>>   end;
>>   
>>   test_finalizers()
>>   test_finalizers()
>> -
>> diff --git a/test/long_run-py/suite.lua b/test/long_run-py/suite.lua
>> index 0b33dec7d..7a09dd2b8 100644
>> --- a/test/long_run-py/suite.lua
>> +++ b/test/long_run-py/suite.lua
>> @@ -109,3 +106,8 @@ function delete_insert(engine_name)
>>       box.space.tester:drop()
>>       return {counter, string_value_2}
>>   end
>> +
>> +return {
>> +    delete_replace_update = delete_replace_update;
>> +    delete_insert = delete_insert;
> 3. Please, use ',' instead of ';'.
>
Fixed in a branch.

--- a/test/long_run-py/suite.lua
+++ b/test/long_run-py/suite.lua
@@ -108,6 +108,6 @@ local function delete_insert(engine_name)
  end

  return {
-    delete_replace_update = delete_replace_update;
-    delete_insert = delete_insert;
+    delete_replace_update = delete_replace_update,
+    delete_insert = delete_insert,
  }

>> +}
>>

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-14  8:13   ` Sergey Bronnikov via Tarantool-patches
@ 2021-01-14  8:24     ` Sergey Bronnikov via Tarantool-patches
  2021-01-14 21:46     ` Vladislav Shpilevoy via Tarantool-patches
  1 sibling, 0 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2021-01-14  8:24 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches

Changes applied and pushed to the branch.

CI: https://gitlab.com/tarantool/tarantool/-/pipelines/241507935

On 14.01.2021 11:13, Sergey Bronnikov via Tarantool-patches wrote:
> Thanks for review!

<snipped>


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-14  8:13   ` Sergey Bronnikov via Tarantool-patches
  2021-01-14  8:24     ` Sergey Bronnikov via Tarantool-patches
@ 2021-01-14 21:46     ` Vladislav Shpilevoy via Tarantool-patches
  2021-01-15  9:47       ` Sergey Bronnikov via Tarantool-patches
  1 sibling, 1 reply; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-01-14 21:46 UTC (permalink / raw)
  To: Sergey Bronnikov, tarantool-patches

>>> diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
>>> index 69146a323..cb6400363 100644
>>> --- a/test/long_run-py/lua/finalizers.lua
>>> +++ b/test/long_run-py/lua/finalizers.lua
>>> @@ -1,19 +1,17 @@
>>>   #!/usr/bin/env tarantool
>>>   -function on_gc(t)
>>> +local function on_gc()
>>>   end;
>>>   -function test_finalizers()
>>> +local function test_finalizers()
>>>       local result = {}
>>>       local i = 1
>>>       local ffi = require('ffi')
>>>       while true do
>>> -        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
>>> +        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
>> 2. This change is not correct. Even luacheck tells it, if you don't
>> ignore this file. You assign a value to a table member, not
>> declare a variable.
> 
> It is still not unclear for me why luacheck complains here.
> 
> result table declared before a loop and scope for it is a whole function body, what's wrong?
> 
> Similar lua chunk successfully executed by puc lua:
> 
> local result = {}
> local i = 0
> while i == 0 do
>     result[i] = 1
> end

Your chunk is similar, but not the same. Originally you used 'local result[i] = ...'
expression which is obviously wrong. You can't "declare" a table member as local.
'result[i]' is not a valid variable name, so it can't be used with 'local' keyword
to declare it as a variable.

>>> diff --git a/test/long_run-py/suite.lua b/test/long_run-py/suite.lua
>>> index 0b33dec7d..7a09dd2b8 100644
>>> --- a/test/long_run-py/suite.lua
>>> +++ b/test/long_run-py/suite.lua
>>> @@ -109,3 +106,8 @@ function delete_insert(engine_name)
>>>       box.space.tester:drop()
>>>       return {counter, string_value_2}
>>>   end
>>> +
>>> +return {
>>> +    delete_replace_update = delete_replace_update;
>>> +    delete_insert = delete_insert;
>> 3. Please, use ',' instead of ';'.
>>
> Fixed in a branch.

I realized suite.lua functions are never used. The file is imported in
long_run-py/box.lua, but the imported functions are not used. I deleted
it and the tests pass. I suggest you to delete this file entirely:

====================
diff --git a/test/long_run-py/box.lua b/test/long_run-py/box.lua
index b4f65dcdb..354e680b4 100644
--- a/test/long_run-py/box.lua
+++ b/test/long_run-py/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/long_run-py/suite.ini b/test/long_run-py/suite.ini
index 110bbb548..7561fdb5a 100644
--- a/test/long_run-py/suite.ini
+++ b/test/long_run-py/suite.ini
@@ -5,7 +5,6 @@ script = box.lua
 long_run =  finalizers.test.py
 valgrind_disabled =
 release_disabled =
-lua_libs = suite.lua
 use_unix_sockets = True
 use_unix_sockets_iproto = True
 is_parallel = True

+ delete suite.lua file.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-14 21:46     ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-01-15  9:47       ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2021-01-15  9:47 UTC (permalink / raw)
  To: Vladislav Shpilevoy, tarantool-patches


On 15.01.2021 00:46, Vladislav Shpilevoy wrote:
>>>> diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua
>>>> index 69146a323..cb6400363 100644
>>>> --- a/test/long_run-py/lua/finalizers.lua
>>>> +++ b/test/long_run-py/lua/finalizers.lua
>>>> @@ -1,19 +1,17 @@
>>>>    #!/usr/bin/env tarantool
>>>>    -function on_gc(t)
>>>> +local function on_gc()
>>>>    end;
>>>>    -function test_finalizers()
>>>> +local function test_finalizers()
>>>>        local result = {}
>>>>        local i = 1
>>>>        local ffi = require('ffi')
>>>>        while true do
>>>> -        result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
>>>> +        local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc)
>>> 2. This change is not correct. Even luacheck tells it, if you don't
>>> ignore this file. You assign a value to a table member, not
>>> declare a variable.
>> It is still not unclear for me why luacheck complains here.
>>
>> result table declared before a loop and scope for it is a whole function body, what's wrong?
>>
>> Similar lua chunk successfully executed by puc lua:
>>
>> local result = {}
>> local i = 0
>> while i == 0 do
>>      result[i] = 1
>> end
> Your chunk is similar, but not the same. Originally you used 'local result[i] = ...'
> expression which is obviously wrong. You can't "declare" a table member as local.
> 'result[i]' is not a valid variable name, so it can't be used with 'local' keyword
> to declare it as a variable.
Got it, thanks.
>>>> diff --git a/test/long_run-py/suite.lua b/test/long_run-py/suite.lua
>>>> index 0b33dec7d..7a09dd2b8 100644
>>>> --- a/test/long_run-py/suite.lua
>>>> +++ b/test/long_run-py/suite.lua
>>>> @@ -109,3 +106,8 @@ function delete_insert(engine_name)
>>>>        box.space.tester:drop()
>>>>        return {counter, string_value_2}
>>>>    end
>>>> +
>>>> +return {
>>>> +    delete_replace_update = delete_replace_update;
>>>> +    delete_insert = delete_insert;
>>> 3. Please, use ',' instead of ';'.
>>>
>> Fixed in a branch.
> I realized suite.lua functions are never used. The file is imported in
> long_run-py/box.lua, but the imported functions are not used. I deleted
> it and the tests pass. I suggest you to delete this file entirely:
>
> ====================
> diff --git a/test/long_run-py/box.lua b/test/long_run-py/box.lua
> index b4f65dcdb..354e680b4 100644
> --- a/test/long_run-py/box.lua
> +++ b/test/long_run-py/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/long_run-py/suite.ini b/test/long_run-py/suite.ini
> index 110bbb548..7561fdb5a 100644
> --- a/test/long_run-py/suite.ini
> +++ b/test/long_run-py/suite.ini
> @@ -5,7 +5,6 @@ script = box.lua
>   long_run =  finalizers.test.py
>   valgrind_disabled =
>   release_disabled =
> -lua_libs = suite.lua
>   use_unix_sockets = True
>   use_unix_sockets_iproto = True
>   is_parallel = True
>
> + delete suite.lua file.


Done.

CI: https://gitlab.com/tarantool/tarantool/-/pipelines/242068204


^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-13 14:35 [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py Sergey Bronnikov via Tarantool-patches
  2021-01-13 17:04 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-01-15 22:20 ` Vladislav Shpilevoy via Tarantool-patches
  2021-01-18 13:43 ` Kirill Yukhin via Tarantool-patches
  2 siblings, 0 replies; 8+ messages in thread
From: Vladislav Shpilevoy via Tarantool-patches @ 2021-01-15 22:20 UTC (permalink / raw)
  To: sergeyb, tarantool-patches

Thanks for the patch!

LGTM.

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py
  2021-01-13 14:35 [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py Sergey Bronnikov via Tarantool-patches
  2021-01-13 17:04 ` Vladislav Shpilevoy via Tarantool-patches
  2021-01-15 22:20 ` Vladislav Shpilevoy via Tarantool-patches
@ 2021-01-18 13:43 ` Kirill Yukhin via Tarantool-patches
  2 siblings, 0 replies; 8+ messages in thread
From: Kirill Yukhin via Tarantool-patches @ 2021-01-18 13:43 UTC (permalink / raw)
  To: sergeyb; +Cc: tarantool-patches, v.shpilevoy

Hello,

On 13 янв 17:35, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> Closes #5460
> 
> 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>
> ---
> 
> Changelog v7:
> 
> - updated an exclusion mask in .luacheckrc
> 
> Changelog v6:
> 
> - splitted patch in test/ for patches per sub-directory
> - adjusted supressions in .luacheckrc
> - fixed formatting issues in .luacheckrc
> 
> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/241108315
> Issue: https://github.com/tarantool/tarantool/issues/5460
> Branch: ligurio/gh-5460-luacheck-warnings-test-long_run-py

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

--
Regards, Kirill Yukhin

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2021-01-18 13:43 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-13 14:35 [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py Sergey Bronnikov via Tarantool-patches
2021-01-13 17:04 ` Vladislav Shpilevoy via Tarantool-patches
2021-01-14  8:13   ` Sergey Bronnikov via Tarantool-patches
2021-01-14  8:24     ` Sergey Bronnikov via Tarantool-patches
2021-01-14 21:46     ` Vladislav Shpilevoy via Tarantool-patches
2021-01-15  9:47       ` Sergey Bronnikov via Tarantool-patches
2021-01-15 22:20 ` Vladislav Shpilevoy via Tarantool-patches
2021-01-18 13:43 ` 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