Re: [Tarantool-patches] [PATCH 3/3] ci: enable LuaJIT tests in CI

[Igor Munkin via Tarantool-patches <tarantool-patches@dev.tarantool.org>]

Timur,

On 24.02.21, Timur Safin wrote:
> This 1 thing is the only one that is bother me: it looks dependency of
> Tarantool LuaJIT tests are not yet properly assigned between build 
> Targets we have.
> 
> Let me put it how it looks like from my side:
> - LuaJIT-test is phony, defined in third_party/luajit target and 
>   depends on tarantool-tests 
> - tarantool-tests are not phony (i.e. you could not run Tarantool-tests 
>   Twice, if you wish to)

Please, do not take it personally (at least since I've heard such
"bothering" from several teammates), but I consider this as a disease.
Why do you need to run the *passed* tests twice? To make *more* sure?
Kinda Windows-way... Unfortunately, I can't create a dialog window with
"Are you sure these tests passed?" message, so I removed the artefacts
produced by .PHONY rules you mentioned above. Now you can run <test> and
<luacheck> targets as much as you want.

> - third_party/luajit/test is also depending on LuaJIT-luacheck
>   Which is not phone either, and is not runnable if you
>   Run it outside of Tarantool build (because .luacheckrc is 
>   At the root of Tarantool repo, not LuaJIT repo).

This is not true, there is a LuaJIT specific .luacheckrc in LuaJIT repo.
The issue you've shared with me relates to the case when <bindir> is
located within <srcdir> -- so called "idiomatic build way". Since
luacheck is run against all Lua files within <srcdir>, autogenerated
jit/vmdef.lua (the single file producing luacheck warnings) is also
checked. I explicitly excluded it from the file list to be checked. Now
everything is fine.

> - Also LuaJIT-test is explicitly referred in the travis makefiles
>   And not implicitly called by dependency of Tarantool testing.

LuaJIT-test is a dependency for Tarantool testing (i.e. <test> target),
but Make is not used for running Tarantool tests in CI, so there is no
other convenient way than using <make LuaJIT-test> in .travis.mk. I hope
this will have been fixed in the future and it is definitely out of the
scope of this issue.

> 
> This bothers me a lot, looks like dependency tree is not working
> correctly at the moment. Could you please make luacheck and Tarantool-test
> targets phony, and add LuaJIT-test as prerequisite of Tarantool
> testing (in cmake)?

I hope the changes below are enough. Fixed in LuaJIT submodule,
squashed, force-pushed to the branch.

================================================================================

diff --git a/.gitignore b/.gitignore
index 22921bf..2103a30 100644
--- a/.gitignore
+++ b/.gitignore
@@ -18,7 +18,5 @@ cmake_install.cmake
 cmake_uninstall.cmake
 compile_commands.json
 install_manifest.txt
-luacheck.ok
 luajit-parse-memprof
 luajit.pc
-tests.ok
diff --git a/test/CMakeLists.txt b/test/CMakeLists.txt
index bb0ff91..d166c9d 100644
--- a/test/CMakeLists.txt
+++ b/test/CMakeLists.txt
@@ -6,33 +6,32 @@ cmake_minimum_required(VERSION 3.1 FATAL_ERROR)
 find_program(LUACHECK luacheck)
 if(LUACHECK)
   set(LUACHECK_RC ${PROJECT_SOURCE_DIR}/.luacheckrc)
-  set(LUACHECK_OK ${CMAKE_CURRENT_BINARY_DIR}/luacheck.ok)
   file(GLOB_RECURSE LUACHECK_DEPS ${PROJECT_SOURCE_DIR}/*.lua)
-  add_custom_command(
+  add_custom_target(${PROJECT_NAME}-luacheck
+    DEPENDS ${LUACHECK_RC} ${LUACHECK_DEPS}
+  )
+  add_custom_command(TARGET ${PROJECT_NAME}-luacheck
     COMMENT "Running luacheck static analysis"
-    OUTPUT ${LUACHECK_OK}
-    DEPENDS ${LUACHECK} ${LUACHECK_RC} ${LUACHECK_DEPS}
     COMMAND
       ${LUACHECK} ${PROJECT_SOURCE_DIR}
         --codes
         --config ${LUACHECK_RC}
-      && touch ${LUACHECK_OK}
-      # XXX: Filenames in .luacheckrc are considered relative to
-      # the working directory, hence luacheck should be run in the
-      # project root directory.
-      WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
+        # XXX: jit/vmdef.lua is an autogenerated Lua source, so
+        # there is no need to run luacheck against it. Hence
+        # explicitly exclude this file from the list for check.
+        --exclude-files ${LUAJIT_BINARY_DIR}/jit/vmdef.lua
+    # XXX: Filenames in .luacheckrc are considered relative to
+    # the working directory, hence luacheck should be run in the
+    # project root directory.
+    WORKING_DIRECTORY ${PROJECT_SOURCE_DIR}
   )
 else()
-  add_custom_command(
+  add_custom_target(${PROJECT_NAME}-luacheck)
+  add_custom_command(TARGET ${PROJECT_NAME}-luacheck
     COMMENT "`luacheck' is not found, so ${PROJECT_NAME}-luacheck target is dummy"
-    OUTPUT luacheck.ok
-    COMMAND touch luacheck.ok
-    WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
   )
 endif()
 
-add_custom_target(${PROJECT_NAME}-luacheck DEPENDS luacheck.ok)
-
 add_subdirectory(tarantool-tests)
 
 add_custom_target(${PROJECT_NAME}-test DEPENDS
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 52c3007..e9ac62a 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -72,10 +72,11 @@ file(GLOB TEST_DEPS ${CMAKE_CURRENT_SOURCE_DIR}/*${LUA_TEST_SUFFIX})
 
 # LUA_CPATH and LD_LIBRARY_PATH variables and also TESTLIBS list
 # with dependecies are set in scope of BuildTestLib macro.
-add_custom_command(
-  COMMENT "Running Tarantool tests"
-  OUTPUT tests.ok
+add_custom_target(tarantool-tests
   DEPENDS ${LUAJIT_TEST_BINARY} ${TESTLIBS} ${TEST_DEPS}
+)
+add_custom_command(TARGET tarantool-tests
+  COMMENT "Running Tarantool tests"
   COMMAND
   env
     LUA_PATH="${LUA_PATH}\;\;"
@@ -85,8 +86,6 @@ add_custom_command(
       --exec ${LUAJIT_TEST_BINARY}
       --ext ${LUA_TEST_SUFFIX}
       --failures --shuffle
-    && touch tests.ok
   WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}
 )
 
-add_custom_target(tarantool-tests DEPENDS tests.ok)

================================================================================

> 
> Thanks,
> Timur
> 

<snipped>

> 

-- 
Best regards,
IM