Re: [Tarantool-patches] [PATCH v2 luajit 09/26] test: enable <ffi_convert.lua> in LuaJIT-tests

[Maxim Kokryashkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the patch!
Please consider my comments below.

On Mon, Jan 29, 2024 at 01:45:09PM +0300, Sergey Kaplun wrote:
> This patch changes `dofile()` (which uses an unreliable relative file
> path and mutates `_G` with global functions to be defined) to the
> corresponding require and renames the C library in the `require()` call.
> It adds groups with the names of subtests and enables the test in
> <index>. It renames structure and enum typedefs to avoid conflicts with
> cdef in other tests. The GC threshold is increased since all tests run
> in a single process, so the number of GC objects is greater.
>
> Part of tarantool/tarantool#9398
> ---
>  test/LuaJIT-tests/lib/ffi/ffi_convert.lua | 35 +++++++++++------------
>  test/LuaJIT-tests/lib/ffi/index           |  1 +
>  2 files changed, 18 insertions(+), 18 deletions(-)
>
> diff --git a/test/LuaJIT-tests/lib/ffi/ffi_convert.lua b/test/LuaJIT-tests/lib/ffi/ffi_convert.lua
> index bd3fb1f9..284f2b53 100644
> --- a/test/LuaJIT-tests/lib/ffi/ffi_convert.lua
> +++ b/test/LuaJIT-tests/lib/ffi/ffi_convert.lua
> @@ -1,8 +1,7 @@
>  local ffi = require("ffi")
>
> -local ctest = require("ctest")
> -
> -dofile("../common/ffi_util.inc")
> +local ctest = require("libctest")
> +local fails = require("common.fails")
>
>  local tonumber = tonumber
>
> @@ -32,15 +31,15 @@ typedef struct arrinc_t {
>    int a[];
>  } arrinc_t;
>
> -typedef enum uenum_t {
> +typedef enum uenum_convert_t {
>    UE0, UE71 = 71, UE72
> -} uenum_t;
> +} uenum_convert_t;
`convert` acts as a namespace here, and those are usually used as a
prefix. Here and below.
>
> -typedef enum ienum_t {
> +typedef enum ienum_convert_t {
>    IE0, IEM12 = -12, IEM11
> -} ienum_t;
> +} ienum_convert_t;
>
> -typedef struct foo_t {
> +typedef struct foo_convert_t {
>    bool b;
>    int8_t i8;
>    uint8_t u8;
> @@ -79,9 +78,9 @@ typedef struct foo_t {
>    int si_guard;
>    nest_t sn;
>    uni_t ui;
> -  uenum_t ue;
> -  ienum_t ie;
> -} foo_t;
> +  uenum_convert_t ue;
> +  ienum_convert_t ie;
> +} foo_convert_t;
>
>  char *strcpy(char *dest, const char *src);
>  typedef struct FILE FILE;
> @@ -89,11 +88,11 @@ int fileno(FILE *stream);
>  int _fileno(FILE *stream);
>  ]]
>
> -do
> -  local foo_t = ffi.typeof("foo_t")
> -  local sz = ffi.sizeof(foo_t)
> -  local x = foo_t()
> -  local y = foo_t()
> +do --- misc
Is there any chance to split it up?
> +  local foo_convert_t = ffi.typeof("foo_convert_t")
> +  local sz = ffi.sizeof(foo_convert_t)
> +  local x = foo_convert_t()
> +  local y = foo_convert_t()
>    ffi.fill(x, sz, 0xff)
>    ffi.fill(y, sz, 0xee)
>
> @@ -769,7 +768,7 @@ do
>    x.ppf = ffi.C.strcpy
>  end
>
> -do
> +do --- GC cdata __index
>    collectgarbage()
>    local oc = collectgarbage("count")
>    local cd = ffi.new"struct { struct { int a; } x;}"
> @@ -780,7 +779,7 @@ do
>    for i=1,2 do
>      f(cd)
>      local nc = collectgarbage("count")
> -    assert(nc < oc + 200, "GC step missing for cdata __index")
Are you sure it is the lower bound threshold?
> +    assert(nc < oc * 3, "GC step missing for cdata __index")
>      jit.off(f)
>    end
>  end
> diff --git a/test/LuaJIT-tests/lib/ffi/index b/test/LuaJIT-tests/lib/ffi/index
> index 0a72daed..5c2be87f 100644
> --- a/test/LuaJIT-tests/lib/ffi/index
> +++ b/test/LuaJIT-tests/lib/ffi/index
> @@ -7,6 +7,7 @@ ffi_bitfield.lua
>  ffi_call.lua
>  ffi_callback.lua
>  ffi_const.lua
> +ffi_convert.lua
>  istype.lua
>  jit_array.lua
>  jit_complex.lua
> --
> 2.43.0
>