* [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
@ 2023-10-23 9:48 Sergey Bronnikov via Tarantool-patches
2023-10-24 13:41 ` Maxim Kokryashkin via Tarantool-patches
` (2 more replies)
0 siblings, 3 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-10-23 9:48 UTC (permalink / raw)
To: tarantool-patches, Sergey Kaplun, max.kokryashkin
From: Sergey Bronnikov <sergeyb@tarantool.org>
The patch replaces the main test runner prove(1) with CTest, see [1] and
[2].
The benefits are:
- less external dependencies, prove(1) is gone
- running tests by test title
- extended test running options in comparison to prove(1)
- unified test output (finally!)
Note, it is not possible to attach targets to the automatically
generated `test` target. It means that target `test` now is executing
`LuaJIT-test`, but not `LuaJIT-lint`.
Note, the testsuites in `test/LuaJIT-tests` and in
`test/PUC-Rio-Lua-5.1` directories have their own test runners and
currently CTest runs these testsuites as a single tests. In a future we
could change these test runners to specify test from outside and after
this we could run tests separately by CTest in these test suites.
Note, it is not possible to add dependencies to `add_test()` in CMake,
see [3]. It means that all object files and executables must be build
before running `ctest(1)`.
Examples of using CTest in a build directory:
$ ctest # Running all tests.
$ ctest -L tarantool # Running tests in tarantool-tests dir.
$ ctest -L tarantool-c # Running tests in tarantool-c-tests dir.
$ ctest -L lua-Harness # Running tests in lua-Harness dir.
$ ctest -L luajit # Running tests in LuaJIT dir.
$ ctest -L lua # Running tests in PUC-Rio-Lua-5.1 dir.
$ ctest --rerun-fail
$ ctest --output-on-failure
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
---
Changes v2:
- rebased to master
- fixed tests that require loading object files
- fixed backward compatibility with CMake targets
PR: https://github.com/tarantool/tarantool/pull/9286
Branch: https://github.com/tarantool/luajit/tree/ligurio/enable_test_target
Patch v1 was sent as RFC without list address in CC.
CMakeLists.txt | 1 +
test/CMakeLists.txt | 28 ++++--------
test/LuaJIT-tests/CMakeLists.txt | 17 +++++--
test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt | 23 ++++++----
test/lua-Harness-tests/CMakeLists.txt | 42 +++++++++---------
test/tarantool-c-tests/CMakeLists.txt | 49 ++++++++++----------
test/tarantool-tests/CMakeLists.txt | 54 ++++++++---------------
7 files changed, 100 insertions(+), 114 deletions(-)
diff --git a/CMakeLists.txt b/CMakeLists.txt
index eebf3d6f..1943d894 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -356,6 +356,7 @@ set(LUAJIT_TEST_INIT "${PROJECT_SOURCE_DIR}/test/luajit-test-init.lua" CACHE STR
"Lua code need to be run before tests are started."
)
+enable_testing()
add_subdirectory(test)
# --- Misc rules ---------------------------------------------------------------
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 58cba5ba..bc0c8dd8 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -73,6 +73,15 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
separate_arguments(LUAJIT_TEST_COMMAND)
+set(TEST_FLAGS
+ --output-on-failure
+ --schedule-random
+ --parallel ${CMAKE_BUILD_PARALLEL_LEVEL}
+)
+if(CMAKE_VERBOSE_MAKEFILE)
+ list(APPEND TEST_FLAGS --verbose)
+endif()
+
add_subdirectory(LuaJIT-tests)
add_subdirectory(PUC-Rio-Lua-5.1-tests)
add_subdirectory(lua-Harness-tests)
@@ -86,22 +95,3 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS
tarantool-c-tests
tarantool-tests
)
-
-if(LUAJIT_USE_TEST)
- if(POLICY CMP0037)
- if(CMAKE_VERSION VERSION_LESS 3.11)
- # CMake below 3.11 reserves the name 'test'. Use old policy.
- # https://cmake.org/cmake/help/v3.11/release/3.11.html#other-changes
- cmake_policy(SET CMP0037 OLD)
- else()
- # Starting from CMake 3.11 the name 'test' is reserved in
- # special cases and can be used as a target name.
- cmake_policy(SET CMP0037 NEW)
- endif()
- endif(POLICY CMP0037)
-
- add_custom_target(test DEPENDS
- ${PROJECT_NAME}-test
- ${PROJECT_NAME}-lint
- )
-endif()
diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
index 9cd76ee9..aa36097a 100644
--- a/test/LuaJIT-tests/CMakeLists.txt
+++ b/test/LuaJIT-tests/CMakeLists.txt
@@ -1,12 +1,21 @@
# See the rationale in the root CMakeLists.txt
cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
-add_custom_target(LuaJIT-tests DEPENDS ${LUAJIT_TEST_BINARY})
-
-add_custom_command(TARGET LuaJIT-tests
- COMMENT "Running LuaJIT-tests"
+# The test suite has its own test runner
+# (test/LuaJIT-tests/test.lua), it is not possible
+# to run these tests separately by CTest.
+message(STATUS "Add test test/LuaJIT-tests")
+add_test(NAME "test/LuaJIT-tests"
COMMAND
${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua
+slow +ffi +bit +jit
WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
)
+set_tests_properties("test/LuaJIT-tests" PROPERTIES
+ LABELS luajit
+)
+
+add_custom_target(LuaJIT-tests
+ COMMAND ${CMAKE_CTEST_COMMAND} -L luajit ${TEST_FLAGS}
+ DEPENDS luajit-main
+)
diff --git a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
index 98277f9a..af180c0a 100644
--- a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
+++ b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
@@ -34,17 +34,22 @@ add_subdirectory(libs)
# But, unfortunately, <ltests.c> depends on specific PUC-Rio
# Lua 5.1 internal headers and should be adapted for LuaJIT.
-add_custom_target(PUC-Rio-Lua-5.1-tests
- DEPENDS ${LUAJIT_TEST_BINARY} PUC-Rio-Lua-5.1-tests-prepare
+# The test suite has its own test runner
+# (test/PUC-Rio-Lua-5.1-tests/all.lua), it is not possible
+# to run these tests separately by CTest.
+message(STATUS "Add test test/PUC-Rio-Lua-5.1")
+add_test(NAME "test/PUC-Rio-Lua-5.1"
+ COMMAND ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
+ WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+)
+set_tests_properties("test/PUC-Rio-Lua-5.1" PROPERTIES
+ ENVIRONMENT "LUA_PATH=${LUA_PATH}\;\;"
+ LABELS lua
)
-add_custom_command(TARGET PUC-Rio-Lua-5.1-tests
- COMMENT "Running PUC-Rio Lua 5.1 tests"
- COMMAND
- env
- LUA_PATH="${LUA_PATH}"
- ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
- WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+add_custom_target(PUC-Rio-Lua-5.1-tests
+ COMMAND ${CMAKE_CTEST_COMMAND} -L lua ${TEST_FLAGS}
+ DEPENDS luajit-main PUC-Rio-Lua-5.1-tests-prepare
)
# vim: expandtab tabstop=2 shiftwidth=2
diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
index f748a8fd..70489061 100644
--- a/test/lua-Harness-tests/CMakeLists.txt
+++ b/test/lua-Harness-tests/CMakeLists.txt
@@ -4,12 +4,6 @@
# See the rationale in the root CMakeLists.txt
cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
-find_program(PROVE prove)
-if(NOT PROVE)
- message(WARNING "`prove' is not found, so lua-Harness-tests target is not generated")
- return()
-endif()
-
# Tests create temporary files (see 303-package.t and 411-luajit.t
# for example) to be required. Also, they require some files from
# the original test source directory.
@@ -20,23 +14,29 @@ make_lua_path(LUA_PATH
${LUAJIT_SOURCE_DIR}/?.lua
${LUAJIT_BINARY_DIR}/?.lua
)
-set(LUA_TEST_FLAGS --failures --shuffle)
-if(CMAKE_VERBOSE_MAKEFILE)
- list(APPEND LUA_TEST_FLAGS --verbose)
-endif()
+set(SUITE_NAME "lua-Harness-tests")
+file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.t")
+foreach(test_path ${tests})
+ get_filename_component(test_name ${test_path} NAME)
+ if (${test_name} STREQUAL ${SUITE_NAME})
+ continue()
+ endif (${test_name} STREQUAL ${SUITE_NAME})
+ set(test_title "test/${SUITE_NAME}/${test_name}")
+ message(STATUS "Add test ${test_title}")
+ add_test(NAME ${test_title}
+ 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}\;"
+ LABELS lua-Harness
+ )
+endforeach()
-add_custom_target(lua-Harness-tests DEPENDS ${LUAJIT_TEST_BINARY})
-add_custom_command(TARGET lua-Harness-tests
- COMMENT "Running lua-Harness tests"
- COMMAND
- env
- LUA_PATH="${LUA_PATH}"
- ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
- --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
- --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
- ${LUA_TEST_FLAGS}
- WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+add_custom_target(lua-Harness-tests
+ COMMAND ${CMAKE_CTEST_COMMAND} -L lua-Harness ${TEST_FLAGS}
+ DEPENDS luajit-main
)
# vim: expandtab tabstop=2 shiftwidth=2
diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
index 17255345..902828bf 100644
--- a/test/tarantool-c-tests/CMakeLists.txt
+++ b/test/tarantool-c-tests/CMakeLists.txt
@@ -1,15 +1,4 @@
-find_program(PROVE prove)
-if(NOT PROVE)
- message(WARNING "`prove' is not found, so tarantool-c-tests target is not generated")
- return()
-endif()
-
set(C_TEST_SUFFIX .c_test)
-set(C_TEST_FLAGS --failures --shuffle)
-
-if(CMAKE_VERBOSE_MAKEFILE)
- list(APPEND C_TEST_FLAGS --verbose)
-endif()
# Build libtest.
@@ -49,21 +38,29 @@ foreach(test_source ${tests})
LIST(APPEND TESTS_COMPILED ${exe})
endforeach()
+# Note, we cannot globbing generated files in CMake, see
+# https://stackoverflow.com/questions/44076307/cmake-globbing-generated-files
+# To workaround this we globbing source files and generated tests
+# with path to executable binaries.
+file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.test.c")
+set(SUITE_NAME "tarantool-c-tests")
+foreach(test_path ${tests})
+ get_filename_component(test_name ${test_path} NAME_WE)
+ if (${test_name} STREQUAL ${SUITE_NAME})
+ continue()
+ endif (${test_name} STREQUAL ${SUITE_NAME})
+ set(test_title "test/${SUITE_NAME}/${test_name}${C_TEST_SUFFIX}")
+ message(STATUS "Add test ${test_title}")
+ add_test(NAME ${test_title}
+ COMMAND ${CMAKE_CURRENT_BINARY_DIR}/${test_name}${C_TEST_SUFFIX}
+ WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+ )
+ set_tests_properties(${test_title} PROPERTIES
+ LABELS tarantool-c
+ )
+endforeach()
+
add_custom_target(tarantool-c-tests
+ COMMAND ${CMAKE_CTEST_COMMAND} -L tarantool-c ${TEST_FLAGS}
DEPENDS libluajit libtest ${TESTS_COMPILED}
)
-
-add_custom_command(TARGET tarantool-c-tests
- COMMENT "Running Tarantool C tests"
- COMMAND
- ${PROVE}
- ${CMAKE_CURRENT_BINARY_DIR}
- --ext ${C_TEST_SUFFIX}
- --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
- # Report any TAP parse errors, if any, since test module is
- # maintained by us.
- --parse
- ${C_TEST_FLAGS}
- WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
-)
-
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index c15d6037..e9d89366 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -4,12 +4,6 @@
# See the rationale in the root CMakeLists.txt.
cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
-find_program(PROVE prove)
-if(NOT PROVE)
- message(WARNING "`prove' is not found, so tarantool-tests target is not generated")
- return()
-endif()
-
macro(BuildTestCLib lib sources)
add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
target_include_directories(${lib} PRIVATE
@@ -83,21 +77,14 @@ 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
# <BuildTestLib> macro.
make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
set(LUA_TEST_SUFFIX .test.lua)
-set(LUA_TEST_FLAGS --failures --shuffle)
-set(LUA_TEST_ENV
- "LUA_PATH=\"${LUA_PATH}\""
- "LUA_CPATH=\"${LUA_CPATH}\""
-)
-
-if(CMAKE_VERBOSE_MAKEFILE)
- list(APPEND LUA_TEST_FLAGS --verbose)
-endif()
# XXX: Since the auxiliary libraries are built as a dynamically
# loaded modules on MacOS instead of shared libraries as it is
@@ -133,25 +120,22 @@ else()
list(APPEND LUA_TEST_ENV_MORE LD_LIBRARY_PATH=${LD_LIBRARY_PATH})
endif()
-# LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
-# with dependecies are set in scope of BuildTestLib macro.
+file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
+foreach(test_path ${tests})
+ get_filename_component(test_name ${test_path} NAME)
+ set(test_title "test/tarantool-tests/${test_name}")
+ message(STATUS "Add test ${test_title}")
+ add_test(NAME ${test_title}
+ COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
+ WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
+ )
+ set_tests_properties(${test_title} PROPERTIES
+ ENVIRONMENT "LUA_PATH=${LUA_PATH};\;\;;LUA_CPATH=${LUA_CPATH}\;\;;${LUA_TEST_ENV_MORE}"
+ LABELS tarantool
+ )
+endforeach()
+
add_custom_target(tarantool-tests
- DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS}
+ COMMAND ${CMAKE_CTEST_COMMAND} -L tarantool ${TEST_FLAGS}
+ DEPENDS luajit-main ${TESTLIBS}
)
-add_custom_command(TARGET tarantool-tests
- COMMENT "Running Tarantool tests"
- COMMAND
- # XXX: We can't move everything to the "inner" env, since there
- # are some issues with escaping ';' for different shells. As
- # a result LUA_PATH/LUA_CPATH variables are set via the "outer"
- # env, since they are not stripped by SIP like LD_*/DYLD_* are.
- env
- ${LUA_TEST_ENV}
- ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
- --exec 'env ${LUA_TEST_ENV_MORE} ${LUAJIT_TEST_COMMAND}'
- --ext ${LUA_TEST_SUFFIX}
- --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
- ${LUA_TEST_FLAGS}
- WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
-)
-
--
2.34.1
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2023-10-23 9:48 [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest Sergey Bronnikov via Tarantool-patches
@ 2023-10-24 13:41 ` Maxim Kokryashkin via Tarantool-patches
2023-10-31 10:42 ` Sergey Bronnikov via Tarantool-patches
2023-10-31 19:17 ` Igor Munkin via Tarantool-patches
2024-03-10 20:03 ` Igor Munkin via Tarantool-patches
2 siblings, 1 reply; 8+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-10-24 13:41 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: max.kokryashkin, tarantool-patches
Hi, Sergey!
Thanks for the patch!
Please consider my comments below.
On Mon, Oct 23, 2023 at 12:48:56PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
> From: Sergey Bronnikov <sergeyb@tarantool.org>
>
> The patch replaces the main test runner prove(1) with CTest, see [1] and
> [2].
>
> The benefits are:
>
> - less external dependencies, prove(1) is gone
> - running tests by test title
> - extended test running options in comparison to prove(1)
> - unified test output (finally!)
>
> Note, it is not possible to attach targets to the automatically
Typo: s/Note,/Note that/
> generated `test` target. It means that target `test` now is executing
Typo: s/now is/is now/
> `LuaJIT-test`, but not `LuaJIT-lint`.
>
> Note, the testsuites in `test/LuaJIT-tests` and in
Typo: s/Note,/Note that/
Typo: s/and in/and/
> `test/PUC-Rio-Lua-5.1` directories have their own test runners and
> currently CTest runs these testsuites as a single tests. In a future we
Typo: s/as a/as/
Typo: s/In a future/In the future/
> could change these test runners to specify test from outside and after
Typo: s/test from/tests from/
Typo: s/outside/outside,/
> this we could run tests separately by CTest in these test suites.
>
> Note, it is not possible to add dependencies to `add_test()` in CMake,
Typo: s/Note,/Note that/
> see [3]. It means that all object files and executables must be build
Typo: s/build/built/
> before running `ctest(1)`.
>
> Examples of using CTest in a build directory:
>
> $ ctest # Running all tests.
> $ ctest -L tarantool # Running tests in tarantool-tests dir.
> $ ctest -L tarantool-c # Running tests in tarantool-c-tests dir.
> $ ctest -L lua-Harness # Running tests in lua-Harness dir.
> $ ctest -L luajit # Running tests in LuaJIT dir.
> $ ctest -L lua # Running tests in PUC-Rio-Lua-5.1 dir.
> $ ctest --rerun-fail
> $ ctest --output-on-failure
>
> 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
> ---
> Changes v2:
> - rebased to master
> - fixed tests that require loading object files
> - fixed backward compatibility with CMake targets
>
> PR: https://github.com/tarantool/tarantool/pull/9286
> Branch: https://github.com/tarantool/luajit/tree/ligurio/enable_test_target
For whatever reason, CI consistently fails for ASAN tests.
Also, AFAIK, `ctest` just checks the exit status for each of the test cases.
Hence, here are two questions:
1. Do we consider this a sufficient validation?
2. Shall we keep the `tap` module, or we can move to something
much simplier (even conventional assertions will do at this point)?
> Patch v1 was sent as RFC without list address in CC.
>
> CMakeLists.txt | 1 +
> test/CMakeLists.txt | 28 ++++--------
> test/LuaJIT-tests/CMakeLists.txt | 17 +++++--
> test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt | 23 ++++++----
> test/lua-Harness-tests/CMakeLists.txt | 42 +++++++++---------
> test/tarantool-c-tests/CMakeLists.txt | 49 ++++++++++----------
> test/tarantool-tests/CMakeLists.txt | 54 ++++++++---------------
> 7 files changed, 100 insertions(+), 114 deletions(-)
>
> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index eebf3d6f..1943d894 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -356,6 +356,7 @@ set(LUAJIT_TEST_INIT "${PROJECT_SOURCE_DIR}/test/luajit-test-init.lua" CACHE STR
> "Lua code need to be run before tests are started."
> )
>
> +enable_testing()
> add_subdirectory(test)
>
> # --- Misc rules ---------------------------------------------------------------
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index 58cba5ba..bc0c8dd8 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -73,6 +73,15 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
> separate_arguments(LUAJIT_TEST_COMMAND)
>
> +set(TEST_FLAGS
> + --output-on-failure
> + --schedule-random
> + --parallel ${CMAKE_BUILD_PARALLEL_LEVEL}
> +)
> +if(CMAKE_VERBOSE_MAKEFILE)
> + list(APPEND TEST_FLAGS --verbose)
> +endif()
> +
> add_subdirectory(LuaJIT-tests)
> add_subdirectory(PUC-Rio-Lua-5.1-tests)
> add_subdirectory(lua-Harness-tests)
> @@ -86,22 +95,3 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS
> tarantool-c-tests
> tarantool-tests
> )
> -
> -if(LUAJIT_USE_TEST)
> - if(POLICY CMP0037)
> - if(CMAKE_VERSION VERSION_LESS 3.11)
> - # CMake below 3.11 reserves the name 'test'. Use old policy.
> - # https://cmake.org/cmake/help/v3.11/release/3.11.html#other-changes
> - cmake_policy(SET CMP0037 OLD)
> - else()
> - # Starting from CMake 3.11 the name 'test' is reserved in
> - # special cases and can be used as a target name.
> - cmake_policy(SET CMP0037 NEW)
> - endif()
> - endif(POLICY CMP0037)
> -
> - add_custom_target(test DEPENDS
> - ${PROJECT_NAME}-test
> - ${PROJECT_NAME}-lint
> - )
> -endif()
> diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
> index 9cd76ee9..aa36097a 100644
> --- a/test/LuaJIT-tests/CMakeLists.txt
> +++ b/test/LuaJIT-tests/CMakeLists.txt
> @@ -1,12 +1,21 @@
> # See the rationale in the root CMakeLists.txt
> cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>
> -add_custom_target(LuaJIT-tests DEPENDS ${LUAJIT_TEST_BINARY})
> -
> -add_custom_command(TARGET LuaJIT-tests
> - COMMENT "Running LuaJIT-tests"
> +# The test suite has its own test runner
> +# (test/LuaJIT-tests/test.lua), it is not possible
> +# to run these tests separately by CTest.
It is better to say 'one-by-one' than 'separately'.
> +message(STATUS "Add test test/LuaJIT-tests")
> +add_test(NAME "test/LuaJIT-tests"
> COMMAND
> ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua
> +slow +ffi +bit +jit
> WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
> )
> +set_tests_properties("test/LuaJIT-tests" PROPERTIES
> + LABELS luajit
> +)
> +
> +add_custom_target(LuaJIT-tests
> + COMMAND ${CMAKE_CTEST_COMMAND} -L luajit ${TEST_FLAGS}
> + DEPENDS luajit-main
> +)
> diff --git a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> index 98277f9a..af180c0a 100644
> --- a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> +++ b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
> @@ -34,17 +34,22 @@ add_subdirectory(libs)
> # But, unfortunately, <ltests.c> depends on specific PUC-Rio
> # Lua 5.1 internal headers and should be adapted for LuaJIT.
>
> -add_custom_target(PUC-Rio-Lua-5.1-tests
> - DEPENDS ${LUAJIT_TEST_BINARY} PUC-Rio-Lua-5.1-tests-prepare
> +# The test suite has its own test runner
> +# (test/PUC-Rio-Lua-5.1-tests/all.lua), it is not possible
> +# to run these tests separately by CTest.
Same comment about 'one-by-one'
> +message(STATUS "Add test test/PUC-Rio-Lua-5.1")
> +add_test(NAME "test/PUC-Rio-Lua-5.1"
> + COMMAND ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> +)
> +set_tests_properties("test/PUC-Rio-Lua-5.1" PROPERTIES
> + ENVIRONMENT "LUA_PATH=${LUA_PATH}\;\;"
> + LABELS lua
> )
>
> -add_custom_command(TARGET PUC-Rio-Lua-5.1-tests
> - COMMENT "Running PUC-Rio Lua 5.1 tests"
> - COMMAND
> - env
> - LUA_PATH="${LUA_PATH}"
> - ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> +add_custom_target(PUC-Rio-Lua-5.1-tests
> + COMMAND ${CMAKE_CTEST_COMMAND} -L lua ${TEST_FLAGS}
> + DEPENDS luajit-main PUC-Rio-Lua-5.1-tests-prepare
> )
>
> # vim: expandtab tabstop=2 shiftwidth=2
> diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
> index f748a8fd..70489061 100644
> --- a/test/lua-Harness-tests/CMakeLists.txt
> +++ b/test/lua-Harness-tests/CMakeLists.txt
> @@ -4,12 +4,6 @@
> # See the rationale in the root CMakeLists.txt
> cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>
> -find_program(PROVE prove)
> -if(NOT PROVE)
> - message(WARNING "`prove' is not found, so lua-Harness-tests target is not generated")
> - return()
> -endif()
> -
> # Tests create temporary files (see 303-package.t and 411-luajit.t
> # for example) to be required. Also, they require some files from
> # the original test source directory.
> @@ -20,23 +14,29 @@ make_lua_path(LUA_PATH
> ${LUAJIT_SOURCE_DIR}/?.lua
> ${LUAJIT_BINARY_DIR}/?.lua
> )
> -set(LUA_TEST_FLAGS --failures --shuffle)
>
> -if(CMAKE_VERBOSE_MAKEFILE)
> - list(APPEND LUA_TEST_FLAGS --verbose)
> -endif()
> +set(SUITE_NAME "lua-Harness-tests")
> +file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.t")
> +foreach(test_path ${tests})
> + get_filename_component(test_name ${test_path} NAME)
> + if (${test_name} STREQUAL ${SUITE_NAME})
> + continue()
> + endif (${test_name} STREQUAL ${SUITE_NAME})
> + set(test_title "test/${SUITE_NAME}/${test_name}")
> + message(STATUS "Add test ${test_title}")
> + add_test(NAME ${test_title}
> + 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}\;"
> + LABELS lua-Harness
> + )
> +endforeach()
>
> -add_custom_target(lua-Harness-tests DEPENDS ${LUAJIT_TEST_BINARY})
> -add_custom_command(TARGET lua-Harness-tests
> - COMMENT "Running lua-Harness tests"
> - COMMAND
> - env
> - LUA_PATH="${LUA_PATH}"
> - ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
> - --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
> - --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
> - ${LUA_TEST_FLAGS}
> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> +add_custom_target(lua-Harness-tests
> + COMMAND ${CMAKE_CTEST_COMMAND} -L lua-Harness ${TEST_FLAGS}
> + DEPENDS luajit-main
> )
>
> # vim: expandtab tabstop=2 shiftwidth=2
> diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
> index 17255345..902828bf 100644
> --- a/test/tarantool-c-tests/CMakeLists.txt
> +++ b/test/tarantool-c-tests/CMakeLists.txt
> @@ -1,15 +1,4 @@
> -find_program(PROVE prove)
> -if(NOT PROVE)
> - message(WARNING "`prove' is not found, so tarantool-c-tests target is not generated")
> - return()
> -endif()
> -
> set(C_TEST_SUFFIX .c_test)
> -set(C_TEST_FLAGS --failures --shuffle)
> -
> -if(CMAKE_VERBOSE_MAKEFILE)
> - list(APPEND C_TEST_FLAGS --verbose)
> -endif()
>
> # Build libtest.
>
> @@ -49,21 +38,29 @@ foreach(test_source ${tests})
> LIST(APPEND TESTS_COMPILED ${exe})
> endforeach()
>
> +# Note, we cannot globbing generated files in CMake, see
Typo: s/Note,/Note that/
Typo: s/globbing/glob/
> +# https://stackoverflow.com/questions/44076307/cmake-globbing-generated-files
> +# To workaround this we globbing source files and generated tests
Typo: s/this/this,/
Typo: s/globbing/glob/
> +# with path to executable binaries.
Typo: s/path/paths/
> +file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.test.c")
> +set(SUITE_NAME "tarantool-c-tests")
> +foreach(test_path ${tests})
> + get_filename_component(test_name ${test_path} NAME_WE)
> + if (${test_name} STREQUAL ${SUITE_NAME})
> + continue()
> + endif (${test_name} STREQUAL ${SUITE_NAME})
> + set(test_title "test/${SUITE_NAME}/${test_name}${C_TEST_SUFFIX}")
> + message(STATUS "Add test ${test_title}")
> + add_test(NAME ${test_title}
> + COMMAND ${CMAKE_CURRENT_BINARY_DIR}/${test_name}${C_TEST_SUFFIX}
> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> + )
> + set_tests_properties(${test_title} PROPERTIES
> + LABELS tarantool-c
> + )
> +endforeach()
> +
> add_custom_target(tarantool-c-tests
> + COMMAND ${CMAKE_CTEST_COMMAND} -L tarantool-c ${TEST_FLAGS}
> DEPENDS libluajit libtest ${TESTS_COMPILED}
> )
> -
> -add_custom_command(TARGET tarantool-c-tests
> - COMMENT "Running Tarantool C tests"
> - COMMAND
> - ${PROVE}
> - ${CMAKE_CURRENT_BINARY_DIR}
> - --ext ${C_TEST_SUFFIX}
> - --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
> - # Report any TAP parse errors, if any, since test module is
> - # maintained by us.
> - --parse
> - ${C_TEST_FLAGS}
> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> -)
> -
> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> index c15d6037..e9d89366 100644
> --- a/test/tarantool-tests/CMakeLists.txt
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -4,12 +4,6 @@
> # See the rationale in the root CMakeLists.txt.
> cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>
> -find_program(PROVE prove)
> -if(NOT PROVE)
> - message(WARNING "`prove' is not found, so tarantool-tests target is not generated")
> - return()
> -endif()
> -
> macro(BuildTestCLib lib sources)
> add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
> target_include_directories(${lib} PRIVATE
> @@ -83,21 +77,14 @@ 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
> # <BuildTestLib> macro.
> make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
>
> set(LUA_TEST_SUFFIX .test.lua)
> -set(LUA_TEST_FLAGS --failures --shuffle)
> -set(LUA_TEST_ENV
> - "LUA_PATH=\"${LUA_PATH}\""
> - "LUA_CPATH=\"${LUA_CPATH}\""
> -)
> -
> -if(CMAKE_VERBOSE_MAKEFILE)
> - list(APPEND LUA_TEST_FLAGS --verbose)
> -endif()
>
> # XXX: Since the auxiliary libraries are built as a dynamically
> # loaded modules on MacOS instead of shared libraries as it is
> @@ -133,25 +120,22 @@ else()
> list(APPEND LUA_TEST_ENV_MORE LD_LIBRARY_PATH=${LD_LIBRARY_PATH})
> endif()
>
> -# LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
> -# with dependecies are set in scope of BuildTestLib macro.
> +file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
> +foreach(test_path ${tests})
> + get_filename_component(test_name ${test_path} NAME)
> + set(test_title "test/tarantool-tests/${test_name}")
> + message(STATUS "Add test ${test_title}")
> + add_test(NAME ${test_title}
> + COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> + )
> + set_tests_properties(${test_title} PROPERTIES
> + ENVIRONMENT "LUA_PATH=${LUA_PATH};\;\;;LUA_CPATH=${LUA_CPATH}\;\;;${LUA_TEST_ENV_MORE}"
Not sure if it is better to construct the test env here. IMO, it becomes
more readable, when constructed prior to the iteration.
Also, I tend to believe that it kind of starnge to have `LUA_TEST_ENV_MORE`
now. IINM, it was needed because of the shell shenenigans and `prove` usage,
but know it sould be possible to put everything into a single env.
> + LABELS tarantool
> + )
> +endforeach()
> +
> add_custom_target(tarantool-tests
> - DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS}
> + COMMAND ${CMAKE_CTEST_COMMAND} -L tarantool ${TEST_FLAGS}
> + DEPENDS luajit-main ${TESTLIBS}
> )
> -add_custom_command(TARGET tarantool-tests
> - COMMENT "Running Tarantool tests"
> - COMMAND
> - # XXX: We can't move everything to the "inner" env, since there
> - # are some issues with escaping ';' for different shells. As
> - # a result LUA_PATH/LUA_CPATH variables are set via the "outer"
> - # env, since they are not stripped by SIP like LD_*/DYLD_* are.
> - env
> - ${LUA_TEST_ENV}
> - ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
> - --exec 'env ${LUA_TEST_ENV_MORE} ${LUAJIT_TEST_COMMAND}'
> - --ext ${LUA_TEST_SUFFIX}
> - --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
> - ${LUA_TEST_FLAGS}
> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> -)
> -
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2023-10-24 13:41 ` Maxim Kokryashkin via Tarantool-patches
@ 2023-10-31 10:42 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 0 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-10-31 10:42 UTC (permalink / raw)
To: Maxim Kokryashkin, Sergey Bronnikov; +Cc: max.kokryashkin, tarantool-patches
Hello,
On 10/24/23 16:41, Maxim Kokryashkin via Tarantool-patches wrote:
> Hi, Sergey!
> Thanks for the patch!
> Please consider my comments below.
> On Mon, Oct 23, 2023 at 12:48:56PM +0300, Sergey Bronnikov via Tarantool-patches wrote:
>> From: Sergey Bronnikov <sergeyb@tarantool.org>
>>
>> The patch replaces the main test runner prove(1) with CTest, see [1] and
>> [2].
>>
>> The benefits are:
>>
>> - less external dependencies, prove(1) is gone
>> - running tests by test title
>> - extended test running options in comparison to prove(1)
>> - unified test output (finally!)
>>
>> Note, it is not possible to attach targets to the automatically
> Typo: s/Note,/Note that/
Fixed.
>> generated `test` target. It means that target `test` now is executing
> Typo: s/now is/is now/
Fixed.
>> `LuaJIT-test`, but not `LuaJIT-lint`.
>>
>> Note, the testsuites in `test/LuaJIT-tests` and in
> Typo: s/Note,/Note that/
> Typo: s/and in/and/
Fixed.
>> `test/PUC-Rio-Lua-5.1` directories have their own test runners and
>> currently CTest runs these testsuites as a single tests. In a future we
> Typo: s/as a/as/
> Typo: s/In a future/In the future/
Fixed. But I would left "a future" because our future is undefined. :)
>> could change these test runners to specify test from outside and after
> Typo: s/test from/tests from/
> Typo: s/outside/outside,/
Fixed.
>> this we could run tests separately by CTest in these test suites.
>>
>> Note, it is not possible to add dependencies to `add_test()` in CMake,
> Typo: s/Note,/Note that/
Fixed.
>> see [3]. It means that all object files and executables must be build
> Typo: s/build/built/
Fixed.
>> before running `ctest(1)`.
>>
>> Examples of using CTest in a build directory:
>>
>> $ ctest # Running all tests.
>> $ ctest -L tarantool # Running tests in tarantool-tests dir.
>> $ ctest -L tarantool-c # Running tests in tarantool-c-tests dir.
>> $ ctest -L lua-Harness # Running tests in lua-Harness dir.
>> $ ctest -L luajit # Running tests in LuaJIT dir.
>> $ ctest -L lua # Running tests in PUC-Rio-Lua-5.1 dir.
>> $ ctest --rerun-fail
>> $ ctest --output-on-failure
>>
>> 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
>> ---
>> Changes v2:
>> - rebased to master
>> - fixed tests that require loading object files
>> - fixed backward compatibility with CMake targets
>>
>> PR: https://github.com/tarantool/tarantool/pull/9286
>> Branch: https://github.com/tarantool/luajit/tree/ligurio/enable_test_target
> For whatever reason, CI consistently fails for ASAN tests.
>
> Also, AFAIK, `ctest` just checks the exit status for each of the test cases.
> Hence, here are two questions:
> 1. Do we consider this a sufficient validation?
Yes, we do.
However, there is a nuance.
- exit code calculated in test:done method [1]. Under the hood :done
uses result provided by :check.
- test:done is not a mandatory in Lua test, you can run a test without it
- test will always exit with exit code equal to zero if someone will
forget to add test:done
I suggest to add atexit handler that will execute test:done.
1.
https://github.com/tarantool/luajit/blob/8aaac18647bcbfdf1f8c5a241219269b7829f9f7/test/tarantool-tests/tap.lua#L353-L362
> 2. Shall we keep the `tap` module, or we can move to something
> much simplier (even conventional assertions will do at this point)?
This patch changes test runner, not a unit-testing framework.
>
>> Patch v1 was sent as RFC without list address in CC.
>>
>> CMakeLists.txt | 1 +
>> test/CMakeLists.txt | 28 ++++--------
>> test/LuaJIT-tests/CMakeLists.txt | 17 +++++--
>> test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt | 23 ++++++----
>> test/lua-Harness-tests/CMakeLists.txt | 42 +++++++++---------
>> test/tarantool-c-tests/CMakeLists.txt | 49 ++++++++++----------
>> test/tarantool-tests/CMakeLists.txt | 54 ++++++++---------------
>> 7 files changed, 100 insertions(+), 114 deletions(-)
>>
>> diff --git a/CMakeLists.txt b/CMakeLists.txt
>> index eebf3d6f..1943d894 100644
>> --- a/CMakeLists.txt
>> +++ b/CMakeLists.txt
>> @@ -356,6 +356,7 @@ set(LUAJIT_TEST_INIT "${PROJECT_SOURCE_DIR}/test/luajit-test-init.lua" CACHE STR
>> "Lua code need to be run before tests are started."
>> )
>>
>> +enable_testing()
>> add_subdirectory(test)
>>
>> # --- Misc rules ---------------------------------------------------------------
>> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
>> index 58cba5ba..bc0c8dd8 100644
>> --- a/test/CMakeLists.txt
>> +++ b/test/CMakeLists.txt
>> @@ -73,6 +73,15 @@ add_custom_target(${PROJECT_NAME}-lint DEPENDS
>> set(LUAJIT_TEST_COMMAND "${LUAJIT_TEST_BINARY} -e dofile[[${LUAJIT_TEST_INIT}]]")
>> separate_arguments(LUAJIT_TEST_COMMAND)
>>
>> +set(TEST_FLAGS
>> + --output-on-failure
>> + --schedule-random
>> + --parallel ${CMAKE_BUILD_PARALLEL_LEVEL}
>> +)
>> +if(CMAKE_VERBOSE_MAKEFILE)
>> + list(APPEND TEST_FLAGS --verbose)
>> +endif()
>> +
>> add_subdirectory(LuaJIT-tests)
>> add_subdirectory(PUC-Rio-Lua-5.1-tests)
>> add_subdirectory(lua-Harness-tests)
>> @@ -86,22 +95,3 @@ add_custom_target(${PROJECT_NAME}-test DEPENDS
>> tarantool-c-tests
>> tarantool-tests
>> )
>> -
>> -if(LUAJIT_USE_TEST)
>> - if(POLICY CMP0037)
>> - if(CMAKE_VERSION VERSION_LESS 3.11)
>> - # CMake below 3.11 reserves the name 'test'. Use old policy.
>> - # https://cmake.org/cmake/help/v3.11/release/3.11.html#other-changes
>> - cmake_policy(SET CMP0037 OLD)
>> - else()
>> - # Starting from CMake 3.11 the name 'test' is reserved in
>> - # special cases and can be used as a target name.
>> - cmake_policy(SET CMP0037 NEW)
>> - endif()
>> - endif(POLICY CMP0037)
>> -
>> - add_custom_target(test DEPENDS
>> - ${PROJECT_NAME}-test
>> - ${PROJECT_NAME}-lint
>> - )
>> -endif()
>> diff --git a/test/LuaJIT-tests/CMakeLists.txt b/test/LuaJIT-tests/CMakeLists.txt
>> index 9cd76ee9..aa36097a 100644
>> --- a/test/LuaJIT-tests/CMakeLists.txt
>> +++ b/test/LuaJIT-tests/CMakeLists.txt
>> @@ -1,12 +1,21 @@
>> # See the rationale in the root CMakeLists.txt
>> cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>>
>> -add_custom_target(LuaJIT-tests DEPENDS ${LUAJIT_TEST_BINARY})
>> -
>> -add_custom_command(TARGET LuaJIT-tests
>> - COMMENT "Running LuaJIT-tests"
>> +# The test suite has its own test runner
>> +# (test/LuaJIT-tests/test.lua), it is not possible
>> +# to run these tests separately by CTest.
> It is better to say 'one-by-one' than 'separately'.
Updated, thanks.
>
>> +message(STATUS "Add test test/LuaJIT-tests")
>> +add_test(NAME "test/LuaJIT-tests"
>> COMMAND
>> ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/test.lua
>> +slow +ffi +bit +jit
>> WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR}
>> )
>> +set_tests_properties("test/LuaJIT-tests" PROPERTIES
>> + LABELS luajit
>> +)
>> +
>> +add_custom_target(LuaJIT-tests
>> + COMMAND ${CMAKE_CTEST_COMMAND} -L luajit ${TEST_FLAGS}
>> + DEPENDS luajit-main
>> +)
>> diff --git a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
>> index 98277f9a..af180c0a 100644
>> --- a/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
>> +++ b/test/PUC-Rio-Lua-5.1-tests/CMakeLists.txt
>> @@ -34,17 +34,22 @@ add_subdirectory(libs)
>> # But, unfortunately, <ltests.c> depends on specific PUC-Rio
>> # Lua 5.1 internal headers and should be adapted for LuaJIT.
>>
>> -add_custom_target(PUC-Rio-Lua-5.1-tests
>> - DEPENDS ${LUAJIT_TEST_BINARY} PUC-Rio-Lua-5.1-tests-prepare
>> +# The test suite has its own test runner
>> +# (test/PUC-Rio-Lua-5.1-tests/all.lua), it is not possible
>> +# to run these tests separately by CTest.
> Same comment about 'one-by-one'
The same - updated, thanks.
>> +message(STATUS "Add test test/PUC-Rio-Lua-5.1")
>> +add_test(NAME "test/PUC-Rio-Lua-5.1"
>> + COMMAND ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
>> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> +)
>> +set_tests_properties("test/PUC-Rio-Lua-5.1" PROPERTIES
>> + ENVIRONMENT "LUA_PATH=${LUA_PATH}\;\;"
>> + LABELS lua
>> )
>>
>> -add_custom_command(TARGET PUC-Rio-Lua-5.1-tests
>> - COMMENT "Running PUC-Rio Lua 5.1 tests"
>> - COMMAND
>> - env
>> - LUA_PATH="${LUA_PATH}"
>> - ${LUAJIT_TEST_COMMAND} ${CMAKE_CURRENT_SOURCE_DIR}/all.lua
>> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> +add_custom_target(PUC-Rio-Lua-5.1-tests
>> + COMMAND ${CMAKE_CTEST_COMMAND} -L lua ${TEST_FLAGS}
>> + DEPENDS luajit-main PUC-Rio-Lua-5.1-tests-prepare
>> )
>>
>> # vim: expandtab tabstop=2 shiftwidth=2
>> diff --git a/test/lua-Harness-tests/CMakeLists.txt b/test/lua-Harness-tests/CMakeLists.txt
>> index f748a8fd..70489061 100644
>> --- a/test/lua-Harness-tests/CMakeLists.txt
>> +++ b/test/lua-Harness-tests/CMakeLists.txt
>> @@ -4,12 +4,6 @@
>> # See the rationale in the root CMakeLists.txt
>> cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>>
>> -find_program(PROVE prove)
>> -if(NOT PROVE)
>> - message(WARNING "`prove' is not found, so lua-Harness-tests target is not generated")
>> - return()
>> -endif()
>> -
>> # Tests create temporary files (see 303-package.t and 411-luajit.t
>> # for example) to be required. Also, they require some files from
>> # the original test source directory.
>> @@ -20,23 +14,29 @@ make_lua_path(LUA_PATH
>> ${LUAJIT_SOURCE_DIR}/?.lua
>> ${LUAJIT_BINARY_DIR}/?.lua
>> )
>> -set(LUA_TEST_FLAGS --failures --shuffle)
>>
>> -if(CMAKE_VERBOSE_MAKEFILE)
>> - list(APPEND LUA_TEST_FLAGS --verbose)
>> -endif()
>> +set(SUITE_NAME "lua-Harness-tests")
>> +file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.t")
>> +foreach(test_path ${tests})
>> + get_filename_component(test_name ${test_path} NAME)
>> + if (${test_name} STREQUAL ${SUITE_NAME})
>> + continue()
>> + endif (${test_name} STREQUAL ${SUITE_NAME})
>> + set(test_title "test/${SUITE_NAME}/${test_name}")
>> + message(STATUS "Add test ${test_title}")
>> + add_test(NAME ${test_title}
>> + 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}\;"
>> + LABELS lua-Harness
>> + )
>> +endforeach()
>>
>> -add_custom_target(lua-Harness-tests DEPENDS ${LUAJIT_TEST_BINARY})
>> -add_custom_command(TARGET lua-Harness-tests
>> - COMMENT "Running lua-Harness tests"
>> - COMMAND
>> - env
>> - LUA_PATH="${LUA_PATH}"
>> - ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
>> - --exec '${LUAJIT_TEST_COMMAND} -l profile_luajit21'
>> - --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
>> - ${LUA_TEST_FLAGS}
>> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> +add_custom_target(lua-Harness-tests
>> + COMMAND ${CMAKE_CTEST_COMMAND} -L lua-Harness ${TEST_FLAGS}
>> + DEPENDS luajit-main
>> )
>>
>> # vim: expandtab tabstop=2 shiftwidth=2
>> diff --git a/test/tarantool-c-tests/CMakeLists.txt b/test/tarantool-c-tests/CMakeLists.txt
>> index 17255345..902828bf 100644
>> --- a/test/tarantool-c-tests/CMakeLists.txt
>> +++ b/test/tarantool-c-tests/CMakeLists.txt
>> @@ -1,15 +1,4 @@
>> -find_program(PROVE prove)
>> -if(NOT PROVE)
>> - message(WARNING "`prove' is not found, so tarantool-c-tests target is not generated")
>> - return()
>> -endif()
>> -
>> set(C_TEST_SUFFIX .c_test)
>> -set(C_TEST_FLAGS --failures --shuffle)
>> -
>> -if(CMAKE_VERBOSE_MAKEFILE)
>> - list(APPEND C_TEST_FLAGS --verbose)
>> -endif()
>>
>> # Build libtest.
>>
>> @@ -49,21 +38,29 @@ foreach(test_source ${tests})
>> LIST(APPEND TESTS_COMPILED ${exe})
>> endforeach()
>>
>> +# Note, we cannot globbing generated files in CMake, see
> Typo: s/Note,/Note that/
> Typo: s/globbing/glob/
Fixed.
>> +# https://stackoverflow.com/questions/44076307/cmake-globbing-generated-files
>> +# To workaround this we globbing source files and generated tests
> Typo: s/this/this,/
> Typo: s/globbing/glob/
Fixed.
>> +# with path to executable binaries.
> Typo: s/path/paths/
Fixed.
>> +file(GLOB tests ${CMAKE_CURRENT_SOURCE_DIR} "*.test.c")
>> +set(SUITE_NAME "tarantool-c-tests")
>> +foreach(test_path ${tests})
>> + get_filename_component(test_name ${test_path} NAME_WE)
>> + if (${test_name} STREQUAL ${SUITE_NAME})
>> + continue()
>> + endif (${test_name} STREQUAL ${SUITE_NAME})
>> + set(test_title "test/${SUITE_NAME}/${test_name}${C_TEST_SUFFIX}")
>> + message(STATUS "Add test ${test_title}")
>> + add_test(NAME ${test_title}
>> + COMMAND ${CMAKE_CURRENT_BINARY_DIR}/${test_name}${C_TEST_SUFFIX}
>> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> + )
>> + set_tests_properties(${test_title} PROPERTIES
>> + LABELS tarantool-c
>> + )
>> +endforeach()
>> +
>> add_custom_target(tarantool-c-tests
>> + COMMAND ${CMAKE_CTEST_COMMAND} -L tarantool-c ${TEST_FLAGS}
>> DEPENDS libluajit libtest ${TESTS_COMPILED}
>> )
>> -
>> -add_custom_command(TARGET tarantool-c-tests
>> - COMMENT "Running Tarantool C tests"
>> - COMMAND
>> - ${PROVE}
>> - ${CMAKE_CURRENT_BINARY_DIR}
>> - --ext ${C_TEST_SUFFIX}
>> - --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
>> - # Report any TAP parse errors, if any, since test module is
>> - # maintained by us.
>> - --parse
>> - ${C_TEST_FLAGS}
>> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> -)
>> -
>> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
>> index c15d6037..e9d89366 100644
>> --- a/test/tarantool-tests/CMakeLists.txt
>> +++ b/test/tarantool-tests/CMakeLists.txt
>> @@ -4,12 +4,6 @@
>> # See the rationale in the root CMakeLists.txt.
>> cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>>
>> -find_program(PROVE prove)
>> -if(NOT PROVE)
>> - message(WARNING "`prove' is not found, so tarantool-tests target is not generated")
>> - return()
>> -endif()
>> -
>> macro(BuildTestCLib lib sources)
>> add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
>> target_include_directories(${lib} PRIVATE
>> @@ -83,21 +77,14 @@ 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
>> # <BuildTestLib> macro.
>> make_lua_path(LUA_CPATH PATHS ${LUA_CPATHS})
>>
>> set(LUA_TEST_SUFFIX .test.lua)
>> -set(LUA_TEST_FLAGS --failures --shuffle)
>> -set(LUA_TEST_ENV
>> - "LUA_PATH=\"${LUA_PATH}\""
>> - "LUA_CPATH=\"${LUA_CPATH}\""
>> -)
>> -
>> -if(CMAKE_VERBOSE_MAKEFILE)
>> - list(APPEND LUA_TEST_FLAGS --verbose)
>> -endif()
>>
>> # XXX: Since the auxiliary libraries are built as a dynamically
>> # loaded modules on MacOS instead of shared libraries as it is
>> @@ -133,25 +120,22 @@ else()
>> list(APPEND LUA_TEST_ENV_MORE LD_LIBRARY_PATH=${LD_LIBRARY_PATH})
>> endif()
>>
>> -# LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
>> -# with dependecies are set in scope of BuildTestLib macro.
>> +file(GLOB_RECURSE tests ${CMAKE_CURRENT_SOURCE_DIR} "*${LUA_TEST_SUFFIX}")
>> +foreach(test_path ${tests})
>> + get_filename_component(test_name ${test_path} NAME)
>> + set(test_title "test/tarantool-tests/${test_name}")
>> + message(STATUS "Add test ${test_title}")
>> + add_test(NAME ${test_title}
>> + COMMAND ${LUAJIT_TEST_COMMAND} ${test_path}
>> + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> + )
>> + set_tests_properties(${test_title} PROPERTIES
>> + ENVIRONMENT "LUA_PATH=${LUA_PATH};\;\;;LUA_CPATH=${LUA_CPATH}\;\;;${LUA_TEST_ENV_MORE}"
> Not sure if it is better to construct the test env here. IMO, it becomes
> more readable, when constructed prior to the iteration.
> Also, I tend to believe that it kind of starnge to have `LUA_TEST_ENV_MORE`
> now. IINM, it was needed because of the shell shenenigans and `prove` usage,
> but know it sould be possible to put everything into a single env.
What is actually strange with LUA_TEST_ENV_MORE?
Value of LUA_TEST_ENV_MORE is different on Darwin and non-Darwin OSes
(see condition above in test/tarantool-tests/CMakeLists.txt).
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -120,6 +120,7 @@ else()
list(APPEND LUA_TEST_ENV_MORE LD_LIBRARY_PATH=${LD_LIBRARY_PATH})
endif()
+set(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)
@@ -130,7 +131,7 @@ foreach(test_path ${tests})
WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
)
set_tests_properties(${test_title} PROPERTIES
- ENVIRONMENT
"LUA_PATH=${LUA_PATH};\;\;;LUA_CPATH=${LUA_CPATH}\;\;;${LUA_TEST_ENV_MORE}"
+ ENVIRONMENT "${ENV}"
LABELS tarantool
)
endforeach()
>> + LABELS tarantool
>> + )
>> +endforeach()
>> +
>> add_custom_target(tarantool-tests
>> - DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS}
>> + COMMAND ${CMAKE_CTEST_COMMAND} -L tarantool ${TEST_FLAGS}
>> + DEPENDS luajit-main ${TESTLIBS}
>> )
>> -add_custom_command(TARGET tarantool-tests
>> - COMMENT "Running Tarantool tests"
>> - COMMAND
>> - # XXX: We can't move everything to the "inner" env, since there
>> - # are some issues with escaping ';' for different shells. As
>> - # a result LUA_PATH/LUA_CPATH variables are set via the "outer"
>> - # env, since they are not stripped by SIP like LD_*/DYLD_* are.
>> - env
>> - ${LUA_TEST_ENV}
>> - ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
>> - --exec 'env ${LUA_TEST_ENV_MORE} ${LUAJIT_TEST_COMMAND}'
>> - --ext ${LUA_TEST_SUFFIX}
>> - --jobs ${CMAKE_BUILD_PARALLEL_LEVEL}
>> - ${LUA_TEST_FLAGS}
>> - WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
>> -)
>> -
>> --
>> 2.34.1
>>
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2023-10-23 9:48 [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest Sergey Bronnikov via Tarantool-patches
2023-10-24 13:41 ` Maxim Kokryashkin via Tarantool-patches
@ 2023-10-31 19:17 ` Igor Munkin via Tarantool-patches
2023-11-11 17:05 ` Sergey Bronnikov via Tarantool-patches
2024-03-10 20:03 ` Igor Munkin via Tarantool-patches
2 siblings, 1 reply; 8+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-10-31 19:17 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: max.kokryashkin, tarantool-patches
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, <test> 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.
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
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.
4. I'm afraid the current changes break integration with Tarantool
tests. The reason is duplication of <test> target both in LuaJIT and
Tarantool build systems. AFAIU, moving <enable_testing> 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.
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.
I believe that's all for the starters.
--
Best regards,
IM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2023-10-31 19:17 ` Igor Munkin via Tarantool-patches
@ 2023-11-11 17:05 ` Sergey Bronnikov via Tarantool-patches
2023-11-16 16:15 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 1 reply; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2023-11-11 17:05 UTC (permalink / raw)
To: Igor Munkin, Sergey Bronnikov; +Cc: max.kokryashkin, 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, <test> 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 <test> target both in LuaJIT and
> Tarantool build systems. AFAIU, moving <enable_testing> 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.
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2023-11-11 17:05 ` Sergey Bronnikov via Tarantool-patches
@ 2023-11-16 16:15 ` Maxim Kokryashkin via Tarantool-patches
0 siblings, 0 replies; 8+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-11-16 16:15 UTC (permalink / raw)
To: Sergey Bronnikov; +Cc: Sergey Bronnikov, max.kokryashkin, tarantool-patches
[-- Attachment #1: Type: text/plain, Size: 3447 bytes --]
Hi, Sergey!
Thanks for the patch!
LGTM
--
Best regards,
Maxim Kokryashkin
>Суббота, 11 ноября 2023, 20:05 +03:00 от Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>:
>
>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, <test> 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 <test> target both in LuaJIT and
>> Tarantool build systems. AFAIU, moving <enable_testing> 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.
[-- Attachment #2: Type: text/html, Size: 4262 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2023-10-23 9:48 [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest Sergey Bronnikov via Tarantool-patches
2023-10-24 13:41 ` Maxim Kokryashkin via Tarantool-patches
2023-10-31 19:17 ` Igor Munkin via Tarantool-patches
@ 2024-03-10 20:03 ` Igor Munkin via Tarantool-patches
2024-03-12 7:09 ` Sergey Bronnikov via Tarantool-patches
2 siblings, 1 reply; 8+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2024-03-10 20:03 UTC (permalink / raw)
To: Sergey Bronnikov, Sergey Kaplun; +Cc: max.kokryashkin, tarantool-patches
Sergey,
I've pushed the new version[1], tweaked and rebased onto the bleeding
master, with a tiny fixup for LuaJIT tests series on the top of the
branch (I left no comments, but the fix relates to Sergey adventures
with CMake lists). Unfortunately, I automatically squashed all the
tweaks made to the original commit, but I hope it's not a problem.
Besides, there are still some issues with the new LuaJIT C tests being
run with the sanitizers. I hope Sergey will help you to figure out
what's going wrong.
When all the issues are resoluved, please send the new version of the
patch to the ml, to get all the required LGTMs.
[1]: https://github.com/tarantool/luajit/commit/7d71534
--
Best regards,
IM
^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest
2024-03-10 20:03 ` Igor Munkin via Tarantool-patches
@ 2024-03-12 7:09 ` Sergey Bronnikov via Tarantool-patches
0 siblings, 0 replies; 8+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2024-03-12 7:09 UTC (permalink / raw)
To: Igor Munkin, Sergey Bronnikov, Sergey Kaplun
Cc: tarantool-patches, max.kokryashkin
[-- Attachment #1: Type: text/plain, Size: 976 bytes --]
Igor,
On 3/10/24 23:03, Igor Munkin wrote:
> Sergey,
>
> I've pushed the new version[1], tweaked and rebased onto the bleeding
> master, with a tiny fixup for LuaJIT tests series on the top of the
> branch (I left no comments, but the fix relates to Sergey adventures
> with CMake lists).
Thanks!
> Unfortunately, I automatically squashed all the
> tweaks made to the original commit, but I hope it's not a problem.
>
> Besides, there are still some issues with the new LuaJIT C tests being
> run with the sanitizers. I hope Sergey will help you to figure out
> what's going wrong.
>
> When all the issues are resoluved, please send the new version of the
> patch to the ml, to get all the required LGTMs.
v3 is here [1]. Please review and respond within an acceptable time frame.
1.
https://lists.tarantool.org/tarantool-patches/62428a61c98eee87dfc91f2d8972988d80589d3e.1710226958.git.sergeyb@tarantool.org/T/#u
> [1]:https://github.com/tarantool/luajit/commit/7d71534
>
[-- Attachment #2: Type: text/html, Size: 2029 bytes --]
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2024-03-12 7:09 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-10-23 9:48 [Tarantool-patches] [PATCH luajit][v2] cmake: replace prove with CTest Sergey Bronnikov via Tarantool-patches
2023-10-24 13:41 ` Maxim Kokryashkin via Tarantool-patches
2023-10-31 10:42 ` Sergey Bronnikov via Tarantool-patches
2023-10-31 19:17 ` Igor Munkin via Tarantool-patches
2023-11-11 17:05 ` Sergey Bronnikov via Tarantool-patches
2023-11-16 16:15 ` Maxim Kokryashkin via Tarantool-patches
2024-03-10 20:03 ` Igor Munkin via Tarantool-patches
2024-03-12 7:09 ` Sergey Bronnikov via Tarantool-patches
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox