From: Sergey Bronnikov <sergeyb@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>, tarantool-patches@dev.tarantool.org, lvasiliev@tarantool.org Subject: Re: [Tarantool-patches] [PATCH v7 1/3] luacheck: fix warnings in test/app-tap Date: Mon, 21 Dec 2020 13:02:27 +0300 [thread overview] Message-ID: <2e9084fc-3420-2f97-d2bb-de08880f08d1@tarantool.org> (raw) In-Reply-To: <92a5b303-4060-35c0-d72f-180b6cc9f744@tarantool.org> Hi! Thanks for review! On 20.12.2020 16:59, Vladislav Shpilevoy wrote: > Hi! Thanks for the patch! > > See 5 comments below. > >> diff --git a/.luacheckrc b/.luacheckrc >> index 29db9eeeb..ae96576ec 100644 >> --- a/.luacheckrc >> +++ b/.luacheckrc >> @@ -102,3 +103,9 @@ files["src/box/lua/console.lua"] = { >> "212", >> } >> } >> +files["test/app-tap/lua/require_mod.lua"] = { >> + globals = {"exports"} > 1. In require_mod.lua the variable 'exports' should be declared 'local'. > It is never used out of this file. Fixed in a branch and force-pushed. --- a/test/app-tap/lua/require_mod.lua +++ b/test/app-tap/lua/require_mod.lua @@ -1,4 +1,4 @@ -exports = {} +local exports = {} function exports.test(a, b) return a+b --- a/.luacheckrc +++ b/.luacheckrc @@ -1,5 +1,5 @@ std = "luajit" -globals = {"box", "_TARANTOOL", "tonumber64"} +globals = {"box", "_TARANTOOL", "tonumber64", "utf8"} ignore = { -- Accessing an undefined field of a global variable <debug>. "143/debug", @@ -102,12 +102,6 @@ files["src/box/lua/console.lua"] = { "212", } } -files["test/app-tap/lua/require_mod.lua"] = { - globals = {"exports"} -} files["test/app/lua/fiber.lua"] = { globals = {"box_fiber_run_test"} } > >> +} >> +files["test/app-tap/string.test.lua"] = { >> + globals = {"utf8"} > 2. utf8 is global everywhere, it is defined by tarantool executable. > It should be in the root 'globals' list, among 'box', '_TARANTOOL', etc. I think it was suppressed for single file because it is now used only there. Fixed in a branch and force-pushed. --- a/.luacheckrc +++ b/.luacheckrc @@ -102,12 +102,6 @@ files["src/box/lua/console.lua"] = { "212", } } -files["test/app-tap/string.test.lua"] = { - globals = {"utf8"} -} files["test/app/lua/fiber.lua"] = { globals = {"box_fiber_run_test"} } > >> 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..c6b31ba61 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 >> @@ -25,7 +25,7 @@ local res = tap.test('gh-4761-json-per-call-options', function(test) >> >> -- Same check for json.encode. >> local nan = 1/0 >> - local ok, err = pcall(json.encode, {a = nan}, >> + local ok = pcall(json.encode, {a = nan}, >> {encode_invalid_numbers = false}) > 3. This line is misaligned now. The same above. Fixed in a branch and force-pushed. --- a/test/app-tap/gh-4761-json-per-call-options.test.lua +++ b/test/app-tap/gh-4761-json-per-call-options.test.lua @@ -14,7 +14,7 @@ local res = tap.test('gh-4761-json-per-call-options', function(test) -- Preparation code: call :decode() with a custom option. local ok = pcall(json.decode, '{"foo": {"bar": 1}}', - {decode_max_depth = 1}) + {decode_max_depth = 1}) assert(not ok, 'expect "too many nested data structures" error') -- Verify that the instance option remains unchanged. @@ -26,7 +26,7 @@ local res = tap.test('gh-4761-json-per-call-options', function(test) -- Same check for json.encode. local nan = 1/0 local ok = pcall(json.encode, {a = nan}, - {encode_invalid_numbers = false}) + {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}) > >> assert(not ok, 'expected "number must not be NaN or Inf" error') >> local exp_res = '{"a":inf}' >> diff --git a/test/app-tap/tarantoolctl.test.lua b/test/app-tap/tarantoolctl.test.lua >> index 9f1464617..44357484b 100755 >> --- a/test/app-tap/tarantoolctl.test.lua >> +++ b/test/app-tap/tarantoolctl.test.lua >> @@ -97,23 +96,14 @@ local function tctl_wait_start(dir, name) >> fiber.sleep(0.01) >> end >> ::again:: >> - while true do >> - local stat, nb = pcall(require('net.box').new, path, { >> - wait_connected = true, console = true >> - }) >> - if stat == false then >> - fiber.sleep(0.01) >> - goto again >> - else >> - break >> - end >> - local stat, msg = pcall(nb.eval, nb, 'require("fiber").time()') >> - if stat == false then >> - fiber.sleep(0.01) >> - else >> - break >> - end >> + local stat, _ = pcall(require('net.box').new, path, { >> + wait_connected = true, console = true >> + }) >> + if stat == false then >> + fiber.sleep(0.01) >> + goto again >> end >> + return > 4. 'return' is not necessary. Agree, fixed in a branch and force-pushed. --- a/test/app-tap/tarantoolctl.test.lua +++ b/test/app-tap/tarantoolctl.test.lua @@ -103,7 +103,6 @@ local function tctl_wait_start(dir, name) fiber.sleep(0.01) goto again end - return end end >> end >> end >> diff --git a/test/app-tap/trigger.test.lua b/test/app-tap/trigger.test.lua >> index a31d45e5f..6cf3fbe53 100755 >> --- a/test/app-tap/trigger.test.lua >> +++ b/test/app-tap/trigger.test.lua >> @@ -45,7 +45,7 @@ test:test("simple trigger test", function(test) >> >> >> -- Check that we've failed to delete trigger >> - local stat, err = pcall(getmetatable(trigger_list).__call, trigger_list, >> + local _, err = pcall(getmetatable(trigger_list).__call, trigger_list, >> nil, trigger_cnt) > 5. This line became misaligned. Fixed in a branch and force-pushed. --- a/test/app-tap/trigger.test.lua +++ b/test/app-tap/trigger.test.lua @@ -46,7 +46,7 @@ test:test("simple trigger test", function(test) -- Check that we've failed to delete trigger local _, err = pcall(getmetatable(trigger_list).__call, trigger_list, - nil, trigger_cnt) + nil, trigger_cnt) test:ok(string.find(err, "is not found"), "check error") end)
next prev parent reply other threads:[~2020-12-21 10:02 UTC|newest] Thread overview: 9+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-12-16 15:59 [Tarantool-patches] [PATCH v7 0/3] Fix luacheck warnings in test/app-tap and test/app sergeyb 2020-12-16 16:00 ` [Tarantool-patches] [PATCH v7 1/3] luacheck: fix warnings in test/app-tap sergeyb 2020-12-20 13:59 ` Vladislav Shpilevoy 2020-12-21 10:02 ` Sergey Bronnikov [this message] 2020-12-16 16:00 ` [Tarantool-patches] [PATCH v7 2/3] luacheck: fix warnings in test/app sergeyb 2020-12-20 13:59 ` Vladislav Shpilevoy 2020-12-21 11:00 ` Sergey Bronnikov 2020-12-16 16:00 ` [Tarantool-patches] [PATCH v7 3/3] luacheck: remove unneeded comment sergeyb 2020-12-21 17:17 ` [Tarantool-patches] [PATCH v7 0/3] Fix luacheck warnings in test/app-tap and test/app Vladislav Shpilevoy
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=2e9084fc-3420-2f97-d2bb-de08880f08d1@tarantool.org \ --to=sergeyb@tarantool.org \ --cc=lvasiliev@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH v7 1/3] luacheck: fix warnings in test/app-tap' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox