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

[Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for review!

On 16.01.2021 01:10, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
>
>> diff --git a/test/sql-tap/with2.test.lua b/test/sql-tap/with2.test.lua
>> index 456ded363..cdf1d75a2 100755
>> --- a/test/sql-tap/with2.test.lua
>> +++ b/test/sql-tap/with2.test.lua
>> @@ -386,22 +386,22 @@ genstmt(255), {
>>   -- Check that adding a WITH clause to an INSERT disables the xfer
>>   -- optimization.
>>   
>> -local function do_xfer_test(test, test_func, test_name, func, exp, opts)
>> -    local opts = opts or {}
>> +local function do_xfer_test(test_arg, test_func, test_name, func, exp, opts)
>> +    opts = opts or {}
>>       local exp_xfer_count = opts.exp_xfer_count
>>       local before = box.stat.sql().sql_xfer_count
>> -    test_func(test, test_name, func, exp)
>> +    test_func(test_arg, test_name, func, exp)
>>       local after = box.stat.sql().sql_xfer_count
>>       test:is(after - before, exp_xfer_count,
>>                      test_name .. '-xfer-count')
>>   end
>>   
>> -test.do_execsql_xfer_test = function(test, test_name, func, exp, opts)
>> -    do_xfer_test(test, test.do_execsql_test, test_name, func, exp, opts)
>> +test.do_execsql_xfer_test = function(test_arg, test_name, func, exp, opts)
>> +    do_xfer_test(test_arg, test_arg.do_execsql_test, test_name, func, exp, opts)
>>   end
>>   
>> -test.do_catchsql_xfer_test = function(test, test_name, func, exp, opts)
>> -    do_xfer_test(test, test.do_catchsql_test, test_name, func, exp, opts)
>> +test.do_catchsql_xfer_test = function(test_arg, test_name, func, exp, opts)
>> +    do_xfer_test(test_arg, test_arg.do_catchsql_test, test_name, func, exp, opts)
>>   end
>>   
>>   test:do_execsql_test(
> I reverted this entire diff hunk and luacheck passed.
> Indeed, in this hunk you only fix upvalue redefinitions,
> which we have agreed to leave as is, and you even added
> ignores to .luacheckrc to the list of global ignores:
>
>      -- Redefining a local variable.
>      "411",
>      -- Redefining an argument.
>      "412",
>      -- Shadowing a local variable.
>      "421",
>      -- Shadowing an upvalue.
>      "431",
>      -- Shadowing an upvalue argument.
>      "432",
>
> Why did you change this file?
> Also I did this and the check also has passed:
>
> diff --git a/test/sql-tap/analyze9.test.lua b/test/sql-tap/analyze9.test.lua
> index 50300cdbe..b8955531a 100755
> --- a/test/sql-tap/analyze9.test.lua
> +++ b/test/sql-tap/analyze9.test.lua
> @@ -244,7 +244,7 @@ test:do_execsql_test(
>   local function insert_filler_rows_n(iStart, nCopy, nVal)
>       for i = 0, nVal-1 do
>           local iVal = iStart+i
> -        for _ = 0, nCopy-1 do
> +        for j = 0, nCopy-1 do
>               box.execute(string.format("INSERT INTO t1 VALUES (null, %s, %s, '%s')", iVal, iVal, iVal))
>           end
>       end
>
> I reverted one of your first changes in this commit.
> The check passes because it looks like you left a ton of
> ignores in files["test/sql-tap/**/*.lua"] in .luacheckrc.
>
> These comments make me think that you checked the files
> without using .luacheckrc config or I don't know.
>
> I suggest you to re-make the patch with these ignores
> and excess diff removed.

sent updated series [1].

1. 
https://lists.tarantool.org/tarantool-patches/cover.1611232655.git.sergeyb@tarantool.org/T/#t