Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Igor Munkin <imun@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 3/5] test: run LuaJIT tests via CMake
Date: Sun, 14 Feb 2021 21:48:58 +0300	[thread overview]
Message-ID: <20210214184858.GE9361@root> (raw)
In-Reply-To: <6a03d693204cacc5791c75e1003efc150abb2979.1612291495.git.imun@tarantool.org>

Hi, Igor!

Thanks for the patch!
Finally LuaJIT-related tests become separated from Tarantool build
system!

I've looked at the branch version.
Please consider my comments below.

On 02.02.21, Igor Munkin wrote:
> This patch introduces a separate target to run all available tests. The
> whole testing machinery is reworked much but the existing tests are left

Typo: s/much but/much, but/

> mostly unchanged. However, considering the different ways to integrate
> LuaJIT into the parent project, this machinery provides two new
> configuration options:
> * LUAJIT_USE_TEST: there might be a parent project integrating LuaJIT
>   sources in its source tree as a third party library (e.g.
>   https://github.com/tarantool/tarantool), so <test> target can be
>   already reserved there. This option allows to omit <test> target
>   configuration for LuaJIT to respect CMP0002 policy.
> * LUAJIT_TEST_BINARY: if LuaJIT tests are used in parent project test
>   routine, provide an option to choose which binary (i.e. Lua runtime)
>   to be used for running them.
> 
> The latter option value is used as a dependency for tests, and its
> default value is $<TARGET_FILE:${LUAJIT_DEPS}>. Unfortunately older

Typo: s/Unfortunately older/Unfortunately, older/

> CMake can't expand the generator expression used in DEPENDS section of

Typo? s/in DEPENDS section/in the DEPENDS section/

> <add_custom_(command|target)>. As a result the CMake minimum required
> version is bumped to 3.1 project-wide. For more info see CMake Release

Typo: s/info see/info, see/

> notes[1] for 3.1 version.
> 
> Finally, existing tests are grouped and moved to a separate directory
> under the root test directory to make the further addition of other
> available test suites in scope of tarantool/tarantool#4064 and
> tarantool/tarantool#4473 easier.
> 
> Tarantool tests are implemented using Tarantool on-board TAP module[2],
> that is moved to LuaJIT repository with a little changes to save Lua

Typo: s/a little changes/little changes/

> chunks untouched. Other auxiliary files for Tarantool-specific testing
> (such as *.skipcond, suite.ini), in turn, are removed.
> 
> [1]: https://cmake.org/cmake/help/latest/release/3.1.html#commands
> [2]: https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/
> 
> Part of tarantool/tarantool#4862
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---

Side note: What kind of quotes is preferable for our code style in
LuaJIT?
The new <tap.lua> module is written with single quotes, but a lot of
code inside LuaJIT uses double quotes (see <src/jit/*.lua>, for
example). Also, as far as you've already changed indentation it will be
nice do not stop there and fix quotes and change `{ }` to `{}` as usual.

Side note: (this is more like discussion question) -- do we really need
to use Tarantool's <tap.lua> module here? I provide some
dissatisfactions, but as I say below, we can't just change this
module API because we want (if you remove this module from Tarantool
and save it only here).
If it is saved untouchable in Tarantool repo I see no reason to
avoid replacing it with another one more suitable (if we want) or
improving it. Amount of tests is really small now and it can be easily
adapted. May be we should take a look at Lua test-modules like [1] and
[2].

>  .gitignore                                    |   1 +
>  CMakeLists.txt                                |  31 +-
>  etc/CMakeLists.txt                            |   3 +-
>  src/CMakeLists.txt                            |   7 +-
>  src/host/CMakeLists.txt                       |   3 +-
>  test/CMakeLists.txt                           |  31 +-
>  test/gh-4427-ffi-sandwich.skipcond            |   7 -
>  test/gh-4427-ffi-sandwich/CMakeLists.txt      |   1 -
>  test/lj-flush-on-trace.skipcond               |   7 -
>  test/lj-flush-on-trace/CMakeLists.txt         |   1 -
>  test/misclib-getmetrics-capi.skipcond         |   7 -
>  test/misclib-getmetrics-capi/CMakeLists.txt   |   1 -
>  test/misclib-getmetrics-lapi.skipcond         |   7 -
>  test/suite.ini                                |   6 -
>  test/tarantool-tests/CMakeLists.txt           |  92 ++++++
>  .../gh-3196-incorrect-string-length.test.lua  |   2 +-
>  .../gh-4427-ffi-sandwich.test.lua             |  24 +-
>  .../gh-4427-ffi-sandwich/CMakeLists.txt       |   1 +
>  .../gh-4427-ffi-sandwich/libsandwich.c        |   0
>  ...gh-4476-fix-string-find-recording.test.lua |   2 +-
>  ...gh-4773-tonumber-fail-on-NUL-char.test.lua |   2 +-
>  .../lj-494-table-chain-infinite-loop.test.lua |   2 +-
>  ...lj-505-fold-no-strref-for-ptrdiff.test.lua |   2 +-
>  .../lj-524-fold-conv-respect-src-irt.test.lua |   2 +-
>  .../lj-flush-on-trace.test.lua                |  24 +-
>  .../lj-flush-on-trace/CMakeLists.txt          |   1 +
>  .../lj-flush-on-trace/libflush.c              |   0
>  .../misclib-getmetrics-capi.test.lua          |   5 +-
>  .../misclib-getmetrics-capi/CMakeLists.txt    |   1 +
>  .../misclib-getmetrics-capi/testgetmetrics.c  |   0
>  .../misclib-getmetrics-lapi.test.lua          |  10 +-
>  .../misclib-memprof-lapi.test.lua             |  15 +-
>  .../or-232-unsink-64-kptr.test.lua            |   0
>  test/tarantool-tests/tap.lua                  | 306 ++++++++++++++++++
>  test/tarantool-tests/utils.lua                |  43 +++
>  test/utils.lua                                |  33 --
>  tools/CMakeLists.txt                          |   2 +-
>  37 files changed, 569 insertions(+), 113 deletions(-)
>  delete mode 100644 test/gh-4427-ffi-sandwich.skipcond
>  delete mode 100644 test/gh-4427-ffi-sandwich/CMakeLists.txt
>  delete mode 100644 test/lj-flush-on-trace.skipcond
>  delete mode 100644 test/lj-flush-on-trace/CMakeLists.txt
>  delete mode 100644 test/misclib-getmetrics-capi.skipcond
>  delete mode 100644 test/misclib-getmetrics-capi/CMakeLists.txt
>  delete mode 100644 test/misclib-getmetrics-lapi.skipcond
>  delete mode 100644 test/suite.ini
>  create mode 100644 test/tarantool-tests/CMakeLists.txt
>  rename test/{ => tarantool-tests}/gh-3196-incorrect-string-length.test.lua (94%)
>  rename test/{ => tarantool-tests}/gh-4427-ffi-sandwich.test.lua (70%)
>  create mode 100644 test/tarantool-tests/gh-4427-ffi-sandwich/CMakeLists.txt
>  rename test/{ => tarantool-tests}/gh-4427-ffi-sandwich/libsandwich.c (100%)
>  rename test/{ => tarantool-tests}/gh-4476-fix-string-find-recording.test.lua (99%)
>  rename test/{ => tarantool-tests}/gh-4773-tonumber-fail-on-NUL-char.test.lua (95%)
>  rename test/{ => tarantool-tests}/lj-494-table-chain-infinite-loop.test.lua (99%)
>  rename test/{ => tarantool-tests}/lj-505-fold-no-strref-for-ptrdiff.test.lua (96%)
>  rename test/{ => tarantool-tests}/lj-524-fold-conv-respect-src-irt.test.lua (95%)
>  rename test/{ => tarantool-tests}/lj-flush-on-trace.test.lua (70%)
>  create mode 100644 test/tarantool-tests/lj-flush-on-trace/CMakeLists.txt
>  rename test/{ => tarantool-tests}/lj-flush-on-trace/libflush.c (100%)
>  rename test/{ => tarantool-tests}/misclib-getmetrics-capi.test.lua (97%)
>  create mode 100644 test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt
>  rename test/{ => tarantool-tests}/misclib-getmetrics-capi/testgetmetrics.c (100%)
>  rename test/{ => tarantool-tests}/misclib-getmetrics-lapi.test.lua (98%)
>  rename test/{ => tarantool-tests}/misclib-memprof-lapi.test.lua (86%)
>  rename test/{ => tarantool-tests}/or-232-unsink-64-kptr.test.lua (100%)
>  create mode 100644 test/tarantool-tests/tap.lua
>  create mode 100644 test/tarantool-tests/utils.lua
>  delete mode 100644 test/utils.lua
> 
> diff --git a/.gitignore b/.gitignore
> index a21ee1c..35d2580 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -19,3 +19,4 @@ compile_commands.json

<snipped>

> diff --git a/CMakeLists.txt b/CMakeLists.txt
> index 0dba5d8..62ac369 100644
> --- a/CMakeLists.txt
> +++ b/CMakeLists.txt
> @@ -6,7 +6,8 @@

<snipped>

> @@ -259,3 +260,31 @@ add_subdirectory(etc)
>  # --- Tools --------------------------------------------------------------------
>  
>  add_subdirectory(tools)
> +
> +# --- Testing source tree ------------------------------------------------------
> +
> +# Auxiliary options for testing.
> +
> +# FIXME: There might be a parent project integrating LuaJIT
> +# sources in its source tree as a third party library
> +# (e.g. https://github.com/tarantool/tarantool), so <test> target
> +# can be already reserved there. This option allows to omit <test>
> +# target configuration for LuaJIT to respect CMP0002 policy.
> +option(LUAJIT_USE_TEST "Generate <test> target" ON)
> +
> +# FIXME: If LuaJIT is used in parent project, provide an option
> +# to choose which binary to be used for running LuaJIT tests.
> +# XXX: This variable is used as a dependency for tests, and its
> +# default value is $<TARGET_FILE:${LUAJIT_DEPS}> assigned in <src>
> +# directory CMakeLists. Unfortunately CMake fails with generator
> +# expressions expansions used in <add_custom_(command|target)>.
> +# As a result the minimal required CMake version is set to 3.1.
> +# For more info see CMake Release notes for 3.1 version.

Typo: s/info see/info, see/

> +# https://cmake.org/cmake/help/latest/release/3.1.html#commands
> +# XXX: This options is moved below source tree processing since

Typo: s/options/option/

> +# the default binary target need to be generated.
> +set(LUAJIT_TEST_BINARY ${LUAJIT_BINARY} CACHE STRING
> +  "Lua implementation to be used for tests. Default is luajit."

Nit: In my opinion "Default is 'luajit'." is better here.
Feel free to ignore.

> +)
> +
> +add_subdirectory(test)
> diff --git a/etc/CMakeLists.txt b/etc/CMakeLists.txt
> index 4a4c3cd..d54fa79 100644
> --- a/etc/CMakeLists.txt
> +++ b/etc/CMakeLists.txt
> @@ -1,6 +1,7 @@
>  # Building supplementary materials for LuaJIT.
>  
> -cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR)
> +# See the rationale in the root CMakeLists.txt.
> +cmake_minimum_required(VERSION 3.1 FATAL_ERROR)

Why is the minimum required version for this module/subdirectory
increased?

>  
>  set(LUAJIT_PC_PREFIX ${CMAKE_INSTALL_PREFIX})
>  if(CMAKE_LIBRARY_ARCHITECTURE)
> diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
> index 8ada1a4..209b5f0 100644
> --- a/src/CMakeLists.txt
> +++ b/src/CMakeLists.txt
> @@ -2,7 +2,8 @@

<snipped>

> diff --git a/src/host/CMakeLists.txt b/src/host/CMakeLists.txt
> index fe2de5c..011a630 100644
> --- a/src/host/CMakeLists.txt
> +++ b/src/host/CMakeLists.txt
> @@ -1,6 +1,7 @@
>  # Building the toolchain for LuaJIT VM preprocessing.
>  
> -cmake_minimum_required(VERSION 2.8.12 FATAL_ERROR)
> +# See the rationale in the root CMakeLists.txt.
> +cmake_minimum_required(VERSION 3.1 FATAL_ERROR)

Ditto.

>  
>  # FIXME: Both minilua and buildvm need to be build with the HOST_*
>  # toolchain.
> diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> index ec37580..f05dd90 100644
> --- a/test/CMakeLists.txt
> +++ b/test/CMakeLists.txt
> @@ -1,3 +1,28 @@
> -add_subdirectory(gh-4427-ffi-sandwich)
> -add_subdirectory(lj-flush-on-trace)
> -add_subdirectory(misclib-getmetrics-capi)
> +# Running various test suites against LuaJIT.
> +
> +# See the rationale in the root CMakeLists.txt.
> +cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
> +
> +add_subdirectory(tarantool-tests)
> +
> +add_custom_target(${PROJECT_NAME}-test DEPENDS
> +  tarantool-tests
> +)
> +
> +if(LUAJIT_USE_TEST)
> +  if(POLICY CMP0037)
> +    if(CMAKE_VERSION VERSION_LESS 3.11)
> +      # cmake below 3.11 reserves name test. Use old policy.

Typo? s/name/the name/
Nit: for me 'test' looks clearer here.
Feel free to ignore.

> +      # https://cmake.org/cmake/help/v3.11/release/3.11.html#other-changes
> +      cmake_policy(SET CMP0037 OLD)
> +    else()
> +      # Starting from cmake 3.11 name test reserved in special

Ditto.

> +      # cases and can be used as target name.

Typo? /target name/the target name/

> +      cmake_policy(SET CMP0037 NEW)
> +    endif()
> +  endif(POLICY CMP0037)
> +
> +  add_custom_target(test DEPENDS
> +    ${PROJECT_NAME}-test
> +  )
> +endif()
> diff --git a/test/gh-4427-ffi-sandwich.skipcond b/test/gh-4427-ffi-sandwich.skipcond
> deleted file mode 100644
> index 2a2ec4d..0000000
> --- a/test/gh-4427-ffi-sandwich.skipcond
> +++ /dev/null
> @@ -1,7 +0,0 @@

<snipped>

> diff --git a/test/gh-4427-ffi-sandwich/CMakeLists.txt b/test/gh-4427-ffi-sandwich/CMakeLists.txt
> deleted file mode 100644
> index 995c6bb..0000000
> --- a/test/gh-4427-ffi-sandwich/CMakeLists.txt
> +++ /dev/null
> @@ -1 +0,0 @@
> -build_lualib(libsandwich libsandwich.c)
> diff --git a/test/lj-flush-on-trace.skipcond b/test/lj-flush-on-trace.skipcond
> deleted file mode 100644
> index 2a2ec4d..0000000
> --- a/test/lj-flush-on-trace.skipcond
> +++ /dev/null
> @@ -1,7 +0,0 @@

<snipped>

> diff --git a/test/lj-flush-on-trace/CMakeLists.txt b/test/lj-flush-on-trace/CMakeLists.txt
> deleted file mode 100644
> index a90452d..0000000
> --- a/test/lj-flush-on-trace/CMakeLists.txt
> +++ /dev/null
> @@ -1 +0,0 @@
> -build_lualib(libflush libflush.c)
> diff --git a/test/misclib-getmetrics-capi.skipcond b/test/misclib-getmetrics-capi.skipcond
> deleted file mode 100644
> index 2a2ec4d..0000000
> --- a/test/misclib-getmetrics-capi.skipcond
> +++ /dev/null
> @@ -1,7 +0,0 @@

<snipped>

> diff --git a/test/misclib-getmetrics-capi/CMakeLists.txt b/test/misclib-getmetrics-capi/CMakeLists.txt
> deleted file mode 100644
> index e7cc8f8..0000000
> --- a/test/misclib-getmetrics-capi/CMakeLists.txt
> +++ /dev/null
> @@ -1 +0,0 @@
> -build_lualib(testgetmetrics testgetmetrics.c)
> diff --git a/test/misclib-getmetrics-lapi.skipcond b/test/misclib-getmetrics-lapi.skipcond
> deleted file mode 100644
> index 2a2ec4d..0000000
> --- a/test/misclib-getmetrics-lapi.skipcond
> +++ /dev/null
> @@ -1,7 +0,0 @@

<snipped>

> diff --git a/test/suite.ini b/test/suite.ini
> deleted file mode 100644
> index 0b9d5e2..0000000
> --- a/test/suite.ini
> +++ /dev/null
> @@ -1,6 +0,0 @@

<snipped>

> diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
> new file mode 100644
> index 0000000..0be4b34
> --- /dev/null
> +++ b/test/tarantool-tests/CMakeLists.txt
> @@ -0,0 +1,92 @@
> +# Test suite that has been moved from Tarantool repository in
> +# scope of https://github.com/tarantool/tarantool/issues/4478.
> +
> +# 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(BuildTestLib lib sources)

Nit: BuildTestCLib or BuildTestLuaCLib is more verbose for me.
Feel free to ignore.

> +  add_library(${lib} SHARED EXCLUDE_FROM_ALL ${sources})
> +  target_include_directories(${lib} PRIVATE
> +    ${LUAJIT_SOURCE_DIR}
> +    ${CMAKE_CURRENT_SOURCE_DIR}
> +  )
> +  set_target_properties(${lib} PROPERTIES
> +    LIBRARY_OUTPUT_DIRECTORY "${CMAKE_CURRENT_BINARY_DIR}"
> +    PREFIX ""
> +  )
> +
> +  # XXX: This change affects current cmake variable scope and so

Typo: s/current/the current/

> +  # a user should care to don't use it in a top level scope.
> +  # The dynamic libraries are loaded with LuaJIT binary and use
> +  # symbols from it. So it is totally OK to have unresolved
> +  # symbols at build time.
> +  if(CMAKE_SYSTEM_NAME STREQUAL "Darwin")
> +    set_target_properties(${lib} PROPERTIES
> +      LINK_FLAGS "-undefined dynamic_lookup"
> +    )
> +  else()
> +    # FIXME: Unfortunately there is no another way to suppress

Typo: s/Unfortunately/Unfortunately,/

> +    # this linker option, so just strip it out from the flags.
> +    string(REPLACE "-Wl,--no-undefined" ""
> +      CMAKE_SHARED_LINKER_FLAGS "${CMAKE_SHARED_LINKER_FLAGS}"
> +    )
> +  endif()
> +  # XXX: Append the lib to be built to the dependecy list.

Typo: s/dependecy/dependency/

> +  # Unfortunately, CMake is a crap and there is no other way to
> +  # extend the list in parent scope but join two strings with
> +  # semicolon. If one finds the normal way to make it work, feel
> +  # free to reach me.
> +  set(TESTLIBS "${lib};${TESTLIBS}" PARENT_SCOPE)
> +  # Add the directory where the lib is built to the LUA_CPATH
> +  # environment variable, so interpreter can find and load it.

Typo: s/interpreter/the interpreter/

> +  # XXX: Here we see the other side of the coin. If one joins two
> +  # strings with semicolon, the value automatically becomes the
> +  # list. I have no idea what is wrong with this tool, but I found
> +  # a single working solution to make LUA_CPATH be a string via
> +  # "escaping" the semicolon right in string interpolation.
> +  set(LUA_CPATH "${CMAKE_CURRENT_BINARY_DIR}/?${CMAKE_SHARED_LIBRARY_SUFFIX}\;${LUA_CPATH}" PARENT_SCOPE)
> +  # Also add this directory to LD_LIBRARY_PATH environment
> +  # variable, so FFI machinery can find and load it.
> +  set(LD_LIBRARY_PATH "${CMAKE_CURRENT_BINARY_DIR}:${LD_LIBRARY_PATH}" PARENT_SCOPE)
> +endmacro()
> +
> +add_subdirectory(gh-4427-ffi-sandwich)
> +add_subdirectory(lj-flush-on-trace)
> +add_subdirectory(misclib-getmetrics-capi)
> +
> +# The part of memory profiler toolchain is located in tools

Typo: s/of memory profiler/of the memory profiler/

> +# directory and auxiliary tests-related modules are located in the
> +# current directory (but tests are run in the binary directory),
> +# so LUA_PATH need to be updated.
> +set(LUA_PATH

Side note: Eeeh, it will be nicier to extend this part only for this
test, but too much extra movement is necessary for that.
The best is the enemy of the good. (c)

> +  "${CMAKE_CURRENT_SOURCE_DIR}/?.lua\;${PROJECT_SOURCE_DIR}/tools/?.lua"
> +)
> +set(LUA_TEST_SUFFIX .test.lua)
> +file(GLOB TEST_DEPS ${CMAKE_CURRENT_SOURCE_DIR}/*${LUA_TEST_SUFFIX})
> +
> +# LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
> +# with dependecies are set in scope of BuildTestLib macro.
> +add_custom_command(
> +  COMMENT "Running Tarantool tests"
> +  OUTPUT tests.ok
> +  DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS} ${TEST_DEPS}
> +  COMMAND
> +  env
> +    LUA_PATH="${LUA_PATH}\;\;"
> +    LUA_CPATH="${LUA_CPATH}\;\;"
> +    LD_LIBRARY_PATH="${LD_LIBRARY_PATH}"
> +    ${PROVE} ${CMAKE_CURRENT_SOURCE_DIR}
> +      --exec ${LUAJIT_TEST_BINARY}
> +      --ext ${LUA_TEST_SUFFIX}
> +      --failures --shuffle

See nothing bad to add `--verbose` option here. It's better to see the
name of the single test, not the test file only.

> +    && touch tests.ok
> +  WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
> +)
> +
> +add_custom_target(tarantool-tests DEPENDS tests.ok)
> diff --git a/test/gh-3196-incorrect-string-length.test.lua b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
> similarity index 94%
> rename from test/gh-3196-incorrect-string-length.test.lua
> rename to test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
> index f135de7..edb728f 100755
> --- a/test/gh-3196-incorrect-string-length.test.lua
> +++ b/test/tarantool-tests/gh-3196-incorrect-string-length.test.lua
> @@ -1,4 +1,4 @@
> -#!/usr/bin/env tarantool
> +#!/usr/bin/env luajit

I think, that this line unnecessary, as much as an executable format of
the file. Same thoughts about all test files below.

>  
>  -- Miscellaneous test for LuaJIT bugs
>  local tap = require('tap')
> diff --git a/test/gh-4427-ffi-sandwich.test.lua b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> similarity index 70%
> rename from test/gh-4427-ffi-sandwich.test.lua
> rename to test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> index 9d5e50f..e9771e8 100755
> --- a/test/gh-4427-ffi-sandwich.test.lua
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich.test.lua
> @@ -1,22 +1,30 @@
> -#!/usr/bin/env tarantool
> +#!/usr/bin/env luajit
>  
>  if #arg == 0 then
> -  require('utils').selfrun(arg, {
> +
> +  local utils = require('utils')
> +
> +  -- Disabled on *BSD due to #4819.
> +  utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +
> +  utils.selfrun(arg, {
>      {
> -      arg = {
> +      arg  = {

This alignment is beauty, but unnecessary, please drop these changes.
Same remarks for the changes below.

>          1, -- hotloop (arg[1])

Side note: this may be not arg[1] or so on for Tarantool or pure LuaJIT
with additional flags. But good for now, so whatever.

>          1, -- trigger (arg[2])
>        },
> -      res = tostring(3), -- hotloop + trigger + 1
> -      msg = 'Trace is aborted',
> +      test = 'is',
> +      res  = tostring(3), -- hotloop + trigger + 1
> +      msg  = 'Trace is aborted',
>      },
>      {
> -      arg = {
> +      arg  = {
>          1, -- hotloop (arg[1])
>          2, -- trigger (arg[2])
>        },
> -      res = 'Lua VM re-entrancy is detected while executing the trace',
> -      msg = 'Trace is recorded',
> +      test = 'like',

What does it mean?

> +      res  = 'Lua VM re%-entrancy is detected while executing the trace',

This % is redundant.

> +      msg  = 'Trace is recorded',
>      },
>    })
>  end
> diff --git a/test/tarantool-tests/gh-4427-ffi-sandwich/CMakeLists.txt b/test/tarantool-tests/gh-4427-ffi-sandwich/CMakeLists.txt
> new file mode 100644
> index 0000000..5515567
> --- /dev/null
> +++ b/test/tarantool-tests/gh-4427-ffi-sandwich/CMakeLists.txt
> @@ -0,0 +1 @@
> +BuildTestLib(libsandwich libsandwich.c)
> diff --git a/test/gh-4427-ffi-sandwich/libsandwich.c b/test/tarantool-tests/gh-4427-ffi-sandwich/libsandwich.c
> similarity index 100%
> rename from test/gh-4427-ffi-sandwich/libsandwich.c
> rename to test/tarantool-tests/gh-4427-ffi-sandwich/libsandwich.c
> diff --git a/test/gh-4476-fix-string-find-recording.test.lua b/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua
> similarity index 99%
> rename from test/gh-4476-fix-string-find-recording.test.lua
> rename to test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua
> index fb55680..92a11d5 100755
> --- a/test/gh-4476-fix-string-find-recording.test.lua
> +++ b/test/tarantool-tests/gh-4476-fix-string-find-recording.test.lua
> @@ -1,4 +1,4 @@

<snipped>

> diff --git a/test/gh-4773-tonumber-fail-on-NUL-char.test.lua b/test/tarantool-tests/gh-4773-tonumber-fail-on-NUL-char.test.lua
> similarity index 95%
> rename from test/gh-4773-tonumber-fail-on-NUL-char.test.lua
> rename to test/tarantool-tests/gh-4773-tonumber-fail-on-NUL-char.test.lua
> index a660979..51e3142 100755
> --- a/test/gh-4773-tonumber-fail-on-NUL-char.test.lua
> +++ b/test/tarantool-tests/gh-4773-tonumber-fail-on-NUL-char.test.lua
> @@ -1,4 +1,4 @@

<snipped>

> diff --git a/test/lj-494-table-chain-infinite-loop.test.lua b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
> similarity index 99%
> rename from test/lj-494-table-chain-infinite-loop.test.lua
> rename to test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
> index 314eb29..9c04ef6 100755
> --- a/test/lj-494-table-chain-infinite-loop.test.lua
> +++ b/test/tarantool-tests/lj-494-table-chain-infinite-loop.test.lua
> @@ -1,4 +1,4 @@

<snipped>

> diff --git a/test/lj-505-fold-no-strref-for-ptrdiff.test.lua b/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua
> similarity index 96%
> rename from test/lj-505-fold-no-strref-for-ptrdiff.test.lua
> rename to test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua
> index a82bafa..6b0d70b 100755
> --- a/test/lj-505-fold-no-strref-for-ptrdiff.test.lua
> +++ b/test/tarantool-tests/lj-505-fold-no-strref-for-ptrdiff.test.lua
> @@ -1,4 +1,4 @@

<snipped>

> diff --git a/test/lj-524-fold-conv-respect-src-irt.test.lua b/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua
> similarity index 95%
> rename from test/lj-524-fold-conv-respect-src-irt.test.lua
> rename to test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua
> index a232eea..fb92bee 100755
> --- a/test/lj-524-fold-conv-respect-src-irt.test.lua
> +++ b/test/tarantool-tests/lj-524-fold-conv-respect-src-irt.test.lua
> @@ -1,4 +1,4 @@

<snipped>

> diff --git a/test/lj-flush-on-trace.test.lua b/test/tarantool-tests/lj-flush-on-trace.test.lua
> similarity index 70%
> rename from test/lj-flush-on-trace.test.lua
> rename to test/tarantool-tests/lj-flush-on-trace.test.lua
> index 0b3ccf4..f9a513f 100755
> --- a/test/lj-flush-on-trace.test.lua
> +++ b/test/tarantool-tests/lj-flush-on-trace.test.lua
> @@ -1,22 +1,30 @@
> -#!/usr/bin/env tarantool
> +#!/usr/bin/env luajit
>  
>  if #arg == 0 then
> -  require('utils').selfrun(arg, {
> +
> +  local utils = require('utils')
> +
> +  -- Disabled on *BSD due to #4819.
> +  utils.skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +
> +  utils.selfrun(arg, {
>      {
> -      arg = {
> +      arg  = {
>          1, -- hotloop (arg[1])
>          1, -- trigger (arg[2])
>        },
> -      res = 'OK',
> -      msg = 'Trace is aborted',
> +      test = 'is',

What does 'is' mean?

> +      res  = 'OK',
> +      msg  = 'Trace is aborted',
>      },
>      {
> -      arg = {
> +      arg  = {
>          1, -- hotloop (arg[1])
>          2, -- trigger (arg[2])
>        },
> -      res = 'JIT mode change is detected while executing the trace',
> -      msg = 'Trace is recorded',
> +      test = 'like',

Ditto.

> +      res  = 'JIT mode change is detected while executing the trace',
> +      msg  = 'Trace is recorded',
>      },
>    })
>  end
> diff --git a/test/tarantool-tests/lj-flush-on-trace/CMakeLists.txt b/test/tarantool-tests/lj-flush-on-trace/CMakeLists.txt
> new file mode 100644
> index 0000000..91a18a6
> --- /dev/null
> +++ b/test/tarantool-tests/lj-flush-on-trace/CMakeLists.txt
> @@ -0,0 +1 @@
> +BuildTestLib(libflush libflush.c)
> diff --git a/test/lj-flush-on-trace/libflush.c b/test/tarantool-tests/lj-flush-on-trace/libflush.c
> similarity index 100%
> rename from test/lj-flush-on-trace/libflush.c
> rename to test/tarantool-tests/lj-flush-on-trace/libflush.c
> diff --git a/test/misclib-getmetrics-capi.test.lua b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> similarity index 97%
> rename from test/misclib-getmetrics-capi.test.lua
> rename to test/tarantool-tests/misclib-getmetrics-capi.test.lua
> index 1ad6958..c418e9f 100755
> --- a/test/misclib-getmetrics-capi.test.lua
> +++ b/test/tarantool-tests/misclib-getmetrics-capi.test.lua
> @@ -1,4 +1,7 @@

<snipped>

> diff --git a/test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt b/test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt
> new file mode 100644
> index 0000000..cff0096
> --- /dev/null
> +++ b/test/tarantool-tests/misclib-getmetrics-capi/CMakeLists.txt
> @@ -0,0 +1 @@
> +BuildTestLib(testgetmetrics testgetmetrics.c)
> diff --git a/test/misclib-getmetrics-capi/testgetmetrics.c b/test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c
> similarity index 100%
> rename from test/misclib-getmetrics-capi/testgetmetrics.c
> rename to test/tarantool-tests/misclib-getmetrics-capi/testgetmetrics.c
> diff --git a/test/misclib-getmetrics-lapi.test.lua b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> similarity index 98%
> rename from test/misclib-getmetrics-lapi.test.lua
> rename to test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> index 3b3d1f8..959293d 100755
> --- a/test/misclib-getmetrics-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-getmetrics-lapi.test.lua
> @@ -1,9 +1,12 @@
> -#!/usr/bin/env tarantool
> +#!/usr/bin/env luajit
>  
>  -- This is a part of tarantool/luajit testing suite.
>  -- Major portions taken verbatim or adapted from the LuaVela testing suite.
>  -- Copyright (C) 2015-2019 IPONWEB Ltd.
>  
> +-- Disabled on *BSD due to #4819.
> +require('utils').skipcond(jit.os == 'BSD', 'Disabled due to #4819')
> +
>  local tap = require('tap')
>  
>  local test = tap.test("lib-misc-getmetrics")
> @@ -49,7 +52,10 @@ test:test("gc-allocated-freed", function(subtest)
>      subtest:plan(1)
>  
>      -- Force up garbage collect all dead objects.

Please, add comments about testing as a third party for the Tarantool
and GC finalizers to describe this change.

> -    collectgarbage("collect")
> +    repeat
> +        local count = collectgarbage("count")
> +        collectgarbage("collect")
> +    until collectgarbage("count") == count
>  
>      -- Bump getmetrics table and string keys allocation.
>      local old_metrics = misc.getmetrics()
> diff --git a/test/misclib-memprof-lapi.test.lua b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> similarity index 86%
> rename from test/misclib-memprof-lapi.test.lua
> rename to test/tarantool-tests/misclib-memprof-lapi.test.lua
> index dd484f4..2b97212 100755
> --- a/test/misclib-memprof-lapi.test.lua
> +++ b/test/tarantool-tests/misclib-memprof-lapi.test.lua
> @@ -8,19 +8,14 @@ test:plan(9)
>  jit.off()
>  jit.flush()
>  
> --- FIXME: Launch tests with LUA_PATH enviroment variable.
> -local path = arg[0]:gsub("[^/]+%.test%.lua", "")
> -local path_suffix = "../tools/?.lua;"
> -package.path = ("%s%s;"):format(path, path_suffix)..package.path
> -

Side note: Nice, thanks!

>  local table_new = require "table.new"
>  
>  local bufread = require "utils.bufread"
>  local memprof = require "memprof.parse"
>  local symtab = require "utils.symtab"
>  
> -local TMP_BINFILE = arg[0]:gsub("[^/]+%.test%.lua", "%.%1.memprofdata.tmp.bin")
> -local BAD_PATH = arg[0]:gsub("[^/]+%.test%.lua", "%1/memprofdata.tmp.bin")
> +local TMP_BINFILE = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%.%1.memprofdata.tmp.bin")
> +local BAD_PATH = arg[0]:gsub(".+/([^/]+)%.test%.lua$", "%1/memprofdata.tmp.bin")
>  
>  local function payload()
>    -- Preallocate table to avoid table array part reallocations.
> @@ -82,7 +77,7 @@ end
>  
>  -- Not a directory.
>  local res, err, errno = misc.memprof.start(BAD_PATH)
> -test:ok(res == nil and err:match("Not a directory"))
> +test:ok(res == nil and err:match("No such file or directory"))
>  test:ok(type(errno) == "number")
>  
>  -- Profiler is running.
> @@ -125,9 +120,9 @@ local free = fill_ev_type(events, symbols, "free")
>  -- the number of allocations.
>  -- 1 event - alocation of table by itself + 1 allocation
>  -- of array part as far it is bigger than LJ_MAX_COLOSIZE (16).
> -test:ok(check_alloc_report(alloc, 27, 25, 2))
> +test:ok(check_alloc_report(alloc, 22, 20, 2))
>  -- 100 strings allocations.
> -test:ok(check_alloc_report(alloc, 32, 25, 100))
> +test:ok(check_alloc_report(alloc, 27, 20, 100))
>  
>  -- Collect all previous allocated objects.
>  test:ok(free.INTERNAL.num == 102)
> diff --git a/test/or-232-unsink-64-kptr.test.lua b/test/tarantool-tests/or-232-unsink-64-kptr.test.lua
> similarity index 100%
> rename from test/or-232-unsink-64-kptr.test.lua
> rename to test/tarantool-tests/or-232-unsink-64-kptr.test.lua
> diff --git a/test/tarantool-tests/tap.lua b/test/tarantool-tests/tap.lua

Side note: firstly I thought that <tap.lua> will be clashing with tap
inside other suites, but these conflicts will be easily resolved by
LUA_PATH.

> new file mode 100644
> index 0000000..fd03132
> --- /dev/null
> +++ b/test/tarantool-tests/tap.lua

Feel free to stop me from refactoring anytime, but I try to dump all my
thoughts about this module now to be referred later.

Side note: It's a pity that there is no such module as Perl's Test::Deep [3]
in Lua (or at least I haven't seen one).

> @@ -0,0 +1,306 @@
> +--- tap.lua internal file.
> +---
> +--- The Test Anything Protocol vesion 13 producer.
> +---
> +
> +-- Initializer FFI for <iscdata> check.

This comment is misleading a bit, NULL usage is important too.

> +local ffi = require('ffi')
> +local NULL = ffi.new('void *')

Nit: What if LuaJIT is built without FFI?

> +
> +local function indent(chars)
> +  return (' '):rep(chars)
> +end
> +
> +local function traceback(level)
> +  local trace = { }

Nit: s/{ }/{}/ here and below.

> +  level = level or 3

Why 3? Please add the comment about this here.
Also, please mention about LuaJIT behaviour here (traceback not
reporting tail call) -- that's why traceback is one and the same for
`ok()` and `fail()` functions.

> +  while true do
> +    local info = debug.getinfo(level, 'nSl')
> +    if not info then break end

Nit: split this line to three.
Feel free to ignore.

> +    table.insert(trace, {
> +      source   = info.source,
> +      src      = info.short_src,
> +      line     = info.linedefined or 0,
> +      what     = info.what,
> +      name     = info.name,
> +      namewhat = info.namewhat,
> +      filename = info.source:sub(1, 1) == '@' and info.source:sub(2) or 'eval',

What does 'eval' mean?

Side note: Also `filename` looks redundant as far as `source` exists,
but break current API is not encouraged...

> +    })
> +    level = level + 1
> +  end
> +  return trace
> +end
> +
> +local function diag(test, fmt, ...)
> +  io.write(indent(4 * test.level), ('# %s\n'):format(fmt:format(...)))
> +end
> +
> +local function ok(test, cond, message, extra)
> +  test.total = test.total + 1
> +  if cond then
> +    io.write(indent(4 * test.level), ('ok - %s\n'):format(message))
> +    return true
> +  end
> +
> +  test.failed = test.failed + 1
> +  io.write(indent(4 * test.level), ('not ok - %s\n'):format(message))

Nit: 4 * test.level repeated several times. May be encapsulated inside
`local cur_level` variable.
Feel free to ignore.

> +
> +  -- Dump extra contents added in outer space.
> +  for key, value in pairs(extra or { }) do
> +    io.write(indent(2 + 4 * test.level), ('%s:\t%s\n'):format(key, value))
> +  end
> +
> +  if not test.trace then

Thanks for the simplifying this code chunk! I've pried into
<tarantool/src/lua/tap.lua> sources, and you really enhance it
gracefully!

> +    return false
> +  end
> +
> +  local trace = traceback()

Nit: Looks like this chunk can be separated inside one function
`dump_traceback()` back or whatever.
Feel free to ignore.

> +  local tindent = indent(4 + 4 * test.level)
> +  io.write(tindent, ('filename:\t%s\n'):format(trace[#trace].filename))
> +  io.write(tindent, ('line:\t%s\n'):format(trace[#trace].line))
> +  for frameno, frame in ipairs(trace) do
> +    io.write(tindent, ('frame #%d\n'):format(frameno))

Nit: Looks inconsistent with other output.

| io.write(tindent, ('frameno: #%d\n'):format(frameno))

hits better, in my opinion.

Looks like it will be good to add the function that does something like:

| io.write(indent, ('%s:\t%s\n'):format(key, value))

Feel free to ignore.

> +    local findent = indent(2) .. tindent

Nit: s/ .. /../
Here and below.

> +    for key, value in pairs(frame) do
> +      io.write(findent, ('%s:\t%s\n'):format(key, value))
> +    end
> +  end
> +  return false
> +end

<snipped>

> +local function cmpdeeply(got, expected, extra)
> +  if type(expected) == 'number' or type(got) == 'number' then
> +    extra.got = got
> +    extra.expected = expected
> +    -- Handle NaN.
> +    if got ~= got and expected ~= expected then
> +      return true
> +    end
> +    return got == expected
> +  end
> +
> +  if ffi.istype('bool', got) then got = (got == 1) end
> +  if ffi.istype('bool', expected) then expected = (expected == 1) end
> +
> +  if extra.strict and type(got) ~= type(expected) then
> +    extra.got = type(got)
> +    extra.expected = type(expected)
> +    return false
> +  end
> +
> +  if type(got) ~= 'table' or type(expected) ~= 'table' then
> +    extra.got = got
> +    extra.expected = expected
> +    return got == expected
> +  end
> +
> +  local path = extra.path or '/'

This `path` in `extra` field looks unusable, please drop it.

> +  local visited_keys = {}
> +
> +  for i, v in pairs(got) do

Nit: `k` instead `i` looks more convenient.

> +    visited_keys[i] = true

This is not good enough, visited keys should be cashed better:

| tarantool> local test = tap.test("<T>") local t1 = {1, 2, 3} t1[4] = {t1} local t2 = {1, 2, 3} t2[4] = {t2} return test:is_deeply(t1, t2), t1, t2
| TAP version 13
| ---
| - error: stack overflow

> +    extra.path = path .. '/' .. i
> +    if not cmpdeeply(v, expected[i], extra) then
> +      return false
> +    end
> +  end
> +
> +  -- Check if expected contains more keys then got.
> +  for i, v in pairs(expected) do

Ditto: `i` -> `k`.

> +    if visited_keys[i] ~= true and (extra.strict or v ~= NULL) then
> +      extra.expected = 'key ' .. tostring(i)
> +      extra.got = 'nil'
> +      return false
> +    end
> +  end
> +
> +  extra.path = path
> +
> +  return true
> +end

<snipped>

> +local function is(test, got, expected, message, extra)
> +  extra = extra or { }
> +  extra.got = got
> +  extra.expected = expected
> +  local rc = (test.strict == false or type(got) == type(expected))

Nit: It is nice to leave the comment here to provide description why
just `==` is not enough. Also, I don't like this behaviour -- it
disrespects `eq` metamethod, and there is not mentioned in the doc [4].

> +             and got == expected
> +  return ok(test, rc, message, extra)
> +end
> +
> +local function isnt(test, got, unexpected, message, extra)
> +  extra = extra or { }
> +  extra.got = got
> +  extra.unexpected = unexpected
> +  local rc = (test.strict == true and type(got) ~= type(unexpected))

Ditto.

> +             or got ~= unexpected
> +  return ok(test, rc, message, extra)
> +end
> +
> +local function is_deeply(test, got, expected, message, extra)
> +  extra = extra or { }
> +  extra.got = got
> +  extra.expected = expected
> +  extra.strict = test.strict
> +  return ok(test, cmpdeeply(got, expected, extra), message, extra)
> +end
> +
> +local function isnil(test, v, message, extra)
> +  return is(test, not v and v == nil and 'nil' or v, 'nil', message, extra)

Looks like type(v) 'nil' is enough. Or please add a comment why it is
necessary.

> +end
> +
> +local function isnumber(test, v, message, extra)
> +  return is(test, type(v), 'number', message, extra)
> +end
> +
> +local function isstring(test, v, message, extra)
> +  return is(test, type(v), 'string', message, extra)
> +end
> +
> +local function istable(test, v, message, extra)
> +  return is(test, type(v), 'table', message, extra)
> +end
> +
> +local function isboolean(test, v, message, extra)
> +  return is(test, type(v), 'boolean', message, extra)
> +end
> +
> +local function isfunction(test, v, message, extra)
> +  return is(test, type(v), 'function', message, extra)
> +end
> +
> +local function isudata(test, v, utype, message, extra)

Side note: Looks like it does not test type(v) equals "udata", that's
inconsistent with other checkers. But changing it equals to break
backward compatibility.

> +  extra = extra or { }
> +  extra.expected = ('userdata<%s>'):format(utype)
> +  if type(v) ~= 'userdata' then
> +    extra.got = type(v)
> +    return fail(test, message, extra)
> +  end
> +  extra.got = ('userdata<%s>'):format(getmetatable(v))
> +  return ok(test, getmetatable(v) == utype, message, extra)

Does it assume that compared udata-s should have the same metatable?

> +end
> +
> +local function iscdata(test, v, ctype, message, extra)

Ditto about inconsistency.

> +  extra = extra or { }
> +  extra.expected = ffi.typeof(ctype)
> +  if type(v) ~= 'cdata' then
> +    extra.got = type(v)
> +    return fail(test, message, extra)
> +  end
> +  extra.got = ffi.typeof(v)
> +  return ok(test, ffi.istype(ctype, v), message, extra)
> +end
> +
> +local test_mt

Nit: Why is it declared here, not inside the `new()` function?

> +
> +local function new(parent, name, fun, ...)

Side note: Why so serious :}?

> +  local level = parent ~= nil and parent.level + 1 or 0
> +  local test = setmetatable({
> +    parent  = parent,
> +    name    = name,
> +    level   = level,
> +    total   = 0,
> +    failed  = 0,
> +    planned = 0,
> +    trace   = parent == nil and true or parent.trace,
> +    strict  = false,

What does strict mean and how it can be changed?
I see nothing bad to copy some rationale from documentation [5] here
as comments for each field.

> +  }, test_mt)
> +  if fun == nil then
> +    return test
> +  end
> +  test:diag('%s', test.name)
> +  fun(test, ...)
> +  test:diag('%s: end', test.name)
> +  return test:check()
> +end
> +
> +local function plan(test, planned)
> +  test.planned = planned
> +  io.write(indent(4 * test.level), ('1..%d\n'):format(planned))

Side note: I like approach from Perl's Test::More with the `done_testing()`
function [6]. It is useful for bazaar-like developers and projects,
when tests are written fast, spontaneous and they don't care about plan
counting and updating. Just reflecting...

> +end
> +
> +local function check(test)
> +  if test.checked then
> +    error('check called twice')

Nit: eror_level = 2 is better here in my opinion.
Feel free to ignore.

> +  end
> +  test.checked = true
> +  if test.planned ~= test.total then
> +    if test.parent ~= nil then
> +      ok(test.parent, false, 'bad plan', {
> +        planned = test.planned,
> +        run = test.total,
> +      })
> +    else
> +      diag(test, ('bad plan: planned %d run %d')
> +        :format(test.planned, test.total))
> +    end
> +  elseif test.failed > 0 then
> +    if test.parent ~= nil then
> +      ok(test.parent, false, 'failed subtests', {
> +        failed = test.failed,
> +        planned = test.planned,
> +      })
> +    else
> +      diag(test, 'failed subtest: %d', test.failed)
> +    end
> +  else
> +    if test.parent ~= nil then
> +      ok(test.parent, true, test.name)
> +    end
> +  end

Nit: looks like `if test.parent ~= nil` can be taken out for branches.
Feel free to ignore.

> +  return test.planned == test.total and test.failed == 0
> +end
> +
> +test_mt = {
> +  __index = {
> +    test       = new,
> +    plan       = plan,
> +    check      = check,
> +    diag       = diag,
> +    ok         = ok,
> +    fail       = fail,
> +    skip       = skip,
> +    is         = is,
> +    isnt       = isnt,
> +    isnil      = isnil,
> +    isnumber   = isnumber,
> +    isstring   = isstring,
> +    istable    = istable,
> +    isboolean  = isboolean,
> +    isfunction = isfunction,
> +    isudata    = isudata,
> +    iscdata    = iscdata,
> +    is_deeply  = is_deeply,
> +    like       = like,
> +    unlike     = unlike,
> +  }
> +}
> +
> +return {
> +  test = function(...)
> +    io.write('TAP version 13\n')
> +    return new(nil, ...)
> +  end
> +}
> diff --git a/test/tarantool-tests/utils.lua b/test/tarantool-tests/utils.lua
> new file mode 100644
> index 0000000..197b138
> --- /dev/null
> +++ b/test/tarantool-tests/utils.lua
> @@ -0,0 +1,43 @@
> +local M = { }
> +
> +local tap = require('tap')
> +
> +function M.selfrun(arg, checks)
> +  local test = tap.test(arg[0]:match('/?(.+)%.test%.lua'))
> +
> +  test:plan(#checks)
> +
> +  local vars = {
> +    LUABIN = arg[-1],
> +    SCRIPT = arg[0],
> +    PATH   = arg[0]:gsub('%.test%.lua', ''),
> +    SUFFIX = package.cpath:match('?.(%a+);'),
> +  }
> +
> +  local cmd = string.gsub('LUA_PATH="<PATH>/?.lua;$LUA_PATH" ' ..

Why do you use that way instead <CMakeLists.txt> configuration?

> +                          'LUA_CPATH="<PATH>/?.<SUFFIX>;$LUA_CPATH" ' ..
> +                          'LD_LIBRARY_PATH=<PATH>:$LD_LIBRARY_PATH ' ..
> +                          '<LUABIN> 2>&1 <SCRIPT>', '%<(%w+)>', vars)
> +
> +  for _, ch in pairs(checks) do
> +    local testf = test[ch.test]
> +    assert(testf, ("tap doesn't provide test.%s function"):format(ch.test))
> +    local proc = io.popen((cmd .. (' %s'):rep(#ch.arg)):format(unpack(ch.arg)))
> +    local res = proc:read('*all'):gsub('^%s+', ''):gsub('%s+$', '')

Are these spaces important?

> +    -- XXX: explicitly pass <test> as an argument to <testf>
> +    -- to emulate test:is(...), test:like(...), etc.
> +    testf(test, res, ch.res, ch.msg)
> +  end
> +
> +  os.exit(test:check() and 0 or 1)
> +end

<snipped>

> diff --git a/test/utils.lua b/test/utils.lua
> deleted file mode 100644
> index 707d6ef..0000000
> --- a/test/utils.lua
> +++ /dev/null
> @@ -1,33 +0,0 @@

<snipped>

> diff --git a/tools/CMakeLists.txt b/tools/CMakeLists.txt
> index f9ffc5e..af3de33 100644
> --- a/tools/CMakeLists.txt
> +++ b/tools/CMakeLists.txt
> @@ -2,7 +2,7 @@
>  # Major portions taken verbatim or adapted from the uJIT.
>  # Copyright (C) 2015-2019 IPONWEB Ltd.
>  
> -# See the rationale in the root CMakeLists.txt
> +# See the rationale in the root CMakeLists.txt.

This is already fixed on branch! Thanks!

>  cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
>  
>  set(LUAJIT_TOOLS_DEPS)
> -- 
> 2.25.0
> 

[1]: https://luaunit.readthedocs.io/en/luaunit_v3_2_1/
[2]: https://fperrad.frama.io/lua-TestMore/
[3]: https://metacpan.org/pod/Test::Deep
[4]: https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/#taptest-is
[5]: https://www.tarantool.io/en/doc/latest/reference/reference_lua/tap/
[6]: https://perldoc.perl.org/Test::More#done_testing

-- 
Best regards,
Sergey Kaplun


  parent reply	other threads:[~2021-02-14 18:49 UTC|newest]

Thread overview: 46+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-02-02 20:57 [Tarantool-patches] [PATCH luajit 0/5] Self-sufficient LuaJIT testing environment Igor Munkin via Tarantool-patches
2021-02-02 20:57 ` [Tarantool-patches] [PATCH luajit 1/5] build: preserve the original build system Igor Munkin via Tarantool-patches
2021-02-04 22:53   ` Timur Safin via Tarantool-patches
2021-02-08 15:56     ` Igor Munkin via Tarantool-patches
2021-02-09 11:38   ` Sergey Kaplun via Tarantool-patches
2021-02-09 12:47     ` Igor Munkin via Tarantool-patches
2021-02-09 14:45       ` Sergey Kaplun via Tarantool-patches
2021-02-09 15:28         ` Igor Munkin via Tarantool-patches
2021-02-10  9:35           ` Sergey Kaplun via Tarantool-patches
2021-02-02 20:57 ` [Tarantool-patches] [PATCH luajit 2/5] build: replace GNU Make with CMake Igor Munkin via Tarantool-patches
2021-02-04 22:53   ` Timur Safin via Tarantool-patches
2021-02-08 15:56     ` Igor Munkin via Tarantool-patches
2021-02-09 13:55       ` Timur Safin via Tarantool-patches
2021-02-09 15:09         ` Igor Munkin via Tarantool-patches
2021-02-11 19:23   ` Sergey Kaplun via Tarantool-patches
2021-02-16 15:28     ` Igor Munkin via Tarantool-patches
2021-02-18  9:56       ` Sergey Kaplun via Tarantool-patches
2021-02-20 19:18         ` Igor Munkin via Tarantool-patches
2021-02-27 10:48           ` Sergey Kaplun via Tarantool-patches
2021-02-28 18:18             ` Igor Munkin via Tarantool-patches
2021-02-13  3:47   ` Sergey Kaplun via Tarantool-patches
2021-02-16 15:32     ` Igor Munkin via Tarantool-patches
2021-02-02 20:57 ` [Tarantool-patches] [PATCH luajit 3/5] test: run LuaJIT tests via CMake Igor Munkin via Tarantool-patches
2021-02-08 15:05   ` Timur Safin via Tarantool-patches
2021-02-08 16:29     ` Igor Munkin via Tarantool-patches
2021-02-09  8:16       ` Timur Safin via Tarantool-patches
2021-02-09  8:43         ` Igor Munkin via Tarantool-patches
2021-02-09 13:59           ` Timur Safin via Tarantool-patches
2021-02-09 15:10             ` Igor Munkin via Tarantool-patches
2021-02-14 18:48   ` Sergey Kaplun via Tarantool-patches [this message]
2021-02-19 19:04     ` Igor Munkin via Tarantool-patches
2021-02-27 13:50       ` Sergey Kaplun via Tarantool-patches
2021-02-28 18:18         ` Igor Munkin via Tarantool-patches
2021-02-02 20:57 ` [Tarantool-patches] [PATCH luajit 4/5] test: fix warnings found with luacheck in misclib* Igor Munkin via Tarantool-patches
     [not found]   ` <012f01d6fe1a$a2aa6890$e7ff39b0$@tarantool.org>
2021-02-08 15:57     ` Igor Munkin via Tarantool-patches
     [not found]     ` <2c495492-50f4-acfd-ad66-2cb44abb5fa1@tarantool.org>
2021-02-08 15:40       ` Sergey Bronnikov via Tarantool-patches
2021-02-08 15:58       ` Igor Munkin via Tarantool-patches
2021-02-14 19:16   ` Sergey Kaplun via Tarantool-patches
2021-02-16 15:29     ` Igor Munkin via Tarantool-patches
2021-02-16 16:36       ` Sergey Kaplun via Tarantool-patches
2021-02-02 20:57 ` [Tarantool-patches] [PATCH luajit 5/5] test: run luacheck static analysis via CMake Igor Munkin via Tarantool-patches
2021-02-04 22:52   ` Timur Safin via Tarantool-patches
2021-02-08 15:57     ` Igor Munkin via Tarantool-patches
2021-02-14 19:32   ` Sergey Kaplun via Tarantool-patches
2021-02-19 19:14     ` Igor Munkin via Tarantool-patches
2021-02-28 22:04 ` [Tarantool-patches] [PATCH luajit 0/5] Self-sufficient LuaJIT testing environment Igor Munkin via Tarantool-patches

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210214184858.GE9361@root \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=imun@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 3/5] test: run LuaJIT tests via CMake' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox