From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 466B2469710 for ; Mon, 1 Jun 2020 14:49:28 +0300 (MSK) Date: Mon, 1 Jun 2020 14:41:01 +0300 From: Igor Munkin Message-ID: <20200601114101.GR21558@tarantool.org> References: <2fe2a41b8028623b6e5d72c59be716b81dce1e37.1590764167.git.sergeyb@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <2fe2a41b8028623b6e5d72c59be716b81dce1e37.1590764167.git.sergeyb@tarantool.org> Subject: Re: [Tarantool-patches] [PATCH v6 08/25] Fix luacheck warnings in test/app-tap List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: sergeyb@tarantool.org Cc: o.piskunov@tarantool.org, tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org, alexander.turenko@tarantool.org Sergey, Thanks, the patch LGTM, except the couple of comments below. On 29.05.20, sergeyb@tarantool.org wrote: > From: Sergey Bronnikov > > Part of #4681 > > Reviewed-by: Vladislav Shpilevoy > Reviewed-by: Igor Munkin > > Co-authored-by: Vladislav Shpilevoy > Co-authored-by: Igor Munkin > --- > .luacheckrc | 3 +- > test/app-tap/cfg.test.lua | 2 +- > test/app-tap/clock.test.lua | 4 +- > test/app-tap/console.test.lua | 16 +- > 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 | 24 +- > test/app-tap/fail_main.test.lua | 6 +- > .../gh-4761-json-per-call-options.test.lua | 11 +- > test/app-tap/http_client.test.lua | 297 +++++------ > test/app-tap/iconv.test.lua | 6 +- > test/app-tap/init_script.test.lua | 14 +- > test/app-tap/inspector.test.lua | 5 +- > test/app-tap/json.test.lua | 1 - > test/app-tap/logger.test.lua | 21 +- > test/app-tap/lua/serializer_test.lua | 44 +- > test/app-tap/minimal.test.lua | 4 +- > test/app-tap/module_api.test.lua | 12 +- > test/app-tap/msgpackffi.test.lua | 3 +- > test/app-tap/pcall.test.lua | 6 +- > test/app-tap/popen.test.lua | 35 +- > test/app-tap/snapshot.test.lua | 17 +- > test/app-tap/string.test.lua | 502 +++++++++--------- > test/app-tap/tap.test.lua | 26 +- > test/app-tap/tarantoolctl.test.lua | 82 ++- > test/app-tap/trigger.test.lua | 48 +- > test/app-tap/yaml.test.lua | 16 +- > 28 files changed, 630 insertions(+), 663 deletions(-) > > diff --git a/.luacheckrc b/.luacheckrc > index 78bb54adb..630834e16 100644 > --- a/.luacheckrc > +++ b/.luacheckrc > @@ -7,7 +7,6 @@ exclude_files = { > "build/**/*.lua", > "src/box/lua/serpent.lua", -- third-party source code > "test/app/*.test.lua", > - "test/app-tap/lua/serializer_test.lua", This occurrence still exists on the branch. > "test/box/**/*.lua", > "test/engine/*.lua", > "test/engine_long/*.lua", > diff --git a/test/app-tap/lua/serializer_test.lua b/test/app-tap/lua/serializer_test.lua > index 2a668f898..fdbfb3b24 100644 > --- a/test/app-tap/lua/serializer_test.lua > +++ b/test/app-tap/lua/serializer_test.lua > @@ -483,21 +473,21 @@ local function test_decode_buffer(test, s) > test:plan(#cases) > > for _, case in ipairs(cases) do > - test:test(case[1], function(test) > - test:plan(4) > + test:test(case[1], function(testcase) > + testcase:plan(4) > local args_len = table.maxn(case.args) > local res, res_buf = case.func(unpack(case.args, 1, args_len)) > - test:is_deeply(res, case.exp_res, 'verify result') > + testcase:is_deeply(res, case.exp_res, 'verify result') > local buf = case.args[1] > local rewind = res_buf - buf > - test:is(rewind, case.exp_rewind, 'verify resulting buffer') > + testcase:is(rewind, case.exp_rewind, 'verify resulting buffer') > -- test:iscdata() is not sufficient here, because it Please, adjust the comment, considering the changes you introduced around here. > -- ignores 'const' qualifier (because of using > -- ffi.istype()). > - test:is(type(res_buf), 'cdata', 'verify resulting buffer type') > + testcase:is(type(res_buf), 'cdata', 'verify resulting buffer type') > local buf_ctype = tostring(ffi.typeof(buf)) > local res_buf_ctype = tostring(ffi.typeof(res_buf)) > - test:is(res_buf_ctype, buf_ctype, 'verify resulting buffer ctype') > + testcase:is(res_buf_ctype, buf_ctype, 'verify resulting buffer ctype') > end) > end > end > -- > 2.23.0 > -- Best regards, IM