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 153C6C833C4; Mon, 18 Mar 2024 17:58:31 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 153C6C833C4 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1710773911; bh=IeDjvqCNRKwDKpEsdAEk8+TH8fzf8c8b2nvF2Gl46Z0=; 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=X6oQXXEEL0ue4LTHIZnsZxutuuwYuoguw9UQdEVJ5IMmkbnJHYlUqWUqesx+KgHH+ P9SAYWPMMEt8bYQrCMSsSMXYCb1WQdrI2wijPGHftzns1je7bkwFIWs8kpUrdDC3Oc Yf2kSMC7605cjavJlv9XtXmZWsM3A0oFJE1xH880= Received: from smtp54.i.mail.ru (smtp54.i.mail.ru [95.163.41.89]) (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 8575CC833C1 for ; Mon, 18 Mar 2024 17:58:30 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8575CC833C1 Received: by smtp54.i.mail.ru with esmtpa (envelope-from ) id 1rmERX-00000003Ak7-1wc7; Mon, 18 Mar 2024 17:58:28 +0300 Message-ID: <24db4ab3-b7c0-422c-970c-7af0a4f03254@tarantool.org> Date: Mon, 18 Mar 2024 17:58:27 +0300 MIME-Version: 1.0 User-Agent: Mozilla Thunderbird To: Igor Munkin References: <62428a61c98eee87dfc91f2d8972988d80589d3e.1710226958.git.sergeyb@tarantool.org> Content-Language: en-US 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: 4F1203BC0FB41BD987C0EE6E7F0A597D21AFE2E000C69B1FBAF020A234192257182A05F53808504093E2CBB5F2EF6812A6D5EE0DB6E1EC8D1937D06BF8E21B0DF1313279157413E661707949AA19B90C X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7466896EF24E80F12EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063710B5F6815F28EB638638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D85146BD3F0D74E3D1D2EEB1E46AE9C192C5674EA37ACBEF11CC7F00164DA146DAFE8445B8C89999728AA50765F7900637F3E38EE449E3E2AE389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D2DCF9CF1F528DBCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947CD95D32202655EC456136E347CC761E074AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C325491C6571998FF8BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CFE97D2AE7161E217F1DD303D21008E298D5E8D9A59859A8B6B372FE9A2E580EFC725E5C173C3A84C384C61F09AF66B0A435872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A54D185808AD86B88E5002B1117B3ED696EA45A3779A92A8A0C66B2B37046EC955823CB91A9FED034534781492E4B8EEAD64C12243F54E5AB4 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFB9B5C26482C2C54E052FAF9F3EEC8F2387EFE826C79D3B521C9F2E07B08910C37E72D8710CBEBF0EE1E51F4E63A6583499AEAFB1683F650C97B2FCD2E2B9667D7C8F6579E003ECFEC226CC413062362A913E6812662D5F2AB9AF64DB4688768036DF5FE9C0001AF333F2C28C22F508233FCF178C6DD14203 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojKEbXFKsUiMnxO8EQIUrtdg== X-Mailru-Sender: 520A125C2F17F0B1E52FEF5D219D6140F0CD52F7B389C0AA76695ADBB4D1A21381361CC33EDAF5F80152A3D17938EB451EB5A0BCEC6A560B3DDE9B364B0DF289BE2DA36745F2EEB5CEBA01FB949A1F1EEAB4BC95F72C04283CDA0F3B3F5B9367 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit][v3] 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" Igor, thanks for the next review iteration. See my comments below. On 3/16/24 20:12, Igor Munkin wrote: > Sergey, > > Thanks for the fixes! The patch L(almost)GTM now, but I still make some > comments. You can see my changes here below. > > On 12.03.24, Sergey Bronnikov wrote: >> From: Sergey Bronnikov >> >> The patch replaces the main test runner `prove(1)` with CTest, >> see [1] and [2]. >> >> Build and test steps looks the same as before: >> >> $ cmake -S . -B build >> $ cmake --build build --parallel >> $ cmake --build build --target [LuaJIT-test, test] >> >> CMake targets lua-Harness-tests, LuaJIT-tests, PUC-Rio-Lua-5.1-tests, >> tarantool-tests and tarantool-c-tests are still functional. >> >> One could use `ctest` tool: >> >> $ ctest --print-labels >> Test project >> All Labels: >> LuaJIT >> PUC-Rio-Lua-5.1 >> lua-Harness >> tarantool >> tarantool-c > Typo: Labels are updated since v2. Updated to: $ ctest --print-labels Test project All Labels:   LuaJIT-tests   PUC-Rio-Lua-5.1-tests   lua-Harness-tests   tarantool-c-tests   tarantool-tests > >> $ ctest # Run all tests. >> $ ctest -L tarantool # Run tests in tarantool-tests dir. >> $ ctest -L tarantool-c # Run tests in tarantool-c-tests dir. >> $ ctest -L lua-Harness # Run tests in lua-Harness-tests dir. >> $ ctest -L luajit # Run tests in LuaJIT-tests dir. >> $ ctest -L PUC-Rio-Lua-5.1 # Run tests in PUC-Rio-Lua-5.1-tests dir. >> $ ctest --rerun-fail >> $ ctest --output-on-failure >> >> The benefits are: >> >> - less external dependencies, `prove(1)` is gone, `ctest` is a part of CMake >> - running tests by test title >> - extended test running options in comparison to prove(1) >> - unified test output (finally!) >> >> Note that it is not possible to attach targets to the automatically >> generated `test` target. It means that target `test` is now executing >> `LuaJIT-test`, but not `LuaJIT-lint`. > Side note: I guees something like can save the behaviour > broken by this change. However, do we really need this? I left the final > decision to skaplun@ here. > >> Note that the testsuites in `test/LuaJIT-tests` and >> `test/PUC-Rio-Lua-5.1` directories have their own test runners written >> in Lua and currently CTest runs these testsuites as single tests. In the >> future we could change these test runners to specify tests from outside, >> and after this we could run tests separately by CTest in these test >> suites. >> >> Note that it is not possible to add dependencies to `add_test()` in >> CMake, see [3]. CMake 3.7 introduces FIXTURES_REQUIRED [4] and >> FIXTURES_SETUP [5], but these test properties cannot be used - used >> CMake version is lower. This workarounded by introducing a special test >> for each testsuite that executes a target that builds test dependencies. > Minor: Maybe it's worth to mention macro here? Rephrased to:   Note that it is not possible to add dependencies to `add_test()` in   CMake, see [3]. CMake 3.7 introduces FIXTURES_REQUIRED [4] and   FIXTURES_SETUP [5], but these test properties cannot be used - used   CMake version is lower. This workarounded by introduced a macro   `add_test_suite_target` that adds a CMake-test for each testsuite   that executes a target that builds test dependencies. > >> 1. https://cmake.org/cmake/help/latest/manual/ctest.1.html >> 2. https://cmake.org/cmake/help/book/mastering-cmake/chapter/Testing%20With%20CMake%20and%20CTest.html >> 3. https://gitlab.kitware.com/cmake/cmake/-/issues/8774 >> 4. https://cmake.org/cmake/help/latest/prop_test/FIXTURES_REQUIRED.html >> 5. https://cmake.org/cmake/help/latest/prop_test/FIXTURES_SETUP.html >> --- >> Changes v3: >> - rebased to master >> - applied fixes suggested by Igor Munkin >> >> PR: https://github.com/tarantool/tarantool/pull/9286 >> Branch: https://github.com/tarantool/luajit/tree/ligurio/enable_test_target >> >> .gitignore | 2 + >> CMakeLists.txt | 11 ++++ >> test/CMakeLists.txt | 78 +++++++++++++++-------- >> test/LuaJIT-tests/CMakeLists.txt | 40 +++++++----- >> test/LuaJIT-tests/src/CMakeLists.txt | 2 +- >> test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt | 25 +++++--- >> test/lua-Harness-tests/CMakeLists.txt | 48 ++++++++------ >> test/tarantool-c-tests/CMakeLists.txt | 56 ++++++++-------- >> test/tarantool-tests/CMakeLists.txt | 62 ++++++++---------- >> 9 files changed, 194 insertions(+), 130 deletions(-) >> >> diff --git a/CMakeLists.txt b/CMakeLists.txt >> index 7f5e2afb..6b2f855f 100644 >> --- a/CMakeLists.txt >> +++ b/CMakeLists.txt >> @@ -345,6 +345,13 @@ set(LUAJIT_TEST_BINARY ${LUAJIT_BINARY} CACHE STRING >> "Lua implementation to be used for tests. Default is 'luajit'." >> ) >> >> +# If LuaJIT is used in a parent project, provide an option >> +# to choose what dependencies to be used for running and building >> +# LuaJIT tests. >> +set(LUAJIT_TEST_DEPS "luajit-main" CACHE STRING >> + "A list of target dependencies to be used for tests." >> +) > This is worth to be mentioned in the commit message (at least). Added:     The commit introduce a CMake option LUAJIT_TEST_DEPS that set     dependencies to be used for running and building LuaJIT tests. > >> + >> # FIXME: If LuaJIT is used in parent project, provide an option >> # to pass Lua code to be run before tests are started. >> # XXX: Attentive reader might point to LUA_INIT environment > > >> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt >> index 3ad5d15f..90aaead2 100644 >> --- a/test/CMakeLists.txt >> +++ b/test/CMakeLists.txt >> @@ -77,35 +77,63 @@ separate_arguments(LUAJIT_TEST_COMMAND) >> set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake") >> include(AddTestLib) >> >> +# TEST_FLAGS is used by CMake targets in test suites. >> +# XXX: CMake 3.17 introduces CMAKE_CTEST_ARGUMENTS that contains CTest options > Typo: We still follows 66 symbol limit for comments in tarantool/luajit, > but I also left the final decision to skaplun@ here. Fixed: --- a/test/CMakeLists.txt +++ b/test/CMakeLists.txt @@ -78,8 +78,8 @@ set(CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/cmake")  include(AddTestLib)  # TEST_FLAGS is used by CMake targets in test suites. -# XXX: CMake 3.17 introduces CMAKE_CTEST_ARGUMENTS that contains CTest options -# and is used by `test` target. +# XXX: CMake 3.17 introduces CMAKE_CTEST_ARGUMENTS that contains +# CTest options and is used by `test` target.  set(TEST_FLAGS    --output-on-failure    --schedule-random > >> +# and is used by `test` target. > > >> -add_custom_target(${PROJECT_NAME}-test DEPENDS >> - LuaJIT-tests >> - PUC-Rio-Lua-5.1-tests >> - lua-Harness-tests >> - tarantool-c-tests >> - tarantool-tests >> +add_custom_target(${PROJECT_NAME}-test >> + COMMAND ${CMAKE_CTEST_COMMAND} ${TEST_FLAGS} >> + DEPENDS tarantool-c-tests-deps >> + tarantool-tests-deps >> + lua-Harness-tests-deps >> + PUC-Rio-Lua-5.1-tests-deps >> + LuaJIT-tests-deps > I suggest to make LuaJIT-test an umbrella rule without the particular > implementation. See the changes below: > > ================================================================================ > > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt > index 90aaead2..ad447e72 100644 > --- a/test/CMakeLists.txt > +++ b/test/CMakeLists.txt > @@ -130,10 +130,9 @@ add_subdirectory(tarantool-c-tests) > add_subdirectory(tarantool-tests) > > add_custom_target(${PROJECT_NAME}-test > - COMMAND ${CMAKE_CTEST_COMMAND} ${TEST_FLAGS} > - DEPENDS tarantool-c-tests-deps > - tarantool-tests-deps > - lua-Harness-tests-deps > - PUC-Rio-Lua-5.1-tests-deps > - LuaJIT-tests-deps > + DEPENDS tarantool-c-tests > + tarantool-tests > + lua-Harness-tests > + PUC-Rio-Lua-5.1-tests > + LuaJIT-tests > ) > > ================================================================================ Here we go again. Already discussed it verbally some time ago. With proposed patch ctest splits output for each testsuite: $ ninja LuaJIT-test [9/15] cd /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/gc64/t...ctest -L tarantool-c-tests --output-on-failure --schedule-random --parallel 8 Test project /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/gc64/test/tarantool-c-tests       Start  7: test/tarantool-c-tests/lj-962-premature-stack-overflow.c_test 10/10 Test  #9: test/tarantool-c-tests/misclib-getmetrics-capi.c_test ...................   Passed    0.06 sec 100% tests passed, 0 tests failed out of 10 Label Time Summary: tarantool-c-tests    =   0.67 sec*proc (10 tests) Total Test time (real) =   0.13 sec [12/15] cd /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/gc64/...ctest -L lua-Harness-tests --output-on-failure --schedule-random --parallel 8 Test project /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/gc64/test/lua-Harness-tests 0/50 Test #33: test/lua-Harness-tests/241-standalone.t .... Passed    1.18 sec [0/6793] 100% tests passed, 0 tests failed out of 50 Label Time Summary: lua-Harness-tests    =   5.22 sec*proc (50 tests) Total Test time (real) =   1.53 sec [13/15] cd /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/gc64/.../bin/ctest -L LuaJIT-tests --output-on-failure --schedule-random --parallel 8 Test project /home/sergeyb/sources/MRG/tarantool/third_party/luajit/build/gc64/test/LuaJIT-tests     Start 2: test/LuaJIT-tests 1/1 Test #2: test/LuaJIT-tests ................   Passed    3.65 sec This output looks ugly and is not convenient in daily use. Left it without changes. > >> ) > > >> diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt >> index a0fb5440..1080987a 100644 >> --- a/test/LuaJIT-tests/CMakeLists.txt >> +++ b/test/LuaJIT-tests/CMakeLists.txt >> @@ -3,17 +3,13 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR) >> >> add_subdirectory(src) >> >> -add_custom_target(LuaJIT-tests >> - DEPENDS ${LUAJIT_TEST_BINARY} LuaJIT-tests-libs >> -) >> - >> make_lua_path(LUA_CPATH >> PATHS >> ${CMAKE_CURRENT_BINARY_DIR}/src/?${CMAKE_SHARED_LIBRARY_SUFFIX} >> ) >> >> set(LUAJIT_TESTS_ENV >> - "LUA_CPATH=\"${LUA_CPATH}\"" >> + "LUA_CPATH=${LUA_CPATH}\;\;" > The trailing semicolons are automatically added by . My > bad, remove it please. --- a/test/LuaJIT-tests/CMakeLists.txt +++ b/test/LuaJIT-tests/CMakeLists.txt @@ -9,7 +9,7 @@ make_lua_path(LUA_CPATH  )  set(LUAJIT_TESTS_ENV -  "LUA_CPATH=${LUA_CPATH}\;\;" +  "LUA_CPATH=${LUA_CPATH}"  )  set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}/src/:") > >> ) >> >> set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}/src/:") >> @@ -50,10 +46,11 @@ if(LUAJIT_USE_ASAN) >> if(CMAKE_C_COMPILER_ID STREQUAL "GNU") >> LibRealPath(LIB_ASAN libasan.so) >> # XXX: Don't use " " (separator in LD_PRELOAD) in `list()`. >> - # ";" will expand to " " as we want. >> - list(APPEND LUAJIT_TESTS_ENV LD_PRELOAD="${LIB_ASAN};${LIB_STDCPP}") >> + # ";" will expand to " " as we want after wrapping >> + # LUAJIT_TESTS_ENV by double quotes. >> + list(APPEND LUAJIT_TESTS_ENV LD_PRELOAD=${LIB_ASAN};${LIB_STDCPP}) >> else() >> - list(APPEND LUAJIT_TESTS_ENV LD_PRELOAD="${LIB_STDCPP}") >> + list(APPEND LUAJIT_TESTS_ENV LD_PRELOAD=${LIB_STDCPP}) >> endif() >> endif() > I really do not understand, why this quotation is required in the > original series and breaks everything now. If skaplun@ is bothered more > than me, he can spend more time with CMake to understand this. Sad, but > OK, so I'm out here. > >> > > >> diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt >> index f748a8fd..d454ba41 100644 >> --- a/test/lua-Harness-tests/CMakeLists.txt >> +++ b/test/lua-Harness-tests/CMakeLists.txt > > >> +file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.t") >> +foreach(test_path ${tests}) >> + get_filename_component(test_name ${test_path} NAME) >> + # FIXME: By default GLOB lists directories. >> + # Directories are omitted in the result if LIST_DIRECTORIES >> + # is set to false. New in version CMake 3.3. >> + if (${test_name} STREQUAL ${TEST_SUITE_NAME}) >> + continue() >> + endif (${test_name} STREQUAL ${TEST_SUITE_NAME}) > Typo: Excess whitespace and condition in . --- a/test/lua-Harness-tests/CMakeLists.txt +++ b/test/lua-Harness-tests/CMakeLists.txt @@ -32,7 +32,7 @@ foreach(test_path ${tests})    # is set to false. New in version CMake 3.3.    if (${test_name} STREQUAL ${TEST_SUITE_NAME})      continue() -  endif (${test_name} STREQUAL ${TEST_SUITE_NAME}) +  endif()    set(test_title "test/${TEST_SUITE_NAME}/${test_name}")    add_test(NAME ${test_title}             COMMAND ${LUAJIT_TEST_COMMAND} -l profile_luajit21 ${test_path} > >> + set(test_title "test/${TEST_SUITE_NAME}/${test_name}") >> + add_test(NAME ${test_title} >> + COMMAND ${LUAJIT_TEST_COMMAND} -l profile_luajit21 ${test_path} >> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} > Typo: Looks like misindent, doesn't it? Fixed:    set(test_title "test/${TEST_SUITE_NAME}/${test_name}")    add_test(NAME ${test_title} -           COMMAND ${LUAJIT_TEST_COMMAND} -l profile_luajit21 ${test_path} -           WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} +    COMMAND ${LUAJIT_TEST_COMMAND} -l profile_luajit21 ${test_path} +    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}    )    set_tests_properties(${test_title} PROPERTIES      ENVIRONMENT "LUA_PATH=${LUA_PATH}\;" > >> + ) >> + set_tests_properties(${test_title} PROPERTIES >> + ENVIRONMENT "LUA_PATH=${LUA_PATH}\;" > Typo: The semicolon looks excess here. --- a/test/lua-Harness-tests/CMakeLists.txt +++ b/test/lua-Harness-tests/CMakeLists.txt @@ -39,7 +39,7 @@ foreach(test_path ${tests})      WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}    )    set_tests_properties(${test_title} PROPERTIES -    ENVIRONMENT "LUA_PATH=${LUA_PATH}\;" +    ENVIRONMENT "LUA_PATH=${LUA_PATH}"      LABELS ${TEST_SUITE_NAME}      DEPENDS lua-Harness-tests-deps    ) > >> + LABELS ${TEST_SUITE_NAME} >> + DEPENDS lua-Harness-tests-deps >> + ) >> +endforeach() >> + >> # vim: expandtab tabstop=2 shiftwidth=2 >> diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt >> index 4b1d8832..2de4a0c8 100644 >> --- a/test/tarantool-c-tests/CMakeLists.txt >> +++ b/test/tarantool-c-tests/CMakeLists.txt > > >> @@ -49,20 +38,35 @@ foreach(test_source ${tests}) > > >> +# XXX: The call produces both test and target >> +# as a side effect. > Typo: Miscopypasting :) Fixed, thanks! --- a/test/tarantool-c-tests/CMakeLists.txt +++ b/test/tarantool-c-tests/CMakeLists.txt @@ -41,7 +41,7 @@ endforeach()  set(TEST_SUITE_NAME "tarantool-c-tests")  # XXX: The call produces both test and target -# as a side effect. +# as a side effect.  add_test_suite_target(tarantool-c-tests    LABEL ${TEST_SUITE_NAME}    DEPENDS ${LUAJIT_TEST_DEPS} libluajit libtest ${TESTS_COMPILED} > >> +add_test_suite_target(tarantool-c-tests >> + LABEL ${TEST_SUITE_NAME} >> + DEPENDS ${LUAJIT_TEST_DEPS} libluajit libtest ${TESTS_COMPILED} >> ) >> + >> +# XXX: Note that we cannot glob generated files in CMake, see >> +# https://stackoverflow.com/questions/44076307/cmake-globbing-generated-files >> +# To workaround this, we glob source files and generated tests >> +# with paths to executable binaries. >> +file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.test.c") >> +foreach(test_path ${tests}) >> + get_filename_component(test_name ${test_path} NAME_WE) >> + # FIXME: By default GLOB lists directories. >> + # Directories are omitted in the result if LIST_DIRECTORIES >> + # is set to false. New in version CMake 3.3. >> + if (${test_name} STREQUAL ${TEST_SUITE_NAME}) >> + continue() >> + endif (${test_name} STREQUAL ${TEST_SUITE_NAME}) > Typo: Excess whitespace and condition in . --- a/test/tarantool-c-tests/CMakeLists.txt +++ b/test/tarantool-c-tests/CMakeLists.txt @@ -59,7 +59,7 @@ foreach(test_path ${tests})    # is set to false. New in version CMake 3.3.    if (${test_name} STREQUAL ${TEST_SUITE_NAME})      continue() -  endif (${test_name} STREQUAL ${TEST_SUITE_NAME}) +  endif()    set(test_title "test/${TEST_SUITE_NAME}/${test_name}${C_TEST_SUFFIX}")    add_test(NAME ${test_title}      COMMAND ${CMAKE_CURRENT_BINARY_DIR}/${test_name}${C_TEST_SUFFIX} > >> + set(test_title "test/${TEST_SUITE_NAME}/${test_name}${C_TEST_SUFFIX}") > > >> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt >> index e6d12984..bb41e53b 100644 >> --- a/test/tarantool-tests/CMakeLists.txt >> +++ b/test/tarantool-tests/CMakeLists.txt > > >> @@ -60,21 +54,14 @@ make_lua_path(LUA_PATH >> ${PROJECT_SOURCE_DIR}/tools/?.lua >> ${LUAJIT_SOURCE_DIR}/?.lua >> ${LUAJIT_BINARY_DIR}/?.lua >> + ${PROJECT_BINARY_DIR}/src/?.lua > This is the same directory as a ${LUAJIT_BINARY_DIR}/?.lua. Fixed: --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -54,7 +54,6 @@ make_lua_path(LUA_PATH      ${PROJECT_SOURCE_DIR}/tools/?.lua      ${LUAJIT_SOURCE_DIR}/?.lua      ${LUAJIT_BINARY_DIR}/?.lua -    ${PROJECT_BINARY_DIR}/src/?.lua  )  # Update LUA_CPATH with the library paths collected within >> ) > > >> @@ -110,22 +97,29 @@ else() > > >> +# LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list >> +# with dependencies are set in scope of BuildTestLib macro. >> +set(TEST_ENV "LUA_PATH=${LUA_PATH};\;\;;LUA_CPATH=${LUA_CPATH}\;\;;${LUA_TEST_ENV_MORE}") > The trailing semicolons are automatically added by . > Please, remove it. Fixed: --- a/test/tarantool-tests/CMakeLists.txt +++ b/test/tarantool-tests/CMakeLists.txt @@ -107,7 +107,7 @@ add_test_suite_target(tarantool-tests  # LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list  # with dependencies are set in scope of BuildTestLib macro. -set(TEST_ENV "LUA_PATH=${LUA_PATH};\;\;;LUA_CPATH=${LUA_CPATH}\;\;;${LUA_TEST_ENV_MORE}") +set(TEST_ENV "LUA_PATH=${LUA_PATH};LUA_CPATH=${LUA_CPATH};${LUA_TEST_ENV_MORE}")  file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")  foreach(test_path ${tests})    get_filename_component(test_name ${test_path} NAME) > >> +file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}") > Minor: Why is used here, and the hack with manual > directory name filtering is omitted? it is better to search test files recursively, no one can guarantee that files always will be in a top testsuite dir. However in lua-Harness suite we cannot use GLOB_RECURSE because suite has a number of files with equal filenames: $ find  test/lua-Harness-tests/ -name lexico.t test/lua-Harness-tests/lexico53/lexico.t test/lua-Harness-tests/lexicojit/lexico.t test/lua-Harness-tests/lexico52/lexico.t test/lua-Harness-tests/lexico54/lexico.t due to this CMake configuration has failed with "add_test given test NAME "test/lua-Harness-tests/lexico.t" which already exists in this directory." > >> +foreach(test_path ${tests}) >> + get_filename_component(test_name ${test_path} NAME) >> + set(test_title "test/tarantool-tests/${test_name}") >> + add_test(NAME ${test_title} >> + COMMAND ${LUAJIT_TEST_COMMAND} ${test_path} >> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR} >> + ) >> + set_tests_properties(${test_title} PROPERTIES >> + ENVIRONMENT "${TEST_ENV}" >> + LABELS ${TEST_SUITE_NAME} >> + DEPENDS tarantool-tests-deps >> + ) >> +endforeach() >> -- >> 2.34.1 >>