Tarantool development patches archive
 help / color / mirror / Atom feed
From: Sergey Bronnikov via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: Sergey Kaplun <skaplun@tarantool.org>,
	Maxim Kokryashkin <m.kokryashkin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org
Subject: Re: [Tarantool-patches] [PATCH luajit 02/25] test: separate LuaJIT helpers from ffi_util.inc
Date: Tue, 23 Jan 2024 12:17:39 +0300	[thread overview]
Message-ID: <0c05d0d4-94d6-4efe-a89f-2a5071561c0b@tarantool.org> (raw)
In-Reply-To: <6dc8864ddf6d90860cf96dff9c5d784a20595b62.1705661401.git.skaplun@tarantool.org>

Hi, Sergey!

thanks for the patch!

On 1/19/24 14:32, Sergey Kaplun wrote:
> This patch moves common helpers from <LuaJIT-tests/common/ffi_util.inc>
> into separate files by analogy with <common/expect_error.lua>. The
Why do you need to move to separate files? What is an idea behind it?
> `include()` helper isn't touched since it is used in system dependend
> tests, which won't be modified for now.
>
> Part of tarantool/tarantool#9398
> ---
>   test/LuaJIT-tests/common/fails.lua          |  3 +++
>   test/LuaJIT-tests/common/ffi/checkfail.lua  | 10 +++++++++
>   test/LuaJIT-tests/common/ffi/checktypes.lua | 11 ++++++++++
>   test/LuaJIT-tests/common/ffi_util.inc       | 23 ---------------------
>   4 files changed, 24 insertions(+), 23 deletions(-)
>   create mode 100644 test/LuaJIT-tests/common/fails.lua
>   create mode 100644 test/LuaJIT-tests/common/ffi/checkfail.lua
>   create mode 100644 test/LuaJIT-tests/common/ffi/checktypes.lua
>
> diff --git a/test/LuaJIT-tests/common/fails.lua b/test/LuaJIT-tests/common/fails.lua
> new file mode 100644
> index 00000000..d555a2b5
> --- /dev/null
> +++ b/test/LuaJIT-tests/common/fails.lua
> @@ -0,0 +1,3 @@
> +return function(f, ...)
> +  if pcall(f, ...) ~= false then error("failure expected", 2) end
> +end
> diff --git a/test/LuaJIT-tests/common/ffi/checkfail.lua b/test/LuaJIT-tests/common/ffi/checkfail.lua
> new file mode 100644
> index 00000000..60d8449a
> --- /dev/null
> +++ b/test/LuaJIT-tests/common/ffi/checkfail.lua
> @@ -0,0 +1,10 @@
> +local ffi = require("ffi")
> +
please add a comment with function's description
> +return function(t, f)
> +  f = f or ffi.typeof
> +  for i=1,1e9 do
> +    local tp = t[i]
> +    if not tp then break end
> +    assert(pcall(f, tp) == false, tp)
> +  end
> +end
> diff --git a/test/LuaJIT-tests/common/ffi/checktypes.lua b/test/LuaJIT-tests/common/ffi/checktypes.lua
> new file mode 100644
> index 00000000..6d748e3b
> --- /dev/null
> +++ b/test/LuaJIT-tests/common/ffi/checktypes.lua
> @@ -0,0 +1,11 @@
> +local ffi = require("ffi")
> +
please add a comment with function's description
> +return function(t)
> +  for i=1,1e9,3 do
> +    local tp = t[i+2]
> +    if not tp then break end
> +    local id = ffi.typeof(tp)
> +    assert(ffi.sizeof(id) == t[i], tp)
> +    assert(ffi.alignof(id) == t[i+1], tp)

Why test function contains asserts()? Wouldn't be better to return 
false/true and raise assert in test itself?

same questions below

