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 207936B63E8; Sat, 11 Nov 2023 20:05:18 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 207936B63E8 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1699722318; bh=jy2Q+F8/z/rHBwo6P6HWdI5c2x+Z5BN+yNOFJvBrSw8=; 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=LKb1x8iVVulG2b3MeIMybMPpFUFgu54iLiPey4gsWiw9OpdD/J2KwLPIxqpmwUxem bofqTM9RAp5M3RYgPyfQxYpT8GLIj5n5bPJluvEZJCknof4EGgaO4XzIFRHm74Rnqg CyMXXtQH5brv1/7W8TldiPSXlK/tf+j/SUs011i8= Received: from smtp39.i.mail.ru (smtp39.i.mail.ru [95.163.41.80]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 0CAC7580F49 for ; Sat, 11 Nov 2023 20:05:15 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 0CAC7580F49 Received: by smtp39.i.mail.ru with esmtpa (envelope-from ) id 1r1rQ1-00EQvt-2X; Sat, 11 Nov 2023 20:05:14 +0300 Message-ID: <7d138847-b21b-4de4-9608-86deff7b38db@tarantool.org> Date: Sat, 11 Nov 2023 20:05:12 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird Content-Language: en-US To: Igor Munkin , Sergey Bronnikov References: <9b4befc22674bcc924ad74313d968d1748b85bd9.1698053812.git.sergeyb@tarantool.org> In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD9C2A6B03AB739174C6E86FCE1CC902D40CD6F1F1BD9F1501500894C459B0CD1B9123C60ABEAE414D31655ECDE148E356F9B5E45DA2C201A241E13962C942EC85E X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE745C0EDBD94D46193EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637416EC277BB71E4518638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D86D4B2325A710BC160DDEE6DEA5326780117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCAE9A1BBD95851C5BA471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7A2AE38A8E97BAFFB1D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE364E7220B7C550592302FCEF25BFAB345C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CA9FF340AA05FB58C2E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89FC0F9454058DFE53C5E1C53F199C2BB95B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A5BE9B58625D745AECD83807977C044619F76EBFBB66855357F87CCE6106E1FC07E67D4AC08A07B9B0AD0E433DBF1FBFA3CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34C53D1911AD7E4199AD2F282552B8BC1AF678A82C1B85B054CAE4EEA2C736DE802F02855E1258FC891D7E09C32AA3244C46766DCAAEB941A5B971DA323EBBF9A730452B15D76AEC14ED9A463B7D0AAFF65DA084F8E80FEBD3FFA33E6B6B2F82C47A83BD0C44CE203720ABEDE4BBDD9CDD X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojTi+B9xGhsLRZ8UoenodVUA== X-Mailru-Sender: C4F68CFF4024C8867DFDF7C7F2588458CBDDB9936FE3835D575E82CBD464F5C01D7950FC0AA432A5282EC151BADDC1D3523A6D01B4765B2DFB59E2DDD9FE06B14FA522850F29BC30B0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest 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 Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Cc: max.kokryashkin@gmail.com, tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Igor On 10/31/23 22:17, Igor Munkin wrote: > Sergey, > > Thanks for the patch! I have some general comments prior to make more > precise review for the particular changeset. > > 1. Unfortunately, the old scenario is broken: > The recipe I used to run for building and running LuaJIT test is the > following one: > | $ cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON && make -j test > As a result of the changes, target doesn't build the > dependencies (neither luajit per se, nor particular test helpers > required to be compiled). So the recipe is transformed to this one: > | $ cmake . -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON > | $ make -j # build LuaJIT > | $ make -j tarantool-tests # run the particular suite to build the requirements > | $ make -j tarantool-c-tests # ditto > | $ make -j PUC-Rio-Lua-5.1-tests # some binary artefacts for Lua suite > | $ make -j test > Looks not such convenient comparing to the old mantra. Updated patch a bit. Now steps are the same:     $ cmake -S . -B build     $ cmake --build build     $ cmake --build --target LuaJIT-test ctest and test target works too. > > 2. The new CTest targets should follow the existing naming practice > (i.e. use the directory name, or its distinctive prefix), since the > following command runs three suites. > | $ ctest -L lua # runs lua, luajit and lua-harness Updated labels, now labels are equals to prefix of appropriate testsuite. > > 3. It looks like the logs will not be showed in CI to debug a > hard-reproducible issue if one occurs. <--output-on-failure> can be a > saviour here for us. This option is already used by test targets, see TEST_FLAGS in test/CMakeLists.txt. If you run tests by ctest then you should set env variable CTEST_OUTPUT_ON_FAILURE=1. Another option is setting default ctest options via CMAKE_CTEST_ARGUMENTS, but it is available since CMake 3.17. > > 4. I'm afraid the current changes break integration with Tarantool > tests. The reason is duplication of target both in LuaJIT and > Tarantool build systems. AFAIU, moving under the > condition similar to the one handled with LUAJIT_USE_TEST option > doesn't solve the issue, since no test will be run as a result. > Please check this part: it might be the main technical blocker for > this changeset. No, it doesn't break integration. Letter with patch has  a link to PR to tarantool repository, all CI tests are passed. > 5. I remember that we're talking regarding running particular tests for > all suites, but I see this is done only for tarantool* and lua-Harness. > Is it experimental and you'll fix it later in scope of this PR? There > is definitely the way to run a particular test in LuaJIT suite > (AFAIR, you can just use its number). Regarding PUC-Rio-Lua-5.1 we > can think about replacing all.lua as a runner as it's done in uJIT. The patch replaces prove with ctest. Updating test runner for PUC Rio Lua and LuaJIT testsuites is out of scope for this patch. This could be done later and by someone else. > > I believe that's all for the starters.