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 378E4445320 for ; Fri, 7 Aug 2020 12:30:10 +0300 (MSK) Date: Fri, 7 Aug 2020 12:19:46 +0300 From: Igor Munkin Message-ID: <20200807091946.GX18920@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 00/19] Add static analysis of Lua code in test/ with luacheck 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, I see little changes changes from the previous version. However, I addressed the following comment to you in my last review[1]: |> > Sergey, |> > |> > I guess we definitely need another series for this. |> |> Ja-ja, will do. |> |> > Furthermore, IIRC we |> > decided to leave shadowing, so the current changes are simply not |> > relevant. Please adjust the patches related to tests considering our |> > recent discussion in cover letter and send a new patchset. |> |> I have reverted all changes related to globally supressed warnings. |> As I can see you claimed the irrelevant changes are reverted. Unfortunately, I still see excess hunks in the patch[2] (e.g. related to arguments and locals shadowing). Sorry, you *might* be misled by my wording, since I mentioned cover letter, not the Sasha's review for the previous patch[3]. OK, let's try one more time: please consider the results of the discussion we had related to source Lua files series. On 16.07.20, sergeyb@tarantool.org wrote: > From: Sergey Bronnikov > > Note: patch series for test/ was already sent to list in a thread with > static analysis for src/ and extra/. Igor M. started to review it there, see > [1], and then in private discussion we decided to split changes for > test/ and {src/,extra/} for two different patch series. As changes for src/ and > extra/ has been merged to master [2] I have rebased existed changes for > test/, applied changes suggested by Igor in the initial review and > reverted changes for warnings supressed globally in .luacheckrc. > > 1. https://lists.tarantool.org/pipermail/tarantool-patches/2020-June/017301.html > 2. https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018568.html > > Changelog v2: > > - reverted changes for diff-based tests > - reverted changes for suppressed warnings > - format .luacheckrc according to Lua style guide > - created GH issue for every ignored warning in a separated files > - make each commit atomic again - all changes in commits are related to > commit itself, not other commits > > GH issue: https://github.com/tarantool/tarantool/issues/4681 > GitHub branch: https://github.com/tarantool/tarantool/tree/ligurio/gh-4681-fix-luacheck-warnings-test > CI: https://gitlab.com/tarantool/tarantool/-/pipelines/167419608 > > Sergey Bronnikov (19): > Fix luacheck warnings in test/app-tap > Fix luacheck warnings in test/app > Fix luacheck warnings in test/box > Fix luacheck warnings in test/box-py > Fix luacheck warnings in test/box-tap > Fix luacheck warnings in test/engine > Fix luacheck warnings in test/engine_long > Fix luacheck warnings in test/long_run-py > Fix luacheck warnings in test/replication > Fix luacheck warnings in test/replication-py > Fix luacheck warnings in test/sql > Fix luacheck warnings in test/sql-tap > Fix luacheck warnings in test/swim > Fix luacheck warnings in test/vinyl > Fix luacheck warnings in test/wal_off > Fix luacheck warnings in test/xlog > Fix luacheck warnings in test/xlog-py > Add luacheck supression for luajit test > luajit: bump new version > > .luacheckrc | 195 ++++++- > test/app-tap/cfg.test.lua | 2 +- > test/app-tap/clock.test.lua | 4 +- > test/app-tap/console.test.lua | 14 +- > test/app-tap/console_lua.test.lua | 18 +- > 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 | 16 +- > test/app-tap/inspector.test.lua | 5 +- > test/app-tap/json.test.lua | 1 - > test/app-tap/logger.test.lua | 23 +- > test/app-tap/lua/serializer_test.lua | 46 +- > 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 +- > test/box-py/box.lua | 2 +- > test/box-tap/auth.test.lua | 20 +- > test/box-tap/cfg.test.lua | 34 +- > test/box-tap/cfgup.test.lua | 4 +- > test/box-tap/extended_error.test.lua | 11 +- > test/box-tap/feedback_daemon.test.lua | 4 +- > test/box-tap/gc.test.lua | 4 +- > test/box-tap/on_schema_init.test.lua | 4 +- > test/box-tap/schema_mt.test.lua | 22 +- > test/box-tap/session.storage.test.lua | 10 +- > test/box-tap/session.test.lua | 56 +- > test/box-tap/trigger_atexit.test.lua | 10 +- > test/box-tap/trigger_yield.test.lua | 10 +- > test/box/box.lua | 7 +- > 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 | 2 +- > 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/utils.lua | 10 +- > test/box/on_schema_init.lua | 8 +- > test/box/proxy.lua | 2 +- > test/box/tiny.lua | 2 +- > test/engine/box.lua | 8 +- > test/engine/conflict.lua | 6 +- > test/engine_long/suite.lua | 5 +- > test/long_run-py/lua/finalizers.lua | 7 +- > test/long_run-py/suite.lua | 7 +- > test/replication-py/master.lua | 2 +- > test/replication-py/panic.lua | 2 +- > test/replication-py/replica.lua | 4 - > test/replication/lua/fast_replica.lua | 9 +- > test/replication/lua/rlimit.lua | 2 +- > test/replication/master.lua | 2 +- > test/replication/master_quorum.lua | 4 +- > test/replication/on_replace.lua | 6 +- > test/replication/replica.lua | 4 +- > test/replication/replica_on_schema_init.lua | 4 +- > test/sql-tap/alter.test.lua | 4 +- > test/sql-tap/analyze5.test.lua | 2 +- > test/sql-tap/analyze9.test.lua | 22 +- > 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 | 11 +- > test/sql-tap/func.test.lua | 2 +- > test/sql-tap/func3.test.lua | 2 +- > test/sql-tap/gh-2723-concurrency.test.lua | 8 +- > .../gh-3083-ephemeral-unref-tuples.test.lua | 2 +- > .../gh-3307-xfer-optimization-issue.test.lua | 2 +- > .../gh-3332-tuple-format-leak.test.lua | 2 +- > .../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 | 11 +- > 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/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/swim/box.lua | 4 +- > test/vinyl/large.lua | 3 +- > test/vinyl/stress.lua | 22 +- > test/vinyl/txn_proxy.lua | 6 +- > test/vinyl/upgrade/fill.lua | 8 +- > test/vinyl/vinyl.lua | 17 - > test/xlog-py/box.lua | 2 +- > test/xlog/panic.lua | 2 +- > .../2.1.3/gh-4771-upgrade-sequence/fill.lua | 12 +- > test/xlog/xlog.lua | 2 +- > third_party/luajit | 2 +- > 143 files changed, 1135 insertions(+), 1051 deletions(-) > > -- > 2.26.2 > [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018632.html [2]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-July/018634.html [3]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-June/017304.html -- Best regards, IM