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

[Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Hi, Sergey!
Thanks for the review!

Please consider my answers below.

On 23.01.24, Sergey Bronnikov wrote:
> Hi, Sergey!
> 
> thanks for the patch!
> 
> see comments below
> 
> On 1/19/24 14:32, Sergey Kaplun wrote:
> > This patch changes `dofile()` 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;
> >   
> > -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
> why three dashes?

Three dashes are used before testcase name and parsed by test-runner.

> > +  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
> why three dashes?

Three dashes are used before testcase name and parsed by test-runner.

> >     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")
> > +    assert(nc < oc * 3, "GC step missing for cdata __index")
> Hm, why?

See the description in the commit message.

> >       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

-- 
Best regards,
Sergey Kaplun