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 6483D6EC59; Wed, 10 Mar 2021 13:39:10 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 6483D6EC59 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1615372750; bh=NvFPVPYSvgRLANJpCurLmCxhpjQaPGNQQ8tNWBxl8dc=; 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=mgpFnS35GzeeZSXfeAfCzSIqv8JZGdRcZ69NnVxyOy8DqWT3xJ+0zKrKbIO4QGQ7e ZkEM6rMy23lX/XqvytXCgFp0MfokamQZ3rWy08WFKjrtPBZDMMb6YQ4/m//Wuo+rFQ /uqKMr1bHG4+Mj2ssobP9u5mS8dgaE8nMP18TvnI= Received: from smtpng2.m.smailru.net (smtpng2.m.smailru.net [94.100.179.3]) (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 39BF06EC59 for ; Wed, 10 Mar 2021 13:39:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 39BF06EC59 Received: by smtpng2.m.smailru.net with esmtpa (envelope-from ) id 1lJwF9-00011T-CH; Wed, 10 Mar 2021 13:39:07 +0300 Date: Wed, 10 Mar 2021 13:39:01 +0300 To: Sergey Kaplun Message-ID: <20210310103901.GR9042@tarantool.org> References: <20210309235917.GO9042@tarantool.org> <20210310080941.GH6842@root> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline In-Reply-To: <20210310080941.GH6842@root> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.10.1 (2018-07-13) X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D3134714A9BDB69B844F3AC9255B388D8D4E2637BBED27E200894C459B0CD1B9C945EE4C07EB61EBD6B4B7B313728B6CC5C8AF5F909FAABCB7464F346B62472A X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7CC84CC3AD347B910EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379A6B93796C91DCB58638F802B75D45FF5571747095F342E8C7A0BC55FA0FE5FC5FDA59836F905BEFCD79D604EB3DAFDB8A5FEE5C0C33CF7B389733CBF5DBD5E913377AFFFEAFD269176DF2183F8FC7C0ECC8AC47CD0EDEFF8941B15DA834481FCF19DD082D7633A0EF3E4896CB9E6436389733CBF5DBD5E9D5E8D9A59859A8B652D31B9D28593E51CC7F00164DA146DA6F5DAA56C3B73B23C77107234E2CFBA567F23339F89546C55F5C1EE8F4F765FCB835E6E385EA5AF075ECD9A6C639B01BBD4B6F7A4D31EC0BC0CAF46E325F83A522CA9DD8327EE4930A3850AC1BE2E73528A6D463EDFD0DBBC4224003CC836476C0CAF46E325F83A50BF2EBBBDD9D6B0F05F538519369F3743B503F486389A921A5CC5B56E945C8DA X-B7AD71C0: AC4F5C86D027EB782CDD5689AFBDA7A2BBE337FB72E92315FF39D8DB89857825743D937135AA13FF7120598F3739033D109B151A58CD633F834459D11680B505645035CCA8A1F2ACD132CE41481BE051 X-C1DE0DAB: 0D63561A33F958A5065398C2945327A7EEC0CC7A893AA35BE1603B5B67548E63D59269BC5F550898D99A6476B3ADF6B47008B74DF8BB9EF7333BD3B22AA88B938A852937E12ACA75448CF9D3A7B2C848410CA545F18667F91A7EA1CDA0B5A7A0 X-C8649E89: 4E36BF7865823D7055A7F0CF078B5EC49A30900B95165D34D7D5115130EFC82437E55E58956A3FB658B9D0EE5D98800FD7CB55FA070F719B3B6BF6718F7630891D7E09C32AA3244CC3C6ABFFC4EB82269056D844CE317AC93A92A9747B6CC886927AC6DF5659F194 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojsR8tyFmO15Ptu37CljkHdg== X-Mailru-Sender: 689FA8AB762F73936BC43F508A06382291EEE6C2CB42EA278B1F452F46359D38A7C8D0F45F857DBFE9F1EFEE2F478337FB559BB5D741EB964C8C2C849690F8E70A04DAD6CC59E33667EA787935ED9F1B 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" Sergey, On 10.03.21, Sergey Kaplun wrote: > Igor, > > On 10.03.21, 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. > > I hardly wait for the other series. > I've assembled all issues related to PUC-Rio-Lua-5.1 test suite inside > this [1] epic. > > > > > BTW, I would like to leave some major points related to my review. > > > > > > > 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. > > This part is good. Why do you omit +slow flag for this test suite? I'm too bad in copy-pasting... Thanks for catching! Added all necessary flags, squashed, force-pushed to the branch. Diff is below. ================================================================================ diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt index d4886a4..9cd76ee 100644 --- a/test/LuaJIT-tests/CMakeLists.txt +++ b/test/LuaJIT-tests/CMakeLists.txt @@ -7,5 +7,6 @@ add_custom_command(TARGET LuaJIT-tests COMMENT "Running LuaJIT-tests" COMMAND ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua + +slow +ffi +bit +jit WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} ) ================================================================================ > > > > > 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. > > As I see on the branch there is no check of CMAKE_BUILD_TYPE variable. > We need to disable strict only for debug build, IINM. > Feel free to ignore. Agree. BTW, it allows to check that Tarantool is not built with enabled by default in Release. Fixed, squashed, force-pushed to the branch. Diff is below. ================================================================================ diff --git a/cmake/luajit.cmake b/cmake/luajit.cmake index e25eeb3dd..1c05e085b 100644 --- a/cmake/luajit.cmake +++ b/cmake/luajit.cmake @@ -37,17 +37,19 @@ set(LUAJIT_SMART_STRINGS ON CACHE BOOL "Harder string hashing function" FORCE) set(LUAJIT_TEST_BINARY $ CACHE STRING "Lua implementation to be used for tests (tarantool)" FORCE) -set(LUAJIT_TEST_INIT "${PROJECT_SOURCE_DIR}/test/luajit-test-init.lua" - CACHE STRING "Lua code need to be run before tests are started" FORCE) set(LUAJIT_USE_TEST OFF CACHE BOOL "Generate target" FORCE) # Enable internal LuaJIT assertions for Tarantool Debug build. +# XXX: There is module enabled by default in Tarantool +# built in Debug, so we need to tweak LuaJIT testing environment. if(CMAKE_BUILD_TYPE STREQUAL "Debug") set(LUAJIT_USE_APICHECK ON CACHE BOOL "Assertions for the Lua/C API" FORCE) set(LUAJIT_USE_ASSERT ON CACHE BOOL "Assertions for the whole LuaJIT VM" FORCE) + set(LUAJIT_TEST_INIT "${PROJECT_SOURCE_DIR}/test/luajit-test-init.lua" + CACHE STRING "Lua code need to be run before tests are started" FORCE) endif() # Valgrind can be used only with the system allocator. For more ================================================================================ > > Otherwise, LGTM. Added your tags to all patches: | Reviewed-by: Sergey Kaplun Furthermore, I believe it is worth to mention your origin in LUAJIT_TEST_INIT idea, so I've added you as a co-author to the last patch on the branch. > > > > > > > [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 > > [1]: https://github.com/tarantool/tarantool/issues/5870 > > -- > Best regards, > Sergey Kaplun -- Best regards, IM