From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 6BF7C4C883; Thu, 14 Jan 2021 11:13:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6BF7C4C883 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1610612038; bh=4x98tiIVlj6rYCMfkF/f1yl/5PWfPgjrVDM7/ShsgOY=; h=To:References:Date:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=vNXqNHA6MN7X+PNVCbyvpGMqRBZqVF66BHaNYMKlFhsScQHSjg4YygeIKtQDukDDh VDsHlZvHMaaqcy/1uTl8hxt7R3aHH0AY3fJ+ogWSQQZKDQ0Oqr0b57wuGKjCWK9fY2 YAxNA5wNN+jj78UtNxNZU6v/szlqDJeHu4j01e5I= Received: from smtp30.i.mail.ru (smtp30.i.mail.ru [94.100.177.90]) (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 826824C883 for ; Thu, 14 Jan 2021 11:13:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 826824C883 Received: by smtp30.i.mail.ru with esmtpa (envelope-from ) id 1kzxlT-0007gf-51; Thu, 14 Jan 2021 11:13:55 +0300 To: Vladislav Shpilevoy , tarantool-patches@dev.tarantool.org References: <1b302a98-cd94-bee3-ee7d-2e64f328bfde@tarantool.org> Message-ID: <6fd57789-56ce-6ce3-fc95-f946beda5bee@tarantool.org> Date: Thu, 14 Jan 2021 11:13:54 +0300 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:68.0) Gecko/20100101 Thunderbird/68.10.0 MIME-Version: 1.0 In-Reply-To: <1b302a98-cd94-bee3-ee7d-2e64f328bfde@tarantool.org> Content-Type: text/plain; charset=utf-8; format=flowed Content-Transfer-Encoding: 8bit Content-Language: en-US X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9D0E79FBC973162CD3008320BFA2F4C653BABCE9B72A1DEA600894C459B0CD1B93D38FEE15389723DA34864764734650E03094DDDF269A914C9EA2F84C7A79465 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75A6765C746F51968EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063725D748B084CAA27D8638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FC9EF150C440D65FAE22E484FB87FAA9C29CE32CD7744F99A1389733CBF5DBD5E913377AFFFEAFD269A417C69337E82CC2CC7F00164DA146DAFE8445B8C89999729449624AB7ADAF37F6B57BC7E64490611E7FA7ABCAF51C92A417C69337E82CC2CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C2249B880EA3D0D04DCFA76E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8BE6A2F950A10922ED3AA81AA40904B5D9DBF02ECDB25306B2B25CBF701D1BE8734AD6D5ED66289B5278DA827A17800CE729B57AE5D7E6633B67F23339F89546C5A8DF7F3B2552694A6FED454B719173D6725E5C173C3A84C3397F69CF5FE7744E75ECD9A6C639B01B4E70A05D1297E1BBC6867C52282FAC85D9B7C4F32B44FF57285124B2A10EEC6C00306258E7E6ABB4E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A5E625400CDA1F7D14038826F8533A2630D5A83E40B350A1C0D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA758F9E841AEAEC4F2C410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34C2E47CA9A560609072DEC6D45D4E993DC9FDBE53D8C938989F9378AE54607FA6C6F96FCB45D95EE71D7E09C32AA3244C3A42E997C85BF15CFAB7BF43D18F785D33C9DC155518937FFACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj9kdO2HH36x4IPYz540bEpg== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F2588458752B4C09A5B47111ACED8C3DE20DA3A3C62B1D22670AE450282EC151BADDC1D3523A6D01B4765B2DFB59E2DDD9FE06B14FA522850F29BC30112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v7] test: fix luacheck warnings in test/long_run-py X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Thanks for review! On 13.01.2021 20:04, Vladislav Shpilevoy wrote: > Thanks for the patch! > > See 3 comments below. > > On 13.01.2021 15:35, sergeyb@tarantool.org wrote: >> From: Sergey Bronnikov >> >> Closes #5460 >> >> Reviewed-by: Vladislav Shpilevoy >> Reviewed-by: Igor Munkin >> >> Co-authored-by: Vladislav Shpilevoy >> Co-authored-by: Igor Munkin >> --- >> >> Changelog v7: >> >> - updated an exclusion mask in .luacheckrc >> >> Changelog v6: >> >> - splitted patch in test/ for patches per sub-directory >> - adjusted supressions in .luacheckrc >> - fixed formatting issues in .luacheckrc >> >> Gitlab CI: https://gitlab.com/tarantool/tarantool/-/pipelines/241108315 >> Issue: https://github.com/tarantool/tarantool/issues/5460 >> Branch: ligurio/gh-5460-luacheck-warnings-test-long_run-py >> >> .luacheckrc | 2 +- >> test/long_run-py/lua/finalizers.lua | 8 +++----- >> test/long_run-py/suite.lua | 16 +++++++++------- >> 3 files changed, 13 insertions(+), 13 deletions(-) >> >> diff --git a/.luacheckrc b/.luacheckrc >> index 4b829f3dc..68736d8db 100644 >> --- a/.luacheckrc >> +++ b/.luacheckrc >> @@ -37,7 +37,7 @@ exclude_files = { >> "test/box-tap/**/*.lua", >> "test/engine/**/*.lua", >> "test/engine_long/**/*.lua", >> - "test/long_run-py/**/*.lua", >> + "test/long_run-py/lua/finalizers.lua", > 1. Why did you leave it in 'exclude' and yet you fix it? Change in a file was done to fix luacheck warning, but I didn't know how to fix other warnings so decided to exclude it at all. > I tried to remove it, and found that it says your change > in the next hunk is not correct. When I fixed it, I saw it > says the 'result' variable is not used but only mutated. > > Then I tried to 'read' result somehow after the loop, but > it says the code is unreachable. Then I made the loop look > like it can end, and now it seems to be working. > > ==================== > diff --git a/.luacheckrc b/.luacheckrc > index 68736d8db..3f4913df3 100644 > --- a/.luacheckrc > +++ b/.luacheckrc > @@ -37,7 +37,6 @@ exclude_files = { > "test/box-tap/**/*.lua", > "test/engine/**/*.lua", > "test/engine_long/**/*.lua", > - "test/long_run-py/lua/finalizers.lua", > "test/luajit-tap/**/*.lua", > "test/replication/**/*.lua", > "test/replication-py/**/*.lua", > diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua > index cb6400363..88bd7734c 100644 > --- a/test/long_run-py/lua/finalizers.lua > +++ b/test/long_run-py/lua/finalizers.lua > @@ -7,10 +7,14 @@ local function test_finalizers() > local result = {} > local i = 1 > local ffi = require('ffi') > - while true do > - local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc) > + while i ~= 0 do > + result[i] = ffi.gc(ffi.cast('void *', 0), on_gc) > i = i + 1 > end > + -- Fake-read 'result' to calm down 'luacheck' complaining that the variable > + -- is never used. > + assert(#result ~= 0) > + return "done" > end; > > test_finalizers() > ==================== > > It looks a bit ugly maybe, because i ~= 0 is always true, and because > there is unreachable code after the loop. > > I suggest to either find a better way to fix the warning; or don't change > finalizers.lua at all and keep the whole file ignored; or apply my diff, > or revert your 'local' usage in the next diff hunk, and silence this single > warning about 'result' being not used. Agree, it's looks a bit ugly, although I think it is better than keep suppression in .luacheckrc for the whole file. So I applied your change and squashed to commit. Thanks. >> "test/luajit-tap/**/*.lua", >> "test/replication/**/*.lua", >> "test/replication-py/**/*.lua", >> diff --git a/test/long_run-py/lua/finalizers.lua b/test/long_run-py/lua/finalizers.lua >> index 69146a323..cb6400363 100644 >> --- a/test/long_run-py/lua/finalizers.lua >> +++ b/test/long_run-py/lua/finalizers.lua >> @@ -1,19 +1,17 @@ >> #!/usr/bin/env tarantool >> >> -function on_gc(t) >> +local function on_gc() >> end; >> >> -function test_finalizers() >> +local function test_finalizers() >> local result = {} >> local i = 1 >> local ffi = require('ffi') >> while true do >> - result[i] = ffi.gc(ffi.cast('void *', 0), on_gc) >> + local result[i] = ffi.gc(ffi.cast('void *', 0), on_gc) > 2. This change is not correct. Even luacheck tells it, if you don't > ignore this file. You assign a value to a table member, not > declare a variable. It is still not unclear for me why luacheck complains here. result table declared before a loop and scope for it is a whole function body, what's wrong? Similar lua chunk successfully executed by puc lua: local result = {} local i = 0 while i == 0 do     result[i] = 1 end > >> i = i + 1 >> end >> - return "done" >> end; >> >> test_finalizers() >> test_finalizers() >> - >> diff --git a/test/long_run-py/suite.lua b/test/long_run-py/suite.lua >> index 0b33dec7d..7a09dd2b8 100644 >> --- a/test/long_run-py/suite.lua >> +++ b/test/long_run-py/suite.lua >> @@ -109,3 +106,8 @@ function delete_insert(engine_name) >> box.space.tester:drop() >> return {counter, string_value_2} >> end >> + >> +return { >> + delete_replace_update = delete_replace_update; >> + delete_insert = delete_insert; > 3. Please, use ',' instead of ';'. > Fixed in a branch. --- a/test/long_run-py/suite.lua +++ b/test/long_run-py/suite.lua @@ -108,6 +108,6 @@ local function delete_insert(engine_name)  end  return { -    delete_replace_update = delete_replace_update; -    delete_insert = delete_insert; +    delete_replace_update = delete_replace_update, +    delete_insert = delete_insert,  } >> +} >>