[Tarantool-patches] [PATCH v8 03/14] test: fix luacheck warnings W113 in test/sql-tap
Sergey Bronnikov
sergeyb at tarantool.org
Sat Feb 27 20:53:09 MSK 2021
Hello!
On 27.02.2021 02:42, Vladislav Shpilevoy wrote:
> Hi again! Thanks for the fixes!
>
>>>> -if 0>0 then
>>>> -db("nullvalue", "null")
>>>> test:do_select_tests(
>>>> "e_select-7.9",
>>>> {
>>>> - {"1", "SELECT NULL UNION ALL SELECT NULL", {null, null}},
>>>> - {"2", "SELECT NULL UNION SELECT NULL", {null}},
>>>> - {"3", "SELECT NULL INTERSECT SELECT NULL", {null}},
>>>> + {"1", "SELECT NULL UNION ALL SELECT NULL", {"", ""}},
>>>> + {"2", "SELECT NULL UNION SELECT NULL", {""}},
>>>> + {"3", "SELECT NULL INTERSECT SELECT NULL", {""}},
>>>> {"4", "SELECT NULL EXCEPT SELECT NULL", {}},
>>> 2. But why? What was the effect of `db("nullvalue", "null")` in the
>>> original tests? If you don't know then maybe it is worth leaving it
>>> as is + adding `local null = box.NULL` or `local null = nil` so at
>>> least we could preserve the test body unchanged for future
>>> investigations.
>> Well, reverted hunks back and disabled testcase in the test.
>>
>> I found a source code of original test [1] and it is not clear
>>
>> why "nullvalue" redefined there.
>>
>> 1. https://www.sqlite.org/src/file?ci=trunk&name=test%2Fe_select.test&ln=1504
> It is not 'redefinition' it seems.
> https://www.oreilly.com/library/view/using-sqlite/9781449394592/re64.html
>
> It is a marker for output. From what I understood, sqlite prints NULLs as
> empty string, and here in the test they wanted to look for NULLs in the
> result. But you can't look for N empty strings. So NULL's string representation
> was replaced with "null" temporary. In our case it won't happen because we
> know number of returned columns and their exact values. Somebody will find
> the same in the future and will either fix or delete the test.
>
> See 4 more comments below and diff in the end of the email.
>
>> diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua
>> index b22f195ca..4704970c8 100755
>> --- a/test/sql-tap/lua_sql.test.lua
>> +++ b/test/sql-tap/lua_sql.test.lua
>> @@ -75,9 +75,10 @@ box.schema.func.create('CHECK_FROM_SQL_TO_LUA', {language = 'Lua',
>> exports = {'LUA', 'SQL'}})
>>
>> -- check for different types
>> -for i = 1, #from_sql_to_lua, 1 do
>> +for i = 1, #from_sql_to_lua, 1 do -- luacheck: ignore from_sql_to_lua
>> test:do_execsql_test(
>> "lua_sql-2.2."..i,
>> + -- luacheck: ignore from_sql_to_lua
> 1. You can just declare local from_sql_to_lua and also save it to _G. No
> need to ignore it.
Thanks for solution, it's definitely better than suppression.
>> "select check_from_sql_to_lua("..i..","..from_sql_to_lua[i][1]..")",
>> {1})
>> end
>> @@ -102,9 +103,10 @@ box.schema.func.create('CHECK_FROM_LUA_TO_SQL', {language = 'Lua',
>> exports = {'LUA', 'SQL'}})
>>
>> -- check for different types
>> -for i = 1, #from_lua_to_sql, 1 do
>> +for i = 1, #from_lua_to_sql, 1 do -- luacheck: ignore from_lua_to_sql
>> test:do_execsql_test(
>> "lua_sql-2.3."..i,
>> + -- luacheck: ignore from_lua_to_sql
>> "select "..tostring(from_lua_to_sql[i][1]).." = check_from_lua_to_sql("..i..")",
>> {true})
>> end
>> diff --git a/test/sql-tap/minmax2.test.lua b/test/sql-tap/minmax2.test.lua
>> index f3d608aab..b6c02a4cc 100755
>> --- a/test/sql-tap/minmax2.test.lua
>> +++ b/test/sql-tap/minmax2.test.lua
>> @@ -72,6 +72,7 @@ test:do_test(
>> test:do_test(
>> "minmax2-1.2",
>> function()
>> + -- luacheck: ignore sql_search_count
>> return box.stat.sql().sql_search_count - sql_search_count
> 2. There is no reason for sql_search_count to be global.
>
made it local
>> end, 19)
>>
>> @@ -89,6 +90,7 @@ test:do_test(
>> test:do_test(
>> "minmax2-1.4",
>> function()
>> + -- luacheck: ignore sql_search_count
>> return box.stat.sql().sql_search_count - sql_search_count
>> end, 19)
>>
>> diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
>> index f57a22624..bb03f25ab 100755
>> --- a/test/sql-tap/subquery.test.lua
>> +++ b/test/sql-tap/subquery.test.lua
>> @@ -684,6 +684,8 @@ test:do_test(
>> -- This is the key test. The subquery should have only run once. If
>> -- The double-quoted identifier "two" were causing the subquery to be
>> -- processed as a correlated subquery, then it would have run 4 times.
>> +
>> + -- luacheck: ignore callcnt
>> return callcnt
> 3. This can be fixed, lets not ignore it. One way would be
> to store callcnt in a table which is saved in a local variable
> and into _G.
>
> local callcnt = {value = 0}
> _G.callcnt = callcnt
>
> Then you can use callcnt.value from anywhere to reference the
> same counter.
>
> Another way is to have it local and update it via a function
> stored in _G.
>
> Also can simply write _G.callcnt in 3 more places instead of
> ignoring it.
Done
>
>> end, 1)
>>
>> @@ -706,7 +708,9 @@ test:do_test(
>> test:do_test(
>> "subquery-6.2",
>> function()
>> + -- luacheck: ignore callcnt
>> return callcnt
>> +
>> end, 4)
>>
>> test:do_test(
>> @@ -725,6 +729,7 @@ test:do_test(
>> test:do_test(
>> "subquery-6.4",
>> function()
>> + -- luacheck: ignore callcnt
>> return callcnt
>> end, 1)
>>
>> diff --git a/test/sql-tap/triggerC.test.lua b/test/sql-tap/triggerC.test.lua
>> index e95641938..7d496bc4d 100755
>> --- a/test/sql-tap/triggerC.test.lua
>> +++ b/test/sql-tap/triggerC.test.lua
>> @@ -791,7 +791,6 @@ for testno, v in ipairs(tests11) do
>> SELECT a,b FROM log;
>> ]]
>> end, {
>> - defaults
> 4. The variable is supposed to be filled with something, from
> what I see in the commented code.
>
> You can declare it with nil somewhere above to keep the not
> working tests intact.
luacheck doesn't warn about it and tests passed successfully.
Although from code style point of view it's better to define it...
--- a/test/sql-tap/triggerC.test.lua
+++ b/test/sql-tap/triggerC.test.lua
@@ -779,6 +779,8 @@ for testno, v in ipairs(tests11) do
-- -- X(891, "X!cmd",
[=[["concat",["defaults"],["defaults"]]]=])
-- })
+ -- Legacy from the original code. Must be replaced with valid value.
+ local defaults = nil
test:do_test(
"triggerC-11."..testno..".3",
function()
@@ -790,6 +792,7 @@ for testno, v in ipairs(tests11) do
SELECT a,b FROM log;
]]
end, {
+ defaults
})
--
>> })
>>
>> --
>> diff --git a/test/sql-tap/view.test.lua b/test/sql-tap/view.test.lua
>> index 2f1af29b0..40fc71998 100755
>> --- a/test/sql-tap/view.test.lua
>> +++ b/test/sql-tap/view.test.lua
>> @@ -1119,6 +1093,9 @@ if (0 > 0)
>> -- </view-22.1>
>> })
>>
>> + -- Legacy from the original code. Must be replaced with analogue
>> + -- functions from box.
>> + local X = nil
>> test:do_test(
>> "view-22.2",
>> function()
> I tried to fix some of my comments:
>
> ====================
> diff --git a/test/sql-tap/lua_sql.test.lua b/test/sql-tap/lua_sql.test.lua
> index 4704970c8..d073ffe4f 100755
> --- a/test/sql-tap/lua_sql.test.lua
> +++ b/test/sql-tap/lua_sql.test.lua
> @@ -51,7 +51,7 @@ for _, val in ipairs({
> {result})
> end
>
> -_G.from_sql_to_lua = {
> +local from_sql_to_lua = {
> [1] = {1, 1},
> [2] = {"1", 1},
> [3] = {"1.5", 1.5},
> @@ -60,6 +60,7 @@ _G.from_sql_to_lua = {
> [6] = {"x'0500'", "\u{0005}\u{0000}"},
> [7] = {"123123123123123", 123123123123123LL},
> }
> +_G.from_sql_to_lua = from_sql_to_lua
>
> box.schema.func.create('CHECK_FROM_SQL_TO_LUA', {language = 'Lua',
> is_deterministic = true,
> @@ -75,10 +76,9 @@ box.schema.func.create('CHECK_FROM_SQL_TO_LUA', {language = 'Lua',
> exports = {'LUA', 'SQL'}})
>
> -- check for different types
> -for i = 1, #from_sql_to_lua, 1 do -- luacheck: ignore from_sql_to_lua
> +for i = 1, #from_sql_to_lua, 1 do
> test:do_execsql_test(
> "lua_sql-2.2."..i,
> - -- luacheck: ignore from_sql_to_lua
> "select check_from_sql_to_lua("..i..","..from_sql_to_lua[i][1]..")",
> {1})
> end
> diff --git a/test/sql-tap/minmax2.test.lua b/test/sql-tap/minmax2.test.lua
> index b6c02a4cc..6a7859d42 100755
> --- a/test/sql-tap/minmax2.test.lua
> +++ b/test/sql-tap/minmax2.test.lua
> @@ -56,12 +56,12 @@ test:do_execsql_test(
> -- </minmax2-1.0>
> })
>
> -_G.sql_search_count = 0
> +local sql_search_count = 0
>
> test:do_test(
> "minmax2-1.1",
> function()
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT min(x) FROM t1"
> end, {
> -- <minmax2-1.1>
> @@ -72,14 +72,13 @@ test:do_test(
> test:do_test(
> "minmax2-1.2",
> function()
> - -- luacheck: ignore sql_search_count
> return box.stat.sql().sql_search_count - sql_search_count
> end, 19)
>
> test:do_test(
> "minmax2-1.3",
> function()
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT max(x) FROM t1"
> end, {
> -- <minmax2-1.3>
> @@ -90,7 +89,6 @@ test:do_test(
> test:do_test(
> "minmax2-1.4",
> function()
> - -- luacheck: ignore sql_search_count
> return box.stat.sql().sql_search_count - sql_search_count
> end, 19)
>
> @@ -98,7 +96,7 @@ test:do_test(
> "minmax2-1.5",
> function()
> test:execsql "CREATE INDEX t1i1 ON t1(x DESC)"
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT min(x) FROM t1"
> end, {
> -- <minmax2-1.5>
> @@ -109,13 +107,13 @@ test:do_test(
> test:do_test(
> "minmax2-1.6",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 1)
>
> test:do_test(
> "minmax2-1.7",
> function()
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT max(x) FROM t1"
> end, {
> -- <minmax2-1.7>
> @@ -126,13 +124,13 @@ test:do_test(
> test:do_test(
> "minmax2-1.8",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 0)
>
> test:do_test(
> "minmax2-1.9",
> function()
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT max(y) FROM t1"
> end, {
> -- <minmax2-1.9>
> @@ -143,7 +141,7 @@ test:do_test(
> test:do_test(
> "minmax2-1.10",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 19)
>
> test:do_test(
> @@ -153,7 +151,7 @@ test:do_test(
> CREATE TABLE t2(a INTEGER PRIMARY KEY, b INT );
> INSERT INTO t2 SELECT x, y FROM t1;
> ]]
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT min(a) FROM t2"
> end, {
> -- <minmax2-2.0>
> @@ -164,13 +162,13 @@ test:do_test(
> test:do_test(
> "minmax2-2.1",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 0)
>
> test:do_test(
> "minmax2-2.2",
> function()
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT max(a) FROM t2"
> end, {
> -- <minmax2-2.2>
> @@ -181,7 +179,7 @@ test:do_test(
> test:do_test(
> "minmax2-2.3",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 0)
>
> test:do_test(
> @@ -190,7 +188,7 @@ test:do_test(
> test:execsql "INSERT INTO t2 VALUES((SELECT max(a) FROM t2)+1,999)"
>
>
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql "SELECT max(a) FROM t2"
> end, {
> -- <minmax2-3.0>
> @@ -201,7 +199,7 @@ test:do_test(
> test:do_test(
> "minmax2-3.1",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 0)
>
> test:do_test(
> @@ -210,7 +208,7 @@ test:do_test(
> test:execsql "INSERT INTO t2 VALUES((SELECT max(a) FROM t2)+1,999)"
>
>
> - _G.sql_search_count = box.stat.sql().sql_search_count
> + sql_search_count = box.stat.sql().sql_search_count
> return test:execsql " SELECT b FROM t2 WHERE a=(SELECT max(a) FROM t2) "
>
>
> @@ -224,7 +222,7 @@ test:do_test(
> test:do_test(
> "minmax2-3.3",
> function()
> - return box.stat.sql().sql_search_count - _G.sql_search_count
> + return box.stat.sql().sql_search_count - sql_search_count
> end, 1)
>
> test:do_execsql_test(
> diff --git a/test/sql-tap/subquery.test.lua b/test/sql-tap/subquery.test.lua
> index bb03f25ab..df3075ee3 100755
> --- a/test/sql-tap/subquery.test.lua
> +++ b/test/sql-tap/subquery.test.lua
> @@ -685,8 +685,7 @@ test:do_test(
> -- The double-quoted identifier "two" were causing the subquery to be
> -- processed as a correlated subquery, then it would have run 4 times.
>
> - -- luacheck: ignore callcnt
> - return callcnt
> + return _G.callcnt
> end, 1)
>
> -- Ticket #1380. Make sure correlated subqueries on an IN clause work
> @@ -708,8 +707,7 @@ test:do_test(
> test:do_test(
> "subquery-6.2",
> function()
> - -- luacheck: ignore callcnt
> - return callcnt
> + return _G.callcnt
>
> end, 4)
>
> @@ -729,8 +727,7 @@ test:do_test(
> test:do_test(
> "subquery-6.4",
> function()
> - -- luacheck: ignore callcnt
> - return callcnt
> + return _G.callcnt
> end, 1)
>
> box.func.CALLCNT:drop()
>
More information about the Tarantool-patches
mailing list