Re: [Tarantool-patches] [PATCH v8 04/14] test: fix luacheck warnings W211 in test/sql-tap

[Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Thanks for the patch!

See 2 comments below.

On 21.01.2021 13:49, sergeyb@tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
> 
> W211 (Unused local variable)
> 
> Part of #5464
> 
> test: fix luacheck warnings W111 in test/sql-tap
> ---
> diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua
> index 81b08e223..f29b26175 100755
> --- a/test/sql-tap/e_expr.test.lua
> +++ b/test/sql-tap/e_expr.test.lua
> @@ -1080,7 +1080,6 @@ if (0>0) then
>      local function parameter_test(tn, sql, params, result)
>          local stmt = sql_prepare_v2("db", sql, -1)
>          for _ in X(0, "X!foreach", [=[["number name",["params"]]]=]) do
> -            local nm = sql_bind_parameter_name(stmt, number)

1. You should not delete the function call. It seems it must have
had an effect on the statement object. Without this call in future it
will be harder to understand what is missing if you delete it.

>              X(480, "X!cmd", [=[["do_test",[["tn"],".name.",["number"]],[["list","set","",["nm"]]],["name"]]]=])
>              sql_bind_int(stmt, number, ((-1) * number))
>          end
> diff --git a/test/sql-tap/misc1.test.lua b/test/sql-tap/misc1.test.lua
> index 7d928bea0..b1c4f026b 100755
> --- a/test/sql-tap/misc1.test.lua
> +++ b/test/sql-tap/misc1.test.lua
> @@ -726,19 +726,8 @@ test:do_execsql_test(
>  -- MUST_WORK_TEST collate
>  if 0>0 then
>      db("collate", "numeric", "numeric_collate")
> -    local function numeric_collate(lhs, rhs)
> -        if (lhs == rhs)
> -        then
> -            return 0
> -        end
> -        return X(0, "X!expr", [=[["?:",[">",["lhs"],["rhs"]],3,["-",1]]]=])
> -    end

2. The more I look at these db + X pairs, the more it seems like an
artifact after TCL -> Lua conversion. I think originally it was
supposed that the function after 'db()' should be called, and 'X'
should execute something. Worth investigating how these tests looked
originally. Otherwise we may delete too much and not able to resurrect
the tests in future.

If these code blocks are important for understanding the test (which I
don't understand now, but anyway), maybe it would better to comment
them out instead of deleing.

Or delete the entire test cases if we are not going to try to resurrect
them, which is also fine.