> +  end
> +end
> diff --git a/test/LuaJIT-tests/common/ffi_util.inc b/test/LuaJIT-tests/common/ffi_util.inc
> index 1eee8dd9..9604d7b0 100644
> --- a/test/LuaJIT-tests/common/ffi_util.inc
> +++ b/test/LuaJIT-tests/common/ffi_util.inc
> @@ -4,29 +4,6 @@
>   
>   local ffi = require("ffi")
>   
> -function checkfail(t, f)
> -  f = f or ffi.typeof
> -  for i=1,1e9 do
> -    local tp = t[i]
> -    if not tp then break end
> -    assert(pcall(f, tp) == false, tp)
> -  end
> -end
> -
> -function checktypes(t)
> -  for i=1,1e9,3 do
> -    local tp = t[i+2]
> -    if not tp then break end
> -    local id = ffi.typeof(tp)
> -    assert(ffi.sizeof(id) == t[i], tp)
> -    assert(ffi.alignof(id) == t[i+1], tp)
> -  end
> -end
> -
> -function fails(f, ...)
> -  if pcall(f, ...) ~= false then error("failure expected", 2) end
> -end
> -
>   local incroot = os.getenv("INCROOT") or "/usr/include"
>   local cdefs = os.getenv("CDEFS") or ""
What for these env variables need?
>   

  reply	other threads:[~2024-01-23  9:17 UTC|newest]

Thread overview: 45+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-01-19 11:32 [Tarantool-patches] [PATCH luajit 00/25] More tests from LuaJIT-tests, part 1 Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 01/25] test: prepare lauxilarily libs for LuaJIT-tests Sergey Kaplun via Tarantool-patches
2024-01-23  9:10   ` Sergey Bronnikov via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 02/25] test: separate LuaJIT helpers from ffi_util.inc Sergey Kaplun via Tarantool-patches
2024-01-23  9:17   ` Sergey Bronnikov via Tarantool-patches [this message]
2024-01-23 12:35     ` Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 03/25] test: enable <ffi_arith_ptr.lua> in LuaJIT-tests Sergey Kaplun via Tarantool-patches
2024-01-23  9:21   ` Sergey Bronnikov via Tarantool-patches
2024-01-23 13:10     ` Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 04/25] test: enable <ffi_bitfield.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 05/25] test: enable <ffi_call.lua> " Sergey Kaplun via Tarantool-patches
2024-01-23  9:32   ` Sergey Bronnikov via Tarantool-patches
2024-01-23 12:46     ` Sergey Kaplun via Tarantool-patches
2024-01-24 11:05       ` Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 06/25] test: enable <ffi_callback.lua> " Sergey Kaplun via Tarantool-patches
2024-01-23  9:36   ` Sergey Bronnikov via Tarantool-patches
2024-01-23 12:01     ` Sergey Bronnikov via Tarantool-patches
2024-01-23 12:58     ` Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 07/25] test: enable <ffi_const.lua> " Sergey Kaplun via Tarantool-patches
2024-01-23  9:38   ` Sergey Bronnikov via Tarantool-patches
2024-01-23 11:59     ` Sergey Bronnikov via Tarantool-patches
2024-01-23 12:52       ` Sergey Kaplun via Tarantool-patches
2024-01-23 12:49     ` Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 08/25] test: enable <ffi_convert.lua> " Sergey Kaplun via Tarantool-patches
2024-01-23  9:39   ` Sergey Bronnikov via Tarantool-patches
2024-01-23 12:51     ` Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 09/25] test: enable <ffi_enum.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 10/25] test: enable <ffi_gcstep_recursive.lua> Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 11/25] test: enable <ffi_jit_arith.lua> in LuaJIT-tests Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 12/25] test: enable <ffi_jit_call.lua> " Sergey Kaplun via Tarantool-patches
2024-01-24 14:43   ` Sergey Bronnikov via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 13/25] test: enable <ffi_jit_conv.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 14/25] test: enable <ffi_lex_number.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 15/25] test: enable <ffi_metatype.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 16/25] test: enable <ffi_new.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 17/25] test: enable <ffi_parse_array.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 18/25] test: enable <ffi_parse_basic.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 19/25] test: enable <ffi_parse_cdef.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 20/25] test: enable <ffi_parse_struct.lua> LuaJIT test Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 21/25] test: enable <ffi_tabov.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 22/25] test: enable <lightud.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 23/25] test: enable <api_call.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 24/25] test: enable <catch_wrap.lua> " Sergey Kaplun via Tarantool-patches
2024-01-19 11:32 ` [Tarantool-patches] [PATCH luajit 25/25] test: enable <catch_cpp.lua> " Sergey Kaplun via Tarantool-patches
2024-01-23  9:01 ` [Tarantool-patches] [PATCH luajit 00/25] More tests from LuaJIT-tests, part 1 Sergey Bronnikov 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=0c05d0d4-94d6-4efe-a89f-2a5071561c0b@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=m.kokryashkin@tarantool.org \
    --cc=sergeyb@tarantool.org \
    --cc=skaplun@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH luajit 02/25] test: separate LuaJIT helpers from ffi_util.inc' \
    /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