[Tarantool-patches] [PATCH v4 6/10] Fix luacheck warnings in test/
Igor Munkin
imun at tarantool.org
Mon Apr 27 17:38:28 MSK 2020
Sergey,
Thanks for the patch! I didn't finish the review, but I'm not going to
continue with this version (the reason is at the bottom of my reply).
However I left several comments for the part I've checked, please
consider them.
On 21.04.20, sergeyb at tarantool.org wrote:
> From: Sergey Bronnikov <sergeyb at tarantool.org>
>
> Closes #4681
>
> Reviewed-by: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
> Reviewed-by: Igor Munkin <imun at tarantool.org>
>
> Co-authored-by: Vladislav Shpilevoy <v.shpilevoy at tarantool.org>
> Co-authored-by: Igor Munkin <imun at tarantool.org>
> ---
> .luacheckrc | 34 ++++++
> test/app-tap/cfg.test.lua | 2 +-
> test/app-tap/clock.test.lua | 4 +-
> test/app-tap/console.test.lua | 15 +--
> test/app-tap/console_lua.test.lua | 22 ++--
> test/app-tap/csv.test.lua | 56 ++++-----
> test/app-tap/debug.test.lua | 10 +-
> test/app-tap/errno.test.lua | 6 +-
> test/app-tap/fail_main.test.lua | 6 +-
> .../gh-4761-json-per-call-options.test.lua | 11 +-
> test/app-tap/http_client.test.lua | 54 ++++-----
> test/app-tap/iconv.test.lua | 6 +-
> test/app-tap/init_script.test.lua | 14 +--
> test/app-tap/inspector.test.lua | 6 +-
> test/app-tap/json.test.lua | 1 -
> test/app-tap/logger.test.lua | 23 ++--
> test/app-tap/minimal.test.lua | 4 +-
> test/app-tap/module_api.test.lua | 11 +-
> test/app-tap/msgpackffi.test.lua | 3 +-
> test/app-tap/pcall.test.lua | 6 +-
> test/app-tap/snapshot.test.lua | 17 ++-
> test/app-tap/string.test.lua | 54 ++++-----
> test/app-tap/tap.test.lua | 26 ++--
> test/app-tap/tarantoolctl.test.lua | 76 ++++++------
> test/app-tap/trigger.test.lua | 20 ++--
> test/app-tap/yaml.test.lua | 16 +--
> test/box-py/box.lua | 2 +-
> test/box-tap/auth.test.lua | 36 +++---
> test/box-tap/cfg.test.lua | 34 +++---
> test/box-tap/cfgup.test.lua | 2 +-
> test/box-tap/feedback_daemon.test.lua | 4 +-
> test/box-tap/gc.test.lua | 6 +-
> test/box-tap/key_def.test.lua | 58 ++++-----
> test/box-tap/merger.test.lua | 112 +++++++++---------
> test/box-tap/on_schema_init.test.lua | 4 +-
> test/box-tap/schema_mt.test.lua | 18 +--
> test/box-tap/session.storage.test.lua | 10 +-
> test/box-tap/session.test.lua | 59 ++++-----
> test/box-tap/trigger_atexit.test.lua | 10 +-
> test/box-tap/trigger_yield.test.lua | 10 +-
> test/box/box.lua | 2 +-
> test/box/hash_multipart.result | 2 +-
> test/box/hash_multipart.test.lua | 2 +-
> test/box/lua/bitset.lua | 7 +-
> test/box/lua/cfg_bad_vinyl_dir.lua | 2 +-
> test/box/lua/cfg_rtree.lua | 2 +-
> test/box/lua/cfg_test1.lua | 2 +-
> test/box/lua/cfg_test2.lua | 2 +-
> test/box/lua/cfg_test3.lua | 2 +-
> test/box/lua/cfg_test4.lua | 2 +-
> test/box/lua/cfg_test5.lua | 4 +-
> test/box/lua/cfg_test6.lua | 2 +-
> test/box/lua/fifo.lua | 2 +-
> test/box/lua/identifier.lua | 9 +-
> test/box/lua/index_random_test.lua | 2 +-
> test/box/lua/require_init.lua | 3 -
> test/box/lua/test_init.lua | 10 +-
> test/box/lua/utils.lua | 13 +-
> test/box/on_schema_init.lua | 2 +-
> test/box/proxy.lua | 2 +-
> test/box/tiny.lua | 2 +-
> test/box/tree_pk.result | 4 +-
> test/box/tree_pk.test.lua | 4 +-
> test/engine/tree_min_max_count.result | 2 +-
> test/engine/tree_min_max_count.test.lua | 2 +-
> test/engine_long/suite.lua | 4 +-
> test/long_run-py/lua/finalizers.lua | 8 +-
> test/long_run-py/suite.lua | 6 +-
> test/replication-py/master.lua | 2 +-
> test/replication-py/panic.lua | 2 +-
> test/replication-py/replica.lua | 4 -
> test/replication/lua/fast_replica.lua | 3 +-
> test/replication/lua/rlimit.lua | 2 +-
> test/replication/master.lua | 2 +-
> test/replication/replicaset_ro_mostly.result | 2 +-
> .../replication/replicaset_ro_mostly.test.lua | 2 +-
> test/sql-tap/alter.test.lua | 4 +-
> test/sql-tap/analyze3.test.lua | 6 +-
> test/sql-tap/analyze5.test.lua | 2 +-
> test/sql-tap/analyze9.test.lua | 30 ++---
> test/sql-tap/between.test.lua | 4 +-
> test/sql-tap/date.test.lua | 3 +-
> test/sql-tap/delete1.test.lua | 2 +-
> test/sql-tap/e_delete.test.lua | 2 +-
> test/sql-tap/e_expr.test.lua | 22 ++--
> test/sql-tap/func.test.lua | 2 +-
> test/sql-tap/func3.test.lua | 24 ++--
> test/sql-tap/gh-2723-concurrency.test.lua | 8 +-
> .../gh-3083-ephemeral-unref-tuples.test.lua | 2 +-
> .../gh-3307-xfer-optimization-issue.test.lua | 16 +--
> .../gh-3332-tuple-format-leak.test.lua | 2 +-
> .../gh-4077-iproto-execute-no-bind.test.lua | 7 +-
> .../gh2127-indentifier-max-length.test.lua | 10 +-
> test/sql-tap/identifier-characters.test.lua | 2 +-
> test/sql-tap/index1.test.lua | 3 +-
> test/sql-tap/index7.test.lua | 2 +-
> test/sql-tap/join3.test.lua | 2 +-
> test/sql-tap/lua-tables.test.lua | 2 +-
> test/sql-tap/lua/sqltester.lua | 28 ++---
> test/sql-tap/misc1.test.lua | 10 +-
> test/sql-tap/misc5.test.lua | 2 +-
> test/sql-tap/select1.test.lua | 10 +-
> test/sql-tap/select2.test.lua | 8 +-
> test/sql-tap/select4.test.lua | 1 -
> test/sql-tap/select5.test.lua | 1 -
> test/sql-tap/select9.test.lua | 12 +-
> test/sql-tap/selectA.test.lua | 8 +-
> test/sql-tap/selectB.test.lua | 14 +--
> test/sql-tap/selectG.test.lua | 1 -
> test/sql-tap/sort.test.lua | 2 +-
> test/sql-tap/sql-errors.test.lua | 2 +-
> test/sql-tap/table.test.lua | 3 +-
> test/sql-tap/tkt-38cb5df375.test.lua | 1 -
> test/sql-tap/tkt-91e2e8ba6f.test.lua | 3 -
> test/sql-tap/tkt-9a8b09f8e6.test.lua | 3 -
> test/sql-tap/tkt-bd484a090c.test.lua | 3 +-
> test/sql-tap/tkt-fa7bf5ec.test.lua | 6 +-
> test/sql-tap/tkt2192.test.lua | 3 +-
> test/sql-tap/tkt3493.test.lua | 3 -
> test/sql-tap/trigger2.test.lua | 4 +-
> test/sql-tap/triggerA.test.lua | 1 -
> test/sql-tap/where2.test.lua | 9 +-
> test/sql-tap/where3.test.lua | 2 +-
> test/sql-tap/where4.test.lua | 4 +-
> test/sql-tap/where5.test.lua | 2 +-
> test/sql-tap/where6.test.lua | 2 +-
> test/sql-tap/where7.test.lua | 16 +--
> test/sql-tap/whereA.test.lua | 2 +-
> test/sql-tap/whereB.test.lua | 2 +-
> test/sql-tap/whereC.test.lua | 5 +-
> test/sql-tap/whereD.test.lua | 4 +-
> test/sql-tap/whereF.test.lua | 4 +-
> test/sql-tap/whereG.test.lua | 4 +-
> test/sql-tap/whereI.test.lua | 4 +-
> test/sql-tap/whereK.test.lua | 4 +-
> test/sql-tap/with1.test.lua | 14 +--
> test/sql-tap/with2.test.lua | 18 +--
> test/sql/lua/sql_tokenizer.lua | 2 +-
> test/sql/savepoints.result | 6 +-
> test/sql/savepoints.test.lua | 6 +-
> test/sql/triggers.result | 2 +-
> test/sql/triggers.test.lua | 2 +-
> test/vinyl/large.lua | 3 +-
> test/vinyl/txn_proxy.lua | 6 +-
> test/vinyl/upgrade/fill.lua | 8 +-
> test/vinyl/vinyl.lua | 2 +-
> test/wal_off/rtree_benchmark.result | 2 +-
> test/wal_off/rtree_benchmark.test.lua | 2 +-
> test/xlog-py/box.lua | 2 +-
> test/xlog/panic.lua | 2 +-
> test/xlog/reader.result | 2 +-
> test/xlog/reader.test.lua | 2 +-
> test/xlog/snap_io_rate.test.lua | 2 +-
> test/xlog/transaction.result | 2 +-
> test/xlog/transaction.test.lua | 2 +-
> .../2.1.3/gh-4771-upgrade-sequence/fill.lua | 12 +-
> test/xlog/xlog.lua | 2 +-
> 157 files changed, 704 insertions(+), 740 deletions(-)
>
> diff --git a/.luacheckrc b/.luacheckrc
> index 64692b27c..cec1f4ffe 100644
> --- a/.luacheckrc
> +++ b/.luacheckrc
> @@ -37,3 +37,37 @@ files["src/box/lua/console.lua"] = {ignore = {"212"}}
> files["src/box/lua/load_cfg.lua"] = {ignore = {"542"}}
> files["src/box/lua/net_box.lua"] = {ignore = {"431", "432", "231", "411", "212"}}
> files["src/box/lua/schema.lua"] = {ignore = {"431", "432", "542", "212"}}
> +files["test/app/lua/fiber.lua"] = {globals = {"box_fiber_run_test"}}
test/app/{app,loaders}.lua are also Lua source files and need to be
tested via luacheck.
> +files["test/app-tap/console.test.lua"] = {globals = {"long_func"}}
> +files["test/app-tap/lua/require_mod.lua"] = {globals = {"exports"}}
> +files["test/app-tap/module_api.test.lua"] = {ignore = {"311"}}
In previous review I provided a diff with fix for this class of
warnings. You've said nothing against my approaches, but fixed it on
your own way with no discussion. It's simply unproductive, because I
need to double-check all changes now.
> +files["test/app-tap/string.test.lua"] = {globals = {"utf8"}}
> +files["test/app-tap/tarantoolctl.test.lua"] = {ignore = {"113", "421"}}
> +files["test/box-tap/session.test.lua"] = {
> + globals = {"active_connections", "session", "space", "f1", "f2"},
> + ignore = {"211"}
> +}
> +files["test/box/lua/push.lua"] = {globals = {"push_collection"}}
> +files["test/box/lua/index_random_test.lua"] = {globals = {"index_random_test"}}
> +files["test/box/lua/utils.lua"] = {
> + globals = {"space_field_types", "iterate", "arithmetic", "table_shuffle",
> + "table_generate", "tuple_to_string", "check_space", "space_bsize",
> + "create_iterator", "setmap", "sort"}}
> +files["test/box/lua/bitset.lua"] = {
> + globals = {"create_space", "fill", "delete", "clear", "drop_space",
> + "dump", "test_insert_delete"}
> +}
> +files["test/box/lua/fifo.lua"] = {globals = {"fifomax", "find_or_create_fifo", "fifo_push", "fifo_top"}}
> +files["test/box/lua/identifier.lua"] = {globals = {"run_test"}}
> +files["test/box/lua/require_mod.lua"] = {globals = {"exports"}}
> +files["test/luajit-tap/gh-4476-fix-string-find-recording.test.lua"] = {ignore = {"231"}}
> +files["test/luajit-tap/or-232-unsink-64-kptr.test.lua"] = {ignore = {"542"}}
This files have to be added right after luajit submodule bump.
> +files["test/replication/lua/fast_replica.lua"] = {
> + globals = {"join", "start_all", "stop_all", "wait_all",
> + "drop_all", "drop_all", "vclock_diff", "unregister",
> + "delete", "start", "stop", "call_all", "drop", "wait"},
> + ignore = {"212", "213"}
> +}
> +files["test/sql-tap/*.lua"] = {ignore = {"611", "612", "613", "614", "621", "631", "211", "113", "111"}}
> +files["test/sql-tap/lua/sqltester.lua"] = {globals = {"table_match_regex_p"}}
> +files["test/sql-tap/e_expr.test.lua"] = {ignore = {"512"}}
<snipped>
> diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua
> index 4feadfa5e..e81b48a80 100755
> --- a/test/app-tap/console.test.lua
> +++ b/test/app-tap/console.test.lua
<snipped>
> @@ -59,7 +59,7 @@ test:is(client:read(";"), 'true;', "pushed message")
> client:write('\\set output lua\n')
> client:read(";")
>
> -long_func_f = nil
> +local long_func_f
> function long_func()
Please consider the fix I proposed in previous review[1]. This warning
is a false positive and can be suppressed inline. Here is the diff:
================================================================================
diff --git a/.luacheckrc b/.luacheckrc
index cec1f4ffe..4faf770e8 100644
--- a/.luacheckrc
+++ b/.luacheckrc
@@ -38,7 +38,6 @@ files["src/box/lua/load_cfg.lua"] = {ignore = {"542"}}
files["src/box/lua/net_box.lua"] = {ignore = {"431", "432", "231", "411", "212"}}
files["src/box/lua/schema.lua"] = {ignore = {"431", "432", "542", "212"}}
files["test/app/lua/fiber.lua"] = {globals = {"box_fiber_run_test"}}
-files["test/app-tap/console.test.lua"] = {globals = {"long_func"}}
files["test/app-tap/lua/require_mod.lua"] = {globals = {"exports"}}
files["test/app-tap/module_api.test.lua"] = {ignore = {"311"}}
files["test/app-tap/string.test.lua"] = {globals = {"utf8"}}
diff --git a/test/app-tap/console.test.lua b/test/app-tap/console.test.lua
index e81b48a80..454d5a892 100755
--- a/test/app-tap/console.test.lua
+++ b/test/app-tap/console.test.lua
@@ -60,6 +60,7 @@ client:write('\\set output lua\n')
client:read(";")
local long_func_f
+-- luacheck: globals long_func (is called via client socket)
function long_func()
long_func_f = fiber.self()
box.session.push('push')
================================================================================
> long_func_f = fiber.self()
> box.session.push('push')
<snipped>
> diff --git a/test/app-tap/console_lua.test.lua b/test/app-tap/console_lua.test.lua
> index 3ed6aad97..263e86c54 100755
> --- a/test/app-tap/console_lua.test.lua
> +++ b/test/app-tap/console_lua.test.lua
<snipped>
> @@ -57,8 +57,8 @@ end
>
> --
> -- Execute a list of statements, show requests and responses.
> -local function execute_statements(test, client, statements, name)
> - test:test(name, function(test)
> +local function execute_statements(test_obj, client, statements, name)
Minor: I guess you can use <testcase> for argument as well as for
parameter.
> + test_obj:test(name, function(test)
> test:plan(2 * #statements)
>
> for _, stmt in ipairs(statements) do
> @@ -75,15 +75,15 @@ end
>
> --
> -- Execute a statement and verify its response.
> -local function execute_and_verify(test, client, input, exp_output, name)
> - test:test(name, function(test)
> +local function execute_and_verify(test_obj, client, input, exp_output, name)
Minor: I guess you can use <testcase> for argument as well as for
parameter.
> + test_obj:test(name, function(test)
> test:plan(2)
>
> local res = client:write(input .. '\n')
> test:ok(res ~= nil, ('-> [[%s]]'):format(input))
>
> local exp = exp_output .. EOL
> - local res = client:read(EOL)
> + res = client:read(EOL)
> test:is(res, exp, ('<- [[%s]]'):format(exp:gsub('\n', '\\n')))
> end)
> end
<snipped>
> diff --git a/test/app-tap/errno.test.lua b/test/app-tap/errno.test.lua
> index 5fd8eaca4..4a13edf2a 100755
> --- a/test/app-tap/errno.test.lua
> +++ b/test/app-tap/errno.test.lua
> @@ -3,10 +3,10 @@
> local tap = require('tap')
> local errno = require('errno')
>
> -local test = tap.test("errno")
> +local suite = tap.test("errno")
>
> -test:plan(1)
> -test:test("primary", function(test)
> +suite:plan(1)
> +suite:test("primary", function(test)
> test:plan(10)
> test:is(type(errno), "table", "type of table")
> test:ok(errno.EINVAL ~= nil, "errno.EINVAL is available")
These changes introduce a mess in naming: in previous chunks you rename
<test> argument to <testcase>, but here you decided to change naming for
the root suite. I guess you need either to adjust it to the common
naming (<test> and <testcase>) or leave the shadowing, considering this
warning is raised in many other places and we have no policy for it for
now.
<snipped>
> diff --git a/test/app-tap/gh-4761-json-per-call-options.test.lua b/test/app-tap/gh-4761-json-per-call-options.test.lua
> index 1fb24744e..a8c02dd2f 100755
> --- a/test/app-tap/gh-4761-json-per-call-options.test.lua
> +++ b/test/app-tap/gh-4761-json-per-call-options.test.lua
> @@ -13,23 +13,24 @@ local res = tap.test('gh-4761-json-per-call-options', function(test)
> test:plan(2)
>
> -- Preparation code: call :decode() with a custom option.
> - local ok, err = pcall(json.decode, '{"foo": {"bar": 1}}',
> + local ok, _ = pcall(json.decode, '{"foo": {"bar": 1}}',
Underscore can be freely omitted here (as was proposed in the previous
review[1]).
> {decode_max_depth = 1})
> assert(not ok, 'expect "too many nested data structures" error')
>
> -- Verify that the instance option remains unchanged.
> local exp_res = {foo = {bar = 1}}
> - local ok, res = pcall(json.decode, '{"foo": {"bar": 1}}')
> + local res
> + ok, res = pcall(json.decode, '{"foo": {"bar": 1}}')
> test:is_deeply({ok, res}, {true, exp_res},
> 'json instance settings remain unchanged after :decode()')
>
> -- Same check for json.encode.
> local nan = 1/0
> - local ok, err = pcall(json.encode, {a = nan},
> + ok, _ = pcall(json.encode, {a = nan},
Underscore can be freely omitted here (as was proposed in the previous
review[1]).
> {encode_invalid_numbers = false})
> assert(not ok, 'expected "number must not be NaN or Inf" error')
> - local exp_res = '{"a":inf}'
> - local ok, res = pcall(json.encode, {a = nan})
> + exp_res = '{"a":inf}'
> + ok, res = pcall(json.encode, {a = nan})
> test:is_deeply({ok, res}, {true, exp_res},
> 'json instance settings remain unchanged after :encode()')
> end)
> diff --git a/test/app-tap/http_client.test.lua b/test/app-tap/http_client.test.lua
> index b85b605cf..1c5bf853b 100755
> --- a/test/app-tap/http_client.test.lua
> +++ b/test/app-tap/http_client.test.lua
> @@ -3,13 +3,13 @@
> local tap = require('tap')
> local client = require('http.client')
> local json = require('json')
> -local test = tap.test("curl")
> +local suite = tap.test("curl")
These changes introduce a mess in naming: in previous chunks you rename
<test> argument to <testcase>, but here you decided to change naming for
the root suite. I guess you need either to adjust it to the common
naming (<test> and <testcase>) or leave the shadowing, considering this
warning is raised in many other places and we have no policy for it for
now.
> local fiber = require('fiber')
> local socketlib = require('socket')
> local os = require('os')
>
> local TARANTOOL_SRC_DIR = os.getenv("TARANTOOL_SRC_DIR") or "../.."
> -test:diag("TARANTOOL_SRC_DIR=%s", TARANTOOL_SRC_DIR)
> +suite:diag("TARANTOOL_SRC_DIR=%s", TARANTOOL_SRC_DIR)
>
> local function merge(...)
> local res = {}
<snipped>
> @@ -270,7 +269,7 @@ local function test_errors(test)
> test:ok(not status and string.find(json.encode(err),
> "Unsupported protocol"),
> "POST: exception on bad protocol")
> - local r = http:get("http://do_not_exist_8ffad33e0cb01e6a01a03d00089e71e5b2b7e9930dfcba.ru")
> + http:get("http://do_not_exist_8ffad33e0cb01e6a01a03d00089e71e5b2b7e9930dfcba.ru")
Why did you leave this line? What does it test?
> end
>
> -- gh-3679 Check that opts.headers values can be strings only.
<snipped>
> diff --git a/test/app-tap/logger.test.lua b/test/app-tap/logger.test.lua
> index 492d5ea0b..56db1d2c7 100755
> --- a/test/app-tap/logger.test.lua
> +++ b/test/app-tap/logger.test.lua
> @@ -26,7 +26,7 @@ end
> log.info(message)
> local line = file:read()
> test:is(line:sub(-message:len()), message, "message")
> -s, err = pcall(json.decode, line)
> +local s, _ = pcall(json.decode, line)
Underscore can be freely omitted here (as was proposed in the previous
review[1]).
> test:ok(not s, "plain")
> --
> -- gh-700: Crash on calling log.info() with formatting characters
<snipped>
> @@ -56,34 +56,34 @@ file:close()
>
> test:ok(log.pid() >= 0, "pid()")
>
> --- logger uses 'debug', try to set it to nil
> -debug = nil
> +-- luacheck: ignore (logger uses 'debug', try to set it to nil)
> +local debug = nil
This change breaks the test: debug global variable need to be set to
nil, not the local one. Furthermore, the change differs to the one I've
seen in the previous branch. Has the branch been rebased?
> log.info("debug is nil")
> debug = require('debug')
>
<snipped>
> diff --git a/test/app-tap/module_api.test.lua b/test/app-tap/module_api.test.lua
> index a6658cc61..e0f2bf75f 100755
> --- a/test/app-tap/module_api.test.lua
> +++ b/test/app-tap/module_api.test.lua
There is a fix for the warning related to <obj> variable in previous
review[1]. Please consider it, since this warning will never be fixed.
<snipped>
> diff --git a/test/app-tap/snapshot.test.lua b/test/app-tap/snapshot.test.lua
> index 587f8279b..bd5270702 100755
> --- a/test/app-tap/snapshot.test.lua
> +++ b/test/app-tap/snapshot.test.lua
<snipped>
> @@ -126,11 +125,11 @@ local function gh1094()
> break
> end
> end
> - local sf, mf = pcall(box.snapshot)
> - for i, f in pairs(files) do
> + local sf, _ = pcall(box.snapshot)
>
Underscore can be freely omitted here (as was proposed in the previous
review[1]).
+ for _, f in pairs(files) do
> f:close()
> end
> - local ss, ms = pcall(box.snapshot)
> + local ss, _ = pcall(box.snapshot)
Underscore can be freely omitted here (as was proposed in the previous
review[1]).
> test:ok(not sf and ss, msg)
> end
> gh1094()
<snipped>
> diff --git a/test/app-tap/string.test.lua b/test/app-tap/string.test.lua
> index 02a1a84d7..bf1be5c9e 100755
> --- a/test/app-tap/string.test.lua
> +++ b/test/app-tap/string.test.lua
> @@ -1,11 +1,11 @@
> #!/usr/bin/env tarantool
>
> local tap = require('tap')
> -local test = tap.test("string extensions")
> +local suite = tap.test("string extensions")
These changes introduce a mess in naming: in previous chunks you rename
<test> argument to <testcase>, but here you decided to change naming for
the root suite. I guess you need either to adjust it to the common
naming (<test> and <testcase>) or leave the shadowing, considering this
warning is raised in many other places and we have no policy for it for
now.
>
> -test:plan(7)
> +suite:plan(7)
>
> -test:test("split", function(test)
> +suite:test("split", function(test)
> test:plan(10)
>
> -- testing basic split (works over gsplit)
<snipped>
> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
> index 4d7059559..6c3cfd450 100755
> --- a/test/app-tap/tarantoolctl.test.lua
> +++ b/test/app-tap/tarantoolctl.test.lua
> @@ -4,7 +4,6 @@ local ffi = require('ffi')
> local fio = require('fio')
> local tap = require('tap')
> local uuid = require('uuid')
> -local yaml = require('yaml')
> local errno = require('errno')
> local fiber = require('fiber')
> local ok, test_run = pcall(require, 'test_run')
Well, the fact you simply didn't read my reply[1] drives me crazy. I'm
totally OK whether you *fix* the problem other way. I can handle the
fact you introduce unused variables and then suppress them. But ignoring
bug fix for this test is unacceptable for me. OK, I mention it one more
time.
**Here is the fix for the bug you successfully masked in the second
series in a row**:
================================================================================
diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua
index 6c3cfd450..6f5cd0bdd 100755
--- a/test/app-tap/tarantoolctl.test.lua
+++ b/test/app-tap/tarantoolctl.test.lua
@@ -32,7 +32,7 @@ local function recursive_rmdir(path)
end
end
if fio.rmdir(path) == false then
- print(string.format('!!! failed to rmdir path "%s"', file))
+ print(string.format('!!! failed to rmdir path "%s"', path))
print(string.format('!!! [errno %s]: %s', errno(), errno.strerror()))
end
end
================================================================================
It's a bug since there is no <file> variable in this scope and this line
is just a copy-paste from the loop body.
I'm out from the further review for this version and will wait the next
one to proceed.
<snipped>
> --
> 2.23.0
>
[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-April/016066.html
--
Best regards,
IM
More information about the Tarantool-patches
mailing list