Re: [Tarantool-patches] [PATCH 2/4] test: adjust luajit-tap testing machinery

[Igor Munkin <imun@tarantool.org>]

Vlad,

Thanks for your review!

On 05.04.20, Vladislav Shpilevoy wrote:
> Hi! Thanks for the patch!
> 
> On 27/03/2020 14:23, Igor Munkin wrote:
> > This changeset makes possible to run luajit-tap tests requiring
> > libraries implemented in C:
> > * symlink to luajit test is created on configuration phase instead of
> >   build one.
> 
> What is the difference? Is it really necessary for anything? To be
> able to see CMake files in there?

Exactly. I hoisted symlink creation to add Cmake subdirectories in the
further patches.

> 
> > * introduced a CMake function for building shared libraries required for
> >   luajit tests.
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> >  test/CMakeLists.txt | 15 +++++++++------
> >  1 file changed, 9 insertions(+), 6 deletions(-)
> > 
> > diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
> > index 9b5df7dc5..00fce7270 100644
> > --- a/test/CMakeLists.txt
> > +++ b/test/CMakeLists.txt
> > @@ -13,6 +13,13 @@ function(build_module module files)
> >      endif(TARGET_OS_DARWIN)
> >  endfunction()
> >  
> > +function(build_lualib lib sources)
> 
> This function is introduced in tarantool, but required
> for luajit. On the contrary, luajit is required for tarantool.
> This is cyclic dependency, which makes impossible to run such
> tests inside luajit. I think luajit tests should be kept self
> sufficient. Otherwise it makes no sense to keep them in luajit
> repository. If you anyway can't run them in there, and need to
> create some test-wrappers from tarantool's side.
> 
> Alternative is to move them completely to tarantool. But current
> way is not right. It is half of one and half of other solution.

We already moved them out from the tarantool repo in the scope of this
issue[1]. However, as you can see, we moved them only partially. This
fact also makes me frustrating and I'm going to rework LuaJIT test
infrastructure in the scope of the separate ticket[2].

> 
> > +    add_library(${lib} SHARED ${sources})
> > +    set_target_properties(${lib} PROPERTIES PREFIX "")
> > +    if(TARGET_OS_DARWIN)
> > +        set_target_properties(${lib} PROPERTIES LINK_FLAGS "-undefined dynamic_lookup")
> > +    endif(TARGET_OS_DARWIN)
> > +endfunction()

[1]: https://github.com/tarantool/tarantool/issues/4478
[2]: https://github.com/tarantool/tarantool/issues/4862

-- 
Best regards,
IM