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 53A7F7030D; Wed, 24 Feb 2021 00:26:03 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 53A7F7030D DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1614115563; bh=n+hHNUP9hXfWoObsARoDWyy9eMFaFKK7DjhRjVBnPmE=; 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=haRjbBSx5x7e1g258xhYu8HVNkpDTgyZP/e226jEo1+kXh1a0k+Yj2KS0FGKxiNIK C1hahdYNj2G1gqHzVU39u+SuDfCrX7EDV85z6x8Usb18nihQnqi0fwxSmuezWR1nGh oRDnI/mGdkUWJ9+LScu4T5dNM0MEyfW4LbIc8KI0= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [94.100.177.105]) (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 D43DE70202 for ; Wed, 24 Feb 2021 00:26:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D43DE70202 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1lEfBv-0001b1-TX; Wed, 24 Feb 2021 00:26:00 +0300 To: Sergey Bronnikov , tarantool-patches@dev.tarantool.org References: <3a5360daf42cd920e3ce536020a6ff2fbccc095e.1611232655.git.sergeyb@tarantool.org> <140f1c8e-dc25-3295-9bd8-e7835e2e89c8@tarantool.org> Message-ID: Date: Tue, 23 Feb 2021 22:25:58 +0100 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:78.0) Gecko/20100101 Thunderbird/78.7.1 MIME-Version: 1.0 In-Reply-To: Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD975C3EC174F56692254B0AABE1FB071B2BA6557555153D6A0182A05F538085040927FC1460D006BADB4C24445A72D8FBBACC4643EF3F6273645311A52961A5E0B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE79145AB6E9E75F07EEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637B24541F05F0BFC9F8638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FCE3B71345910BC919299F6C39D480167F80C0C9DE34D1ABDA389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C07E7E81EEA8A9722B8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B652D31B9D28593E51CC7F00164DA146DA6F5DAA56C3B73B237318B6A418E8EAB8D32BA5DBAC0009BE9E8FC8737B5C224966D89E201A891B4676E601842F6C81A12EF20D2F80756B5F7E9C4E3C761E06A776E601842F6C81A127C277FBC8AE2E8B2FD16FCC8DB5F8BED81D268191BDAD3D698AB9A7B718F8C442539A7722CA490C13377AFFFEAFD26923F8577A6DFFEA7CB24F08513AFFAC7993EC92FD9297F6715571747095F342E857739F23D657EF2BD5E8D9A59859A8B6E6D0BC7EED730BD2089D37D7C0E48F6C5571747095F342E857739F23D657EF2B6825BDBE14D8E7025EC15B47EAE72BACBD9CCCA9EDD067B1EDA766A37F9254B7 X-B7AD71C0: 14C14B24D00AF5AC321EF223B8115265C69B993890792DF82CDD5689AFBDA7A24A6D60772A99906F8E1CD14B953EB46DF395F339BBC3234C355D89D7DBCDD132 X-C1DE0DAB: 0D63561A33F958A5849A0B640B7E09DC6559FBEA19C7FCFDA6AAC97BA5D40A54D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA7557E988E9157162368E8E86DC7131B365E7726E8460B7C23C X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34987A0E8D235BB534414121EB083D7C3B7B549514F8A1148D83E7DAAD3A93934E806A30A96D3DA7971D7E09C32AA3244CFF20D14BAE61B1A7A16704CDE2F24F3030452B15D76AEC14FACE5A9C96DEB163 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojyK6JYJ15DtK3GVeOtqhL+A== X-Mailru-Sender: 504CC1E875BF3E7D9BC0E5172ADA311007457E538FC37C8976F4130B2D39101CCCCBACDFA816D2E307784C02288277CA03E0582D3806FB6A5317862B1921BA260ED6CFD6382C13A6112434F685709FCF0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH v8 03/14] test: fix luacheck warnings W113 in test/sql-tap 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: Vladislav Shpilevoy via Tarantool-patches Reply-To: Vladislav Shpilevoy Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi! Thanks for the patch! We are almost there. See 3 comments below. > diff --git a/test/sql-tap/badutf1.test.lua b/test/sql-tap/badutf1.test.lua > index 654629bf7..f375fea0f 100755 > --- a/test/sql-tap/badutf1.test.lua > +++ b/test/sql-tap/badutf1.test.lua > @@ -93,8 +93,12 @@ test:do_test( > }) > > -- commented as it uses utf16 > -if 0>0 then > -sql("db2", "") > +-- testcases are broken > +-- https://github.com/tarantool/tarantool/issues/5743 > +local is_gh_5743_closed = false > +if is_gh_5743_closed then > +sql("db2", "") -- luacheck: ignore sql > +local sql_exec = nil >> For me it's not make sense. Replaced push/pop suppression with variable definition. 1. Lets at least be consistent and either use `-- luacheck` comments or `= nil` hack. Not both, especially not on 2 adjacent lines. It simply raises unnecessary questions why couldn't it be done in one way. > test:do_test( > "badutf-1.10", > function()> diff --git a/test/sql-tap/e_select1.test.lua b/test/sql-tap/e_select1.test.lua > index 2a189582d..9ef5e7925 100755 > --- a/test/sql-tap/e_select1.test.lua > +++ b/test/sql-tap/e_select1.test.lua > @@ -1537,32 +1561,26 @@ test:do_select_tests( > -- considered equal to other NULL values and distinct from all non-NULL > -- values. > -- > --- MUST_WORK_TEST > -if 0>0 then > -db("nullvalue", "null") > test:do_select_tests( > "e_select-7.9", > { > - {"1", "SELECT NULL UNION ALL SELECT NULL", {null, null}}, > - {"2", "SELECT NULL UNION SELECT NULL", {null}}, > - {"3", "SELECT NULL INTERSECT SELECT NULL", {null}}, > + {"1", "SELECT NULL UNION ALL SELECT NULL", {"", ""}}, > + {"2", "SELECT NULL UNION SELECT NULL", {""}}, > + {"3", "SELECT NULL INTERSECT SELECT NULL", {""}}, > {"4", "SELECT NULL EXCEPT SELECT NULL", {}}, 2. But why? What was the effect of `db("nullvalue", "null")` in the original tests? If you don't know then maybe it is worth leaving it as is + adding `local null = box.NULL` or `local null = nil` so at least we could preserve the test body unchanged for future investigations. To me it is not clear why null can be "". Empty string is not the same as NULL. The same for the other `db("nullvalue", "null")` removals where `null` was used for anything. If there are such other places. > - {"5", "SELECT NULL UNION ALL SELECT 'ab'", {null, "ab"}}, > - {"6", "SELECT NULL UNION SELECT 'ab'", {null, "ab"}}, > + {"5", "SELECT NULL UNION ALL SELECT 'ab'", {"", "ab"}}, > + {"6", "SELECT NULL UNION SELECT 'ab'", {"", "ab"}}, > {"7", "SELECT NULL INTERSECT SELECT 'ab'", {}}, > - {"8", "SELECT NULL EXCEPT SELECT 'ab'", {null}}, > - {"9", "SELECT NULL UNION ALL SELECT 0", {null, 0}}, > - {"10", "SELECT NULL UNION SELECT 0", {null, 0}}, > + {"8", "SELECT NULL EXCEPT SELECT 'ab'", {""}}, > + {"9", "SELECT NULL UNION ALL SELECT 0", {"", 0}}, > + {"10", "SELECT NULL UNION SELECT 0", {"", 0}}, > {"11", "SELECT NULL INTERSECT SELECT 0", {}}, > - {"12", "SELECT NULL EXCEPT SELECT 0", {null}}, > - {"13", "SELECT c FROM q1 UNION ALL SELECT g FROM q3", {null, -42.47, "null", 2, 2}}, > - {"14", "SELECT c FROM q1 UNION SELECT g FROM q3", {null, -42.47, 2}}, > + {"12", "SELECT NULL EXCEPT SELECT 0", {""}}, > + {"13", "SELECT c FROM q1 UNION ALL SELECT g FROM q3", {"", -42.47, "", 2, 2}}, > + {"14", "SELECT c FROM q1 UNION SELECT g FROM q3", {"", -42.47, 2}}, > {"15", "SELECT c FROM q1 INTERSECT SELECT g FROM q3", {}}, > - {"16", "SELECT c FROM q1 EXCEPT SELECT g FROM q3", {null, -42.47}}, > + {"16", "SELECT c FROM q1 EXCEPT SELECT g FROM q3", {"", -42.47}}, > }) > - > -db("nullvalue", "") > -end > diff --git a/test/sql-tap/index1.test.lua b/test/sql-tap/index1.test.lua > index ea25727a4..574faafd6 100755 > --- a/test/sql-tap/index1.test.lua > +++ b/test/sql-tap/index1.test.lua > @@ -585,7 +585,7 @@ test:do_test( > for i = 1, 50, 1 do > test:execsql(string.format("INSERT INTO t3 VALUES('x%sx',%s,0.%s)", i, i, i)) > end > - local sql_search_count = 0 > + -- luacheck: ignore X 3. Lets be consistent and not jump from one method to another. We either use luacheck comments or `local X = nil` hacks. The same for other similar inconsistencies in this and the rest of the patches. > return X(381, "X!cmd", [=[["concat",[["execsql","SELECT c FROM t3 WHERE b==10"]],["sql_search_count"]]]=]) > end, { > --