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 CFAA757AFDC; Fri, 18 Aug 2023 14:03:39 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org CFAA757AFDC DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1692356619; bh=krw20oMUvxPxxK+dFD5oy6fBRc8XkqGlI0+7GT+OjPY=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=boyX0KBUe7r8Mg13XEQ9cUgomVEf6jhU2JIS6AgPvN+Yf0+InSOkfO82x/JpyHYcr MYLXM+MDKkCIidEQExT5TbZCQNZdrdguLRGQLgne97c5cQS6SG3Ek/Z+nnD0G1lLtT 3Ap0ZAiDhspUg8aJI7lPbMEtdRXkncSxnWKt3l/E= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [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 919BC55C707 for ; Fri, 18 Aug 2023 14:03:38 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 919BC55C707 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1qWxGT-00055x-Pc; Fri, 18 Aug 2023 14:03:38 +0300 Date: Fri, 18 Aug 2023 13:58:50 +0300 To: Sergey Bronnikov Message-ID: References: <28c7aec4df761b06208cc7ccd9055dd444ed2a70.1692089299.git.skaplun@tarantool.org> <29d67ed7-fed4-28ed-8dde-8030189008d6@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <29d67ed7-fed4-28ed-8dde-8030189008d6@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD93C8852532D76B9E34B7F521CF8AAC5F31026AE04F73B8670182A05F53808504086778A2CAA52AC3FC2F82596D2CCA7AFEDBFA762220517975F70D973CC2CA7AB X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE79683A3C835791080EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637F63E14183F8C6AF98638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8EEC7BF48F2F375EBF1A971434E4AF254117882F4460429724CE54428C33FAD305F5C1EE8F4F765FC3EA24FE487C076E5A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F44604297287769387670735201E561CDFBCA1751FF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F7900637081D671B34B8BF6222CA9DD8327EE4930A3850AC1BE2E7356D8C47C27EEC5E9FB5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A58A01729824853018B4E2FD64F322FB23EAC31DC42B4D3D89F87CCE6106E1FC07E67D4AC08A07B9B0CF7CD7A0D5AA5F25CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFDB4A24B82677090965E0590318C19EBEF39EB39AF00C70275D2251BC87D69F0923277A7B3C315D4E24F7434E9608BF9F041CE845A646FD9F1E7F99B0C9222ADDE48CAC7CA610320002C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojgZIFrgxvv3pnlq2GIxJI5Q== X-DA7885C5: 9A0C9FEFCBD35EC6FDFFA1E4C49A78787767012EB2121EB46717EB10A6EA0FB6262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986B9886B4120415757D8557CC6D5FBF686C0FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 1/5] test: introduce `samevalues()` TAP checker 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the review! Please, consider my comments below. On 18.08.23, Sergey Bronnikov wrote: > Hi, Sergey > > > Thanks for the patch! See my comments inline. > > > On 8/15/23 12:36, Sergey Kaplun wrote: > > The introduced `samevalues()` helper checks that values in range from > > 1, to `table.maxn()` of the given table are exactly the same. It may be > > usefull for test consistency of JIT and VM behaviour. Originally, the > > `arr_is_consistent()` function was introduced in the > > . `samevalues()` has the same > > functionallity (except usage of `table.maxn()` instead `#` operator to > > be sure, that the table we check isn't a sparse array). > > I would rename samevalues to something like assert_equals or > assert_items_equals just because > > similar functions are named in unit testing frameworks and helpers with > prefix assert_ As you can see we use naming without _ for exported function in the module, so additional one with strange naming will be inconsistent. Also, discussed this naming with Igor and Max offline and this name is OK for them, feel free also to CC Igor to discuss:). > > more readable from my point of view. See names for assertions in luatest > [1] and JUnit (popular unit testing framework). > > > 1. https://github.com/tarantool/luatest#list-of-luatest-functions > > 2. > https://junit.org/junit5/docs/5.0.1/api/org/junit/jupiter/api/Assertions.html > > > --- > > test/tarantool-tests/gh-6163-min-max.test.lua | 52 ++++++++----------- > > test/tarantool-tests/tap.lua | 14 +++++ > > 2 files changed, 37 insertions(+), 29 deletions(-) > > > > diff --git a/test/tarantool-tests/gh-6163-min-max.test.lua b/test/tarantool-tests/gh-6163-min-max.test.lua > > index 63437955..4bc6155c 100644 > > --- a/test/tarantool-tests/gh-6163-min-max.test.lua > > +++ b/test/tarantool-tests/gh-6163-min-max.test.lua > > @@ -2,25 +2,17 @@ local tap = require('tap') > > local test = tap.test('gh-6163-jit-min-max'):skipcond({ > > ['Test requires JIT enabled'] = not jit.status(), > > }) > > + > Nit: Unneeded change. The code with additional local variable `DUMMY_TABLE` becomes less readable, so I added the empty line. Ignoring. > > > > > > diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua > > index 8559ee52..af1d4b20 100644 > > --- a/test/tarantool-tests/tap.lua > > +++ b/test/tarantool-tests/tap.lua > > @@ -254,6 +254,19 @@ local function iscdata(test, v, ctype, message, extra) > > return ok(test, ffi.istype(ctype, v), message, extra) > > end > > > > +local function isnan(v) > > + return v ~= v > > +end > > + > > I would put isnan() to utils, because it is not related to TAP module > actually and Don't agree here, since module should be self-sufficient (as it is now), so it doesn't require additional modules like utils. > > can be useful for other tests too and exporting it in tap module will be Opposite: it's OK to export it from this module within a check, since we may want to check some value is NaN or not. > strange. > > > > +local function samevalues(test, got, message, extra) > > + for i = 1, table.maxn(got) - 1 do > > + if got[i] ~= got[i + 1] and not (isnan(got[i]) and isnan(got[i + 1])) then > > + return fail(test, message, extra) > > + end > > + end > > + return ok(test, true, message, extra) > > +end > > + > > local test_mt > > > > local function new(parent, name, fun, ...) > > @@ -372,6 +385,7 @@ test_mt = { > > isudata = isudata, > > iscdata = iscdata, > > is_deeply = is_deeply, > > + samevalues = samevalues, > > like = like, > > unlike = unlike, > > } -- Best regards, Sergey Kaplun