[Tarantool-patches] [PATCH v8 03/14] test: fix luacheck warnings W113 in test/sql-tap
Vladislav Shpilevoy
v.shpilevoy at tarantool.org
Sat Feb 6 20:52:00 MSK 2021
Hi! Did you see this response and some of the others?
On 24.01.2021 18:33, Vladislav Shpilevoy via Tarantool-patches wrote:
> Hi! Thanks for the patch!
>
> See 3 comments below.
>
> On 21.01.2021 13:49, sergeyb at tarantool.org wrote:
>> From: Sergey Bronnikov <sergeyb at tarantool.org>
>>
>> W113 (Accessing an undefined global variable)
>>
>> Part of #5464
>> ---
>> .luacheckrc | 37 +++++++++-
>> test/sql-tap/badutf1.test.lua | 10 ++-
>> test/sql-tap/index1.test.lua | 2 +-
>> test/sql-tap/lua/sqltester.lua | 2 +-
>> test/sql-tap/lua_sql.test.lua | 6 +-
>> test/sql-tap/minmax2.test.lua | 2 +
>> test/sql-tap/select1.test.lua | 4 ++
>> test/sql-tap/subquery.test.lua | 7 +-
>> test/sql-tap/table.test.lua | 6 +-
>> test/sql-tap/tkt1443.test.lua | 2 +-
>> test/sql-tap/trigger9.test.lua | 1 +
>> test/sql-tap/triggerC.test.lua | 1 -
>> test/sql-tap/view.test.lua | 126 +++++++++++++++------------------
>> 13 files changed, 122 insertions(+), 84 deletions(-)
>>
>> diff --git a/.luacheckrc b/.luacheckrc
>> index 81aa61a2b..eb709d6a3 100644
>> --- a/.luacheckrc
>> +++ b/.luacheckrc
>> @@ -124,3 +122,38 @@ files["test/box-tap/extended_error.test.lua"] = {
>> "forbidden_function",
>> },
>> }
>> +files["test/sql-tap/analyze5.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>> +files["test/sql-tap/cast.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>> +files["test/sql-tap/func.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>> +files["test/sql-tap/e_expr.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>> +files["test/sql-tap/e_select1.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>> +files["test/sql-tap/misc1.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>> +files["test/sql-tap/sort.test.lua"] = {
>> + ignore = {
>> + "113",
>> + },
>> +}
>
> 1. The commit title says "fix", but it does not look like a fix.
> I suggest you to try harder to remove the warnings, not hide them.
> For example, analyze5 can be fixed like this:
>
> ====================
> diff --git a/.luacheckrc b/.luacheckrc
> index eb709d6a3..977eb2945 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -122,11 +122,6 @@ files["test/box-tap/extended_error.test.lua"] = {
> "forbidden_function",
> },
> }
> -files["test/sql-tap/analyze5.test.lua"] = {
> - ignore = {
> - "113",
> - },
> -}
> files["test/sql-tap/cast.test.lua"] = {
> ignore = {
> "113",
> diff --git a/test/sql-tap/analyze5.test.lua b/test/sql-tap/analyze5.test.lua
> index 1d6665ad0..a9eb4dd2f 100755
> --- a/test/sql-tap/analyze5.test.lua
> +++ b/test/sql-tap/analyze5.test.lua
> @@ -26,6 +26,8 @@ local function eqp(sql)
> return test:execsql("EXPLAIN QUERY PLAN"..sql)
> end
>
> +local X = nil
> +
> local function alpha(blob)
> local ret = ""
> for _, c in ipairs(X(37, "X!cmd", [=[["split",["blob"],""]]=])) do
> @@ -155,6 +157,9 @@ test:do_test(
>
> -- Verify that range queries generate the correct row count estimates
> --
> +local t1x = nil
> +local t1y = nil
> +local t1z = nil
> for i, v in pairs({
> {'z>=0 AND z<=0', t1z, 400},
> {'z>=1 AND z<=1', t1z, 300},
> ====================
>
> It does not make much sense, and the whole test does not make it, because
> it is disabled. But it is better than just move the ignores from one
> place to another. At least now they are local to the disabled places,
> not to the whole file.
>
> Cast.test.lua:
>
> ====================
> diff --git a/.luacheckrc b/.luacheckrc
> index eb709d6a3..15944e378 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -127,11 +127,6 @@ files["test/sql-tap/analyze5.test.lua"] = {
> "113",
> },
> }
> -files["test/sql-tap/cast.test.lua"] = {
> - ignore = {
> - "113",
> - },
> -}
> files["test/sql-tap/func.test.lua"] = {
> ignore = {
> "113",
> diff --git a/test/sql-tap/cast.test.lua b/test/sql-tap/cast.test.lua
> index 4d7c92cfc..bcee65d9b 100755
> --- a/test/sql-tap/cast.test.lua
> +++ b/test/sql-tap/cast.test.lua
> @@ -786,13 +786,19 @@ test:do_execsql_test(
> -- Test to see if it is possible to trick sql into reading past
> -- the end of a blob when converting it to a number.
> if 0 > 0 then
> +-- Legacy from the original code. Must be replaced with analogue
> +-- functions from box.
> +local sql_prepare = nil
> +local sql_bind_blob = nil
> +local sql_step = nil
> +local sql_column_int = nil
> +local STMT
> test:do_test(
> "cast-3.32.1",
> function()
> - local blob, DB, STMT
> + local blob
> blob = 1234567890
> - DB = sql_connection_pointer("db")
> - STMT = sql_prepare(DB, "SELECT CAST(? AS NUMBER)", -1, "TAIL")
> + STMT = sql_prepare("SELECT CAST(? AS NUMBER)", -1, "TAIL")
> sql_bind_blob("-static", STMT, 1, blob, 5)
> return sql_step(STMT)
> end, {
> ====================
>
> Func.test.lua:
>
> ====================
> diff --git a/.luacheckrc b/.luacheckrc
> index eb709d6a3..7608240c5 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -132,11 +132,6 @@ files["test/sql-tap/cast.test.lua"] = {
> "113",
> },
> }
> -files["test/sql-tap/func.test.lua"] = {
> - ignore = {
> - "113",
> - },
> -}
> files["test/sql-tap/e_expr.test.lua"] = {
> ignore = {
> "113",
> diff --git a/test/sql-tap/func.test.lua b/test/sql-tap/func.test.lua
> index d14eef421..a83b15fbc 100755
> --- a/test/sql-tap/func.test.lua
> +++ b/test/sql-tap/func.test.lua
> @@ -1048,7 +1048,8 @@ test:do_execsql_test(
> --
> -- MUST_WORK_TEST testfunc not implemented
> if 0 > 0 then
> -local DB = sql_connection_pointer("db")
> +local DB = nil
> +local X = nil
> X(525, "X!cmd", [=[["sql_register_test_function",["::DB"],"testfunc"]]=])
> test:do_catchsql_test(
> "func-10.1",
> @@ -1142,6 +1143,18 @@ end
> --
> -- MUST_WORK_TEST test_destructor_count not implemented
> if 0 > 0 then
> +local X = nil
> +-- Legacy from the original code. Must be replaced with analogue
> +-- functions from box.
> +local sql_prepare = nil
> +local sql_bind_text = nil
> +local sql_bind_blob = nil
> +local sql_column_text = nil
> +local sql_step = nil
> +local sql_finalize = nil
> +local db = nil
> +local STMT
> +
> if X(595, "X!cmd", "[\"expr\",\"[db eval {PRAGMA encoding}]==\\\"UTF-8\\\"\"]")
> then
> test:do_execsql_test(
> @@ -1300,10 +1313,9 @@ test:do_execsql_test(
> test:do_test(
> "func-13.7",
> function()
> - local DB, sql, STMT, res
> - DB = sql_connection_pointer("db")
> + local sql, STMT, res
> sql = "SELECT test_auxdata( ? , a ) FROM t4;"
> - STMT = sql_prepare(DB, sql, -1, "TAIL")
> + STMT = sql_prepare(sql, -1, "TAIL")
> sql_bind_text(STMT, 1, "hello\0", -1)
> res = { }
> while X(690, "X!cmd", [=[["expr"," \"sql_ROW\"==[sql_step $STMT] "]]=])
> @@ -1455,7 +1467,7 @@ test:do_test(
> test:execsql([[
> CREATE TABLE tbl2(id integer primary key, a INT, b INT);
> ]])
> - local STMT = sql_prepare(DB, "INSERT INTO tbl2 VALUES(1, ?, ?)", -1, "TAIL")
> + local STMT = sql_prepare("INSERT INTO tbl2 VALUES(1, ?, ?)", -1, "TAIL")
> sql_bind_blob(STMT, 1, "abc", 3)
> sql_step(STMT)
> sql_finalize(STMT)
> @@ -2439,6 +2451,7 @@ test:do_execsql_test(
> --
> -- MUST_WORK_TEST
> if 0>0 then
> +local X = nil
> test:do_execsql_test(
> "func-25.1",
> [[
> @@ -2470,7 +2483,7 @@ test:do_test(
> "func-26.2",
> function()
> local a = ""
> - for _ in X(0, "X!for", [=[["set i 1","$i<=$::sql_MAX_FUNCTION_ARG","incr i"]]=]) do
> + for i in X(0, "X!for", [=[["set i 1","$i<=$::sql_MAX_FUNCTION_ARG","incr i"]]=]) do
> table.insert(a,i)
> end
> return test:execsql(string.format([[
> @@ -2486,7 +2499,7 @@ test:do_test(
> "func-26.3",
> function()
> local a = ""
> - for _ in X(0, "X!for", [=[["set i 1","$i<=$::sql_MAX_FUNCTION_ARG+1","incr i"]]=]) do
> + for i in X(0, "X!for", [=[["set i 1","$i<=$::sql_MAX_FUNCTION_ARG+1","incr i"]]=]) do
> table.insert(a,i)
> end
> return test:catchsql(string.format([[
> @@ -2502,7 +2515,7 @@ test:do_test(
> "func-26.4",
> function()
> local a = ""
> - for _ in X(0, "X!for", [=[["set i 1","$i<=$::sql_MAX_FUNCTION_ARG-1","incr i"]]=]) do
> + for i in X(0, "X!for", [=[["set i 1","$i<=$::sql_MAX_FUNCTION_ARG-1","incr i"]]=]) do
> table.insert(a,i)
> end
> return test:catchsql(string.format([[
> ====================
>
> I am sure the other files also can be properly fixed, with stubs
> and comments for legacy old functions, and with normal fixes for
> the things like in the last lines of the diff above, where it was
> a real bug - loop variable was not defined.
>
>> diff --git a/test/sql-tap/badutf1.test.lua b/test/sql-tap/badutf1.test.lua
>> index 654629bf7..b7fa8ce0b 100755
>> --- a/test/sql-tap/badutf1.test.lua
>> +++ b/test/sql-tap/badutf1.test.lua
>> @@ -93,8 +93,12 @@ test:do_test(
>> })
>>
>> -- commented as it uses utf16
>> -if 0>0 then
>> -sql("db2", "")
>> +-- testcases are broken
>> +-- https://github.com/tarantool/tarantool/issues/5743
>> +local is_gh_5743_closed = false
>> +if is_gh_5743_closed then
>> +sql("db2", "") -- luacheck: ignore sql
>> +-- luacheck: push ignore sql_exec
>
> 2. Or you can use 'local sql_exec = nil', and won't need to
> write 'pop'.
>
> The same to the other places, where you try to ignore the same
> unknown variable again and again. However I don't insist on
> hacks vs luacheck comments.
>
>> test:do_test(
>> "badutf-1.10",
>> function()
>> @@ -206,7 +210,7 @@ test:do_test(
>> -- </badutf-1.20>
>> })
>> end
>> -
>> +-- luacheck: pop
>> > diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua
>> index 2f1af29b0..67f461132 100755
>> --- a/test/sql-tap/view.test.lua
>> +++ b/test/sql-tap/view.test.lua
>> @@ -1,6 +1,6 @@
>> #!/usr/bin/env tarantool
>> local test = require("sqltester")
>> -test:plan(78)
>> +test:plan(83)
>>
>> --!./tcltestrunner.lua
>> -- 2002 February 26
>> @@ -501,21 +501,18 @@ test:do_execsql_test(
>> })
>>
>> -- MUST_WORK_TEST
>
> 3. Why did you keep MUST_WORK_TEST, if it passes now?
> The same in other places below.
>
>> -if (0 > 0)
>> - then
>> - test:do_test(
>> - "view-7.2",
>> - function()
>> - return test:execsql [[
>> - SELECT * FROM test;
>> - ]]
>> - end, {
>> - -- <view-7.2>
>> - 1, 2, 3
>> - -- </view-7.2>
>> - })
>> +test:do_test(
>> + "view-7.2",
>> + function()
>> + return test:execsql [[
>> + SELECT * FROM test;
>> + ]]
>> + end, {
>> + -- <view-7.2>
>> + 1, 2, 3
>> + -- </view-7.2>
>> + })
>>
>> -end
>> test:do_execsql_test(
>> "view-7.3",
>> [[
>> @@ -531,21 +528,18 @@ test:do_execsql_test(
>> })
>>
>> -- MUST_WORK_TEST
>> -if (0 > 0)
>> - then
>> - test:do_test(
>> - "view-7.4",
>> - function()
>> - return test:execsql [[
>> - SELECT * FROM test;
>> - ]]
>> - end, {
>> - -- <view-7.4>
>> - 1, 2, 3
>> - -- </view-7.4>
>> - })
>> +test:do_test(
>> + "view-7.4",
>> + function()
>> + return test:execsql [[
>> + SELECT * FROM test;
>> + ]]
>> + end, {
>> + -- <view-7.4>
>> + 1, 2, 3
>> + -- </view-7.4>
>> + })
>>
>> -end
>> test:do_execsql_test(
>> "view-7.5",
>> [[
>> @@ -561,21 +555,18 @@ test:do_execsql_test(
>> })
>>
>> -- MUST_WORK_TEST
>> -if (0 > 0)
>> - then
>> - test:do_test(
>> - "view-7.6",
>> - function()
>> - return test:execsql [[
>> - SELECT * FROM test;
>> - ]]
>> - end, {
>> - -- <view-7.6>
>> - 1, 2, 3
>> - -- </view-7.6>
>> - })
>> +test:do_test(
>> + "view-7.6",
>> + function()
>> + return test:execsql [[
>> + SELECT * FROM test;
>> + ]]
>> + end, {
>> + -- <view-7.6>
>> + 1, 2, 3
>> + -- </view-7.6>
>> + })
>>
>> -end
>> test:do_execsql_test(
>> "view-8.1",
>> [[
>> @@ -588,33 +579,29 @@ test:do_execsql_test(
>> })
>>
>> -- MUST_WORK_TEST
>> -if (0 > 0)
>> - then
>> - test:do_test(
>> - "view-8.2",
>> - function()
>> - return test:execsql [[
>> - SELECT * FROM v6 ORDER BY xyz;
>> - ]]
>> - end, {
>> - -- <view-8.2>
>> - 7, 2, 13, 5, 19, 8, 27, 12
>> - -- </view-8.2>
>> - })
>> -
>> - -- MUST_WORK_TEST problem with column names
>> - test:do_execsql_test(
>> - "view-8.3",
>> - [[
>> - CREATE VIEW v7(a) AS SELECT pqr+xyz FROM v6;
>> - SELECT * FROM v7 ORDER BY a;
>> - ]], {
>> - -- <view-8.3>
>> - 9, 18, 27, 39
>> - -- </view-8.3>
>> - })
>> +test:do_test(
>> + "view-8.2",
>> + function()
>> + return test:execsql [[
>> + SELECT * FROM v6 ORDER BY xyz;
>> + ]]
>> + end, {
>> + -- <view-8.2>
>> + 7, 2, 13, 5, 19, 8, 27, 12
>> + -- </view-8.2>
>> + })
>>
>> -end
>> +-- MUST_WORK_TEST problem with column names
>> +test:do_execsql_test(
>> + "view-8.3",
>> + [[
>> + CREATE VIEW v7(a) AS SELECT pqr+xyz FROM v6;
>> + SELECT * FROM v7 ORDER BY a;
>> + ]], {
>> + -- <view-8.3>
>> + 9, 18, 27, 39
>> + -- </view-8.3>
>> + })
>>
>> test:do_execsql_test(
>> "view-8.4",
More information about the Tarantool-patches
mailing list