[Tarantool-patches] [PATCH] Remove hardcoded paths to LuaJIT tests

[Tarantool-patches] [PATCH] Remove hardcoded paths to LuaJIT tests

[Igor Munkin]

As a result of the review[1] for Timur patch, I've finally got the idea
how to reduce the struggling Kirill faces every time there is a new test
created within LuaJIT third party repo. The series contains of two
patches in both repos:
* the root CMakeLists.txt for test directory is introduced to LuaJIT
  repo in scope of the first patch
* the paths in test/CMakeLists.txt are accordingly adjusted

Issue: https://github.com/tarantool/tarantool/issues/5425
LuaJIT repo branches:
* https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake
* https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake-2.5
Tarantool repo branch (need to be cherry-picked to all stable branches):
* https://github.com/tarantool/tarantool/tree/imun/gh-5425-luajit-tests-cmake

CI is not green, but I've done my best. I see neither build fails nor
failures related to the luajit-tap tests here:
* https://gitlab.com/tarantool/tarantool/-/pipelines/203537445

[1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-October/020266.html

-- 
2.25.0

[Tarantool-patches] [PATCH luajit] test: introduce the root CMakeLists.txt

[Igor Munkin]

Considering the way the tests are built and run now, it's too fragile
and inconvenient to maintain the newly added tests outside this repo.
This change introduces the root CMakeLists.txt for test directory
enclosing all auxilliary activity if one is required.

Part of tarantool/tarantool#5425

Signed-off-by: Igor Munkin <imun@tarantool.org>
---

The patch is properly backported to the old branches too (the branch is
in the cover letter).

 test/CMakeLists.txt | 3 +++
 1 file changed, 3 insertions(+)
 create mode 100644 test/CMakeLists.txt

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
new file mode 100644
index 0000000..ec37580
--- /dev/null
+++ b/test/CMakeLists.txt
@@ -0,0 +1,3 @@
+add_subdirectory(gh-4427-ffi-sandwich)
+add_subdirectory(lj-flush-on-trace)
+add_subdirectory(misclib-getmetrics-capi)
-- 
2.25.0

[Tarantool-patches] [PATCH] test: adjust the path to LuaJIT test directory

[Igor Munkin]

To ease the further maintenance of the tests located in LuaJIT repo,
particular hardcoded paths are replaced with the root path to test
directory within LuaJIT third party submodule.

Closes #5425

Signed-off-by: Igor Munkin <imun@tarantool.org>
---
 test/CMakeLists.txt | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index 7b28e2bd4..10882c6a1 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -75,9 +75,8 @@ add_subdirectory(app-tap)
 add_subdirectory(box)
 add_subdirectory(box-tap)
 add_subdirectory(unit)
-add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/gh-4427-ffi-sandwich ${PROJECT_BINARY_DIR}/third_party/luajit/test/gh-4427-ffi-sandwich)
-add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/lj-flush-on-trace ${PROJECT_BINARY_DIR}/third_party/luajit/test/lj-flush-on-trace)
-add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test/misclib-getmetrics-capi ${PROJECT_BINARY_DIR}/third_party/luajit/test/misclib-getmetrics-capi)
+add_subdirectory(${PROJECT_SOURCE_DIR}/third_party/luajit/test
+                 ${PROJECT_BINARY_DIR}/third_party/luajit/test)
 
 # Move tarantoolctl config
 if (NOT ${PROJECT_BINARY_DIR} STREQUAL ${PROJECT_SOURCE_DIR})
-- 
2.25.0

Re: [Tarantool-patches] [PATCH] Remove hardcoded paths to LuaJIT tests

[Timur Safin]

I could not resist and not highlight the fact that your patch "is not 
working" the same way as mine :)
https://gitlab.com/tarantool/tarantool/-/pipelines/203157106
(Or putting it the other way around - mine was working also. Being less 
intrusive as patch only for single repository, not 2)

LGTM, in any case, if you prefer 2 repo patch.

P.S.
I do not know whether it means anything, but apparently Debian-based 
Distros are green now, and RPM-based and others are red. Could it be 
there anything special which is Debian-specific?

: -----Original Message-----
: From: Igor Munkin <imun@tarantool.org>
: Sent: Friday, October 16, 2020 3:12 PM
: To: Alexander V. Tikhonov <avtikhon@tarantool.org>; Timur Safin
: <tsafin@tarantool.org>; Alexander Turenko
: <alexander.turenko@tarantool.org>
: Cc: Igor Munkin <imun@tarantool.org>; tarantool-patches@dev.tarantool.org
: Subject: [PATCH] Remove hardcoded paths to LuaJIT tests
: 
: As a result of the review[1] for Timur patch, I've finally got the idea
: how to reduce the struggling Kirill faces every time there is a new test
: created within LuaJIT third party repo. The series contains of two
: patches in both repos:
: * the root CMakeLists.txt for test directory is introduced to LuaJIT
:   repo in scope of the first patch
: * the paths in test/CMakeLists.txt are accordingly adjusted
: 
: Issue: https://github.com/tarantool/tarantool/issues/5425
: LuaJIT repo branches:
: * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake
: * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-
: cmake-2.5
: Tarantool repo branch (need to be cherry-picked to all stable branches):
: * https://github.com/tarantool/tarantool/tree/imun/gh-5425-luajit-tests-
: cmake
: 
: CI is not green, but I've done my best. I see neither build fails nor
: failures related to the luajit-tap tests here:
: * https://gitlab.com/tarantool/tarantool/-/pipelines/203537445
: 
: [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-
: October/020266.html
: 
: --
: 2.25.0

Re: [Tarantool-patches] [PATCH] Remove hardcoded paths to LuaJIT tests

[Igor Munkin]

Timur,

Thanks for your review!

On 16.10.20, Timur Safin wrote:
> I could not resist and not highlight the fact that your patch "is not 
> working" the same way as mine :)

No, it's not. As I mentioned below, there are no failures related to the
luajit-tap tests both on my local machine and in CI (at least I failed
to find any). At the same time I see luajit-tap related failures on your
branch both on my local machine and in CI (here[1] and in other jobs
with enabled tests). So my patch works, but CI is blocked with other
fragile tests (e.g. related to the raft or txm).

> https://gitlab.com/tarantool/tarantool/-/pipelines/203157106
> (Or putting it the other way around - mine was working also. Being less 
> intrusive as patch only for single repository, not 2)

I'm not sure whether your patch does not require the follow up patch to
LuaJIT repo. You can try.

> 
> LGTM, in any case, if you prefer 2 repo patch.

Added your tag:
| Reviewed-by: Timur Safin <tsafin@tarantool.org>

> 
> P.S.
> I do not know whether it means anything, but apparently Debian-based 
> Distros are green now, and RPM-based and others are red. Could it be 
> there anything special which is Debian-specific?

It's quite simple: IIRC, Debian-based distros are green since these jobs
tests only package building and doesn't run Tarantool tests.

> 
> : -----Original Message-----
> : From: Igor Munkin <imun@tarantool.org>
> : Sent: Friday, October 16, 2020 3:12 PM
> : To: Alexander V. Tikhonov <avtikhon@tarantool.org>; Timur Safin
> : <tsafin@tarantool.org>; Alexander Turenko
> : <alexander.turenko@tarantool.org>
> : Cc: Igor Munkin <imun@tarantool.org>; tarantool-patches@dev.tarantool.org
> : Subject: [PATCH] Remove hardcoded paths to LuaJIT tests
> : 
> : As a result of the review[1] for Timur patch, I've finally got the idea
> : how to reduce the struggling Kirill faces every time there is a new test
> : created within LuaJIT third party repo. The series contains of two
> : patches in both repos:
> : * the root CMakeLists.txt for test directory is introduced to LuaJIT
> :   repo in scope of the first patch
> : * the paths in test/CMakeLists.txt are accordingly adjusted
> : 
> : Issue: https://github.com/tarantool/tarantool/issues/5425
> : LuaJIT repo branches:
> : * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake
> : * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-
> : cmake-2.5
> : Tarantool repo branch (need to be cherry-picked to all stable branches):
> : * https://github.com/tarantool/tarantool/tree/imun/gh-5425-luajit-tests-
> : cmake
> : 
> : CI is not green, but I've done my best. I see neither build fails nor
> : failures related to the luajit-tap tests here:
> : * https://gitlab.com/tarantool/tarantool/-/pipelines/203537445
> : 
> : [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-
> : October/020266.html
> : 
> : --
> : 2.25.0
> 
> 

[1]: https://gitlab.com/tarantool/tarantool/-/jobs/793387558#L3675

-- 
Best regards,
IM

Re: [Tarantool-patches] [PATCH] Remove hardcoded paths to LuaJIT tests

[Alexander V. Tikhonov]

Hi Igor, thanks for the patchset. As I see now issues appeared on it and
it really fixed testing. Patchset LGTM.

On Fri, Oct 16, 2020 at 03:11:32PM +0300, Igor Munkin wrote:
> As a result of the review[1] for Timur patch, I've finally got the idea
> how to reduce the struggling Kirill faces every time there is a new test
> created within LuaJIT third party repo. The series contains of two
> patches in both repos:
> * the root CMakeLists.txt for test directory is introduced to LuaJIT
>   repo in scope of the first patch
> * the paths in test/CMakeLists.txt are accordingly adjusted
> 
> Issue: https://github.com/tarantool/tarantool/issues/5425
> LuaJIT repo branches:
> * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake
> * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake-2.5
> Tarantool repo branch (need to be cherry-picked to all stable branches):
> * https://github.com/tarantool/tarantool/tree/imun/gh-5425-luajit-tests-cmake
> 
> CI is not green, but I've done my best. I see neither build fails nor
> failures related to the luajit-tap tests here:
> * https://gitlab.com/tarantool/tarantool/-/pipelines/203537445
> 
> [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-October/020266.html
> 
> -- 
> 2.25.0
> 

Re: [Tarantool-patches] [PATCH] Remove hardcoded paths to LuaJIT tests

[Igor Munkin]

Sasha,

Thanks for your review!

On 16.10.20, Alexander V. Tikhonov wrote:
> Hi Igor, thanks for the patchset. As I see now issues appeared on it and
> it really fixed testing. Patchset LGTM.

Added your tag:
| Reviewed-by: Alexander V. Tikhonov <avtikhon@tarantool.org>

> 
> On Fri, Oct 16, 2020 at 03:11:32PM +0300, Igor Munkin wrote:
> > As a result of the review[1] for Timur patch, I've finally got the idea
> > how to reduce the struggling Kirill faces every time there is a new test
> > created within LuaJIT third party repo. The series contains of two
> > patches in both repos:
> > * the root CMakeLists.txt for test directory is introduced to LuaJIT
> >   repo in scope of the first patch
> > * the paths in test/CMakeLists.txt are accordingly adjusted
> > 
> > Issue: https://github.com/tarantool/tarantool/issues/5425
> > LuaJIT repo branches:
> > * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake
> > * https://github.com/tarantool/luajit/tree/imun/gh-5425-luajit-tests-cmake-2.5
> > Tarantool repo branch (need to be cherry-picked to all stable branches):
> > * https://github.com/tarantool/tarantool/tree/imun/gh-5425-luajit-tests-cmake
> > 
> > CI is not green, but I've done my best. I see neither build fails nor
> > failures related to the luajit-tap tests here:
> > * https://gitlab.com/tarantool/tarantool/-/pipelines/203537445
> > 
> > [1]: https://lists.tarantool.org/pipermail/tarantool-patches/2020-October/020266.html
> > 
> > -- 
> > 2.25.0
> > 

-- 
Best regards,
IM