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 5 comments below.

> diff --git a/test/sql-tap/e_expr.test.lua b/test/sql-tap/e_expr.test.lua
> index 37a7b7177..046ed8711 100755
> --- a/test/sql-tap/e_expr.test.lua
> +++ b/test/sql-tap/e_expr.test.lua
> @@ -1087,7 +1087,7 @@ if (0>0) then
>          local number = nil
>          local stmt = sql_prepare_v2(sql, -1)
>          for _ in X(0, "X!foreach", [=[["number name",["params"]]]=]) do
> -            local nm = sql_bind_parameter_name(stmt, number)
> +            local nm = sql_bind_parameter_name(stmt, number) -- luacheck: no unused

1. Why do you need 'nm' if it is unused? You don't need to drop the
entire line. Just delete 'nm' variable and leave the function call
without saving its result to anything. The same below for 'rc' and
'sql_finalize()'.

>              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/func.test.lua b/test/sql-tap/func.test.lua
> index 9cd517673..5dab23007 100755
> --- a/test/sql-tap/func.test.lua
> +++ b/test/sql-tap/func.test.lua
> @@ -1382,7 +1380,6 @@ test:do_execsql_test(
>      })
>  
>  db("cache", "flush")
> -V = "three"

2. 'V' is supposed to be a variable. It is used in the queries
in the surrounding code. Please, don't delete parts of the tests.
It will not make it simpler to restore the context later. The tests
in this patchset either must be deleted, or resurrected, or hacked
to keep their context and not fail luacheck. What to choose - depends
on individual tests. Here clearly we need to keep 'V'. Try to make
another pass of self-review to locate more of such test-"breaking"
changes.

> diff --git a/test/sql-tap/selectA.test.lua b/test/sql-tap/selectA.test.lua
> index a608ab093..db4d03985 100755
> --- a/test/sql-tap/selectA.test.lua
> +++ b/test/sql-tap/selectA.test.lua
> @@ -2359,9 +2357,6 @@ test:do_execsql_test(
>  if (0 > 0)
>   then
>  end
> -local function f(args)
> -    return 1
> -end

3. It seems the function is supposed to be somehow used a few
lines below. Please, keep it.

>  -- TODO stored procedures are not supported by now
>  --db("func", "f", "f")> diff --git a/test/sql-tap/sort.test.lua b/test/sql-tap/sort.test.lua
> index 1cf0d14a8..0de598969 100755
> --- a/test/sql-tap/sort.test.lua
> +++ b/test/sql-tap/sort.test.lua
> @@ -802,7 +801,6 @@ box.internal.sql_create_function("cksum", cksum)
>              "sort-14."..tn,
>              function()
>                  sql_test_control("sql_TESTCTRL_SORTER_MMAP", "db", mmap_limit)
> -                local prev = ""

4. Prev is supposed to be used in the request on the next line. It
has `$prev` expression inside.

>                  X(536, "X!cmd", [=[["db","eval"," SELECT * FROM t11 ORDER BY b ","\n         if {$b != [cksum $a]} {error \"checksum failed\"}\n         if {[string compare $b $prev] < 0} {error \"sort failed\"}\n         set prev $b\n       "]]=])
>                  return X(541, "X!cmd", [=[["set","",""]]=])
>              end, {
> diff --git a/test/sql-tap/trigger1.test.lua b/test/sql-tap/trigger1.test.lua
> index 8224ef1ec..0efdd21bc 100755
> --- a/test/sql-tap/trigger1.test.lua
> +++ b/test/sql-tap/trigger1.test.lua
> @@ -482,8 +482,6 @@ test:execsql [[
>  -- trigger works.  Then drop the trigger.  Make sure the table is
>  -- still there.
>  --
> -local view_v1 = "view v1"

5. The variable is "used" in some of the commented out tests below.

> -
>  
>  -- do_test trigger1-6.1 {
>  --   execsql {SELECT type, name FROM sql_master}