From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtpng3.m.smailru.net (smtpng3.m.smailru.net [94.100.177.149]) (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 A6A98445320 for ; Mon, 20 Jul 2020 22:57:12 +0300 (MSK) Date: Mon, 20 Jul 2020 22:46:53 +0300 From: Igor Munkin Message-ID: <20200720194653.GI18920@tarantool.org> References: MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: Subject: Re: [Tarantool-patches] [PATCH 18/19] Add luacheck supression for luajit test List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: sergeyb@tarantool.org Cc: tarantool-patches@dev.tarantool.org, v.shpilevoy@tarantool.org, alexander.turenko@tarantool.org Sergey, Here we go again... I wonder how exactly you tested LuaJIT related patch if you ended up suppressing the same warning both inline and within .luacheckrc. Consider the following: | $ git diff | cat | $ make luacheck | grep -F 'luajit' | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-3196-incorrect-string-length.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-4427-ffi-sandwich.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-4476-fix-string-find-recording.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-4773-tonumber-fail-on-NUL-char.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-494-table-chain-infinite-loop.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-505-fold-no-strref-for-ptrdiff.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-524-fold-conv-respect-src-irt.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-flush-on-trace.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/or-232-unsink-64-kptr.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/utils.lua OK | $ git stash pop | $ git diff | cat | diff --git a/.luacheckrc b/.luacheckrc | index 21680c08c..2bc2eaa33 100644 | --- a/.luacheckrc | +++ b/.luacheckrc | @@ -171,12 +171,6 @@ files["test/long_run-py/suite.lua"] = { | "delete_insert", | } | } | -files["test/luajit-tap/or-232-unsink-64-kptr.test.lua"] = { | - ignore = { | - -- An empty if branch. | - "542", | - } | -} | files["test/replication/replica_quorum.lua"] = { | globals = { | "INSTANCE_URI", | $ make luacheck | grep -F 'luajit' | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-3196-incorrect-string-length.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-4427-ffi-sandwich.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-4476-fix-string-find-recording.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/gh-4773-tonumber-fail-on-NUL-char.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-494-table-chain-infinite-loop.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-505-fold-no-strref-for-ptrdiff.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-524-fold-conv-respect-src-irt.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/lj-flush-on-trace.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/or-232-unsink-64-kptr.test.lua OK | Checking /home/imun/projects/tarantool-review/test/luajit-tap/utils.lua OK So what does this suppression fix? Actually nothing. Please drop this suppression from .luacheckrc (and thereby the patch itself). By the way, I see another problem: - Wait, it's all just luacheck failing on luajit tests? | $ git co HEAD^ | $ git log --oneline -1 | cat | c9653cbec Add luacheck supression for luajit test | $ make luacheck || echo "Always has been" | | Total: 95 warnings / 0 errors in 492 files | make[3]: *** [CMakeFiles/luacheck.dir/build.make:58: luacheck] Error 1 | make[2]: *** [CMakeFiles/Makefile2:430: CMakeFiles/luacheck.dir/all] Error 2 | make[1]: *** [CMakeFiles/Makefile2:437: CMakeFiles/luacheck.dir/rule] Error 2 | make: *** [Makefile:316: luacheck] Error 2 | Always has been I guess the agreement to make the series passing after *every* patch still stands (like you made for src/ and extra/ directories). I doubt we should violate this rule for test/. On 16.07.20, sergeyb@tarantool.org wrote: > From: Sergey Bronnikov > > Part of #4681 > > Reviewed-by: Igor Munkin > --- > .luacheckrc | 6 ++++++ > 1 file changed, 6 insertions(+) > > diff --git a/.luacheckrc b/.luacheckrc > index 2bc2eaa33..21680c08c 100644 > --- a/.luacheckrc > +++ b/.luacheckrc > -- > 2.26.2 > -- Best regards, IM