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 08ACC6EC59; Wed, 10 Mar 2021 13:43:32 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 08ACC6EC59 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1615373012; bh=i353/5E6RaYp7xbqYVwMNxmiaK1JAT4Ap5jfKOpEIzE=; 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=ZU07RcTmr7HzNEBh518jn+0TiZp/9Qfk0MrDq/gWhpvhtmc1tmhha0wklZFIpfpfX ZQFyseVLruzXURXw7vSsMpvsAn5cDI4QblWwGq4tvd+dUzmZoioxkcAvO3K0QOj0XD SyGYvIM2r7U5S1/XN64YroS8M+A/RXo25K1N1wwA= Received: from smtpng1.m.smailru.net (smtpng1.m.smailru.net [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 AD7FD6EC59 for ; Wed, 10 Mar 2021 13:43:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org AD7FD6EC59 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1lJwJN-0003WX-Ld; Wed, 10 Mar 2021 13:43:30 +0300 Date: Wed, 10 Mar 2021 13:43:23 +0300 To: Sergey Ostanevich Message-ID: <20210310104323.GS9042@tarantool.org> References: <20210309235917.GO9042@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D3134714A9BDB69B68E0B78C18DFC811DDE0A658CF5A64D900894C459B0CD1B9AEEA962E8556A0F924BFCCD0C115C6C34D9BD26CF834F067611C1030484FBD82 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE73AA63C5F29446501EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006373D332FFE8BBF4EB58638F802B75D45FF914D58D5BE9E6BC131B5C99E7648C95C1F4A82545730C761D2CF5DD5845C06E510FE8E1A60E86DF5A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE7328B01A8D746D8839FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3ED8438A78DFE0A9E117882F4460429728AD0CFFFB425014E868A13BD56FB6657A7F4EDE966BC389F9E8FC8737B5C224958CDF7FFE3EBBCC0089D37D7C0E48F6CCF19DD082D7633A0E7DDDDC251EA7DABAAAE862A0553A39223F8577A6DFFEA7C4DB04CA562B6C7F843847C11F186F3C5E7DDDDC251EA7DABCC89B49CDF41148FA8EF81845B15A4842623479134186CDE6BA297DBC24807EABDAD6C7F3747799A X-C1DE0DAB: 0D63561A33F958A52BCDDA0208E2E1FCA6566F8D5FEFD33F5B97F771F5C99D96D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D349FF8F8245A2FAA7B1FEC14D336025C736AF0106EA742937FBB895F2775A7750B800EEF6E6B1E4F261D7E09C32AA3244C9BDE476B534764B26C6D9B9DD57D6464D9ADFF0C0BDB8D1F927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojsR8tyFmO15PSo/nfc1yzqQ== X-Mailru-Sender: 689FA8AB762F73936BC43F508A063822A66960FB8AA9A31D268EDE3C235306D3A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [WIP luajit 00/15] Adapt LuaVela test suites 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: Igor Munkin via Tarantool-patches Reply-To: Igor Munkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Sergos, On 10.03.21, Sergey Ostanevich wrote: > Hi! > > Thanks for the patch! > > > On 10 Mar 2021, at 02:59, Igor Munkin wrote: > > > > Sergey, > > > > I've precisely reviewed patches 04-09 and they look good. I've polished > > a bit[1] the series, so it can be applied to the trunk out of the order. > > CI[2] is green. > > > > BTW, I would like to leave some major points related to my review. > > > > 1. The suite has been taken intact. To check this I used the following > > recipe: > > | $ pwd > > | /LuaJIT-test-cleanup/test > > | $ git remote -v > > | origin https://github.com/LuaJIT/LuaJIT-test-cleanup.git (fetch) > > | origin https://github.com/LuaJIT/LuaJIT-test-cleanup.git (push) > > | $ git lo -1 > > | 014708b (HEAD -> master, origin/master, origin/HEAD) Add test for BC_VARG slot revival > > | $ find . -type f | sort | xargs md5sum > ~/vanilla > > Why did you use sort here? I recall using sort -u to remove duplicates, but it is not… For the same reason people run tests several times in a row: to be *more* sure I do everything right... If find yields the same order every time, sort can be omitted. > > > | <...> > > | $ pwd > > | /tarantool-luajit/test/LuaJIT-test > > | $ git remote -v > > | origin git@github.com:tarantool/luajit (fetch) > > | origin git@github.com:tarantool/luajit (push) > > | $ git lo -1 > > | cc0967c (HEAD) test: add LuaJIT-test-cleanup test suite > > | $ find . -type f | sort | xargs md5sum > ~/tarantool > > | $ diff ~/vanilla ~/tarantool > > | > 91cb30f369b83d7626a8ae852781ebb5 ./CMakeLists.txt > > > > 2. I've introduced a new CMake option: LUAJIT_TEST_INIT. You can read > > the description here[3]. It allows to enclose the partial LUA_INIT > > emulation, implemented via '-e dofile[[${LUAJIT_TEST_INIT}]]' and > > obliges user to set the name of the Lua script to be run prior to the > > testing suite[4]. By default it does nothing by reading from /dev/null. > > > I would like to complain that you’re open extra file each test run, > but there are only 500 tests and AFAIU it is a temporary solution. > What is the plan for the final one? Well... There is nothing more permanent than temporary solution, isn't it?. In "nop" case these manipulations can be omitted in CMake, *but* this doesn't make the situation better for Tarantool CI. So, let's see if there are issues with such extra activity and measure its impact on Tarantool testing and then decide whether LuaJIT testing machinery need to be boosted. Anyway, here are some numbers for the test target (`cmake . && make -j` is already finished before measures): * HEAD^ (with no extra files) | $ time make -j test | real 0m0.842s | user 0m1.018s | sys 0m0.255s * HEAD (with /dev/null access) | $ time make -j test | real 0m0.844s | user 0m1.021s | sys 0m0.235s > > > 3. Since CLI flags can be used for LuaJIT testing now, > > need to be adjusted[5] to respect the passed flags in child process. > > > > This part LGTM, so if nobody is against the version I've checked to the > > branch[1], then I apply it to the trunk after your and Sergos approval. > > > > LGTM. Added your tags to all patches: | Reviewed-by: Sergey Ostanevich > Sergos > > > > > > [1]: https://github.com/tarantool/luajit/tree/imun/tarantool-test > > [2]: https://github.com/tarantool/tarantool/tree/imun/luajit-test > > [3]: https://github.com/tarantool/luajit/commit/9712cc9#diff-1e7de1ae2d059d21e1dd75d5812d5a34b0222cef273b7c3a2af62eb747f9d20aR291-R307 > > [4]: https://github.com/tarantool/tarantool/commit/03b04a7 > > [5]: https://github.com/tarantool/luajit/commit/72f87b2 > > > > -- > > Best regards, > > IM > -- Best regards, IM