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

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Sun Jan 24 20:34:39 MSK 2021


Thanks for the patch!

See 2 comments below.

On 21.01.2021 13:49, sergeyb at tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb at 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.


More information about the Tarantool-patches mailing